macro help

[inftoconc]

Hi all,

I wrote a macro to read 8 bytes from a byte stream. Am not
sure if it is working ok. Can anyone please point out if there looks
to be a problem!

#define READ64(b) ( (uint64_t(*b)) << 56) + ((uint64_t(*(b+1))) << 48)
+ ((uint64_t(*(b+2))) << 40) +((uint64_t(*(b+3))) << 32)+((uint64_t(*(b
+4))) << 24) + (( uint64_t(*(b+5))) << 16) + (( uint64_t(*(b+6))) <<
8) + ( *(b+7))

Here b is a pointer to an unsigned char.

Thanks,

Vishal

 

[Ian Collins]

Hi all,

I wrote a macro to read 8 bytes from a byte stream. Am not
sure if it is working ok. Can anyone please point out if there looks
to be a problem!

What did your test cases show you?

#define READ64(b) ( (uint64_t(*b)) << 56) + ((uint64_t(*(b+1))) << 48)
+ ((uint64_t(*(b+2))) << 40) +((uint64_t(*(b+3))) << 32)+((uint64_t(*(b
+4))) << 24) + (( uint64_t(*(b+5))) << 16) + (( uint64_t(*(b+6))) <<
8) + ( *(b+7))

Why on Earth did you write a totally horrible macro when a function can
do the job and is a kinder on the eye and debugger?

 

[William Pursell]

I wrote a macro to read 8 bytes from a byte stream. Am not
sure if it is working ok. Can anyone please point out if there looks
to be a problem!

#define READ64(b) ( (uint64_t(*b)) << 56) + ((uint64_t(*(b+1))) << 48)
+ ((uint64_t(*(b+2))) << 40) +((uint64_t(*(b+3))) << 32)+((uint64_t(*(b
+4))) << 24) + (( uint64_t(*(b+5))) << 16) + (( uint64_t(*(b+6))) <<
8) + ( *(b+7))

Here b is a pointer to an unsigned char.

Lots of problems:

The endianness of the machine will have
an impact, certainly.

What happens if CHAR_BIT != 8? (If CHAR_BIT == 9,
you can't fit 8 chars into a 64 bit quantity.)

What if a caller wants chars 2 through 9 from a buffer
and calls the macro as:
uint64_t x = READ64( b+2 );

If you want to read 8 bytes from a stream, I highly
recommend fread().

It's not clear how you intend to use this macro. In
what way does it benefit your code?

 

[Charlie Gordon]

Hi all,

I wrote a macro to read 8 bytes from a byte stream. Am not
sure if it is working ok. Can anyone please point out if there looks
to be a problem!

#define READ64(b) ( (uint64_t(*b)) << 56) + ((uint64_t(*(b+1))) << 48)
+ ((uint64_t(*(b+2))) << 40) +((uint64_t(*(b+3))) << 32)+((uint64_t(*(b
+4))) << 24) + (( uint64_t(*(b+5))) << 16) + (( uint64_t(*(b+6))) <<
8) + ( *(b+7))

Here b is a pointer to an unsigned char.

The macro does not fit on a single line, you must use \ to ensure that the 4
lines get pasted together by the preprocessor.

Your casts to uint64_t are incorrect for C: use (uint64_t)x, not uint64_t(x)

You MUST use parentheses around b in the body of the macro in case b is not
a primary expression.

This macro evaluates its argument multiple times, it MUST not be used with
an argument that has side-effects.

You SHOULD use a function for this, possibly an inline function.

You should document that it fetches a 64 bit value from the octet stream in
big endian order.

fixed macro:

#define READ64(b) (((uint64_t)(b)[0] << 56) | \
((uint64_t)(b)[1] << 48) | \
((uint64_t)(b)[2] << 40) | \
((uint64_t)(b)[3] << 32) | \
((uint64_t)(b)[4] << 24) | \
((uint64_t)(b)[5] << 16) | \
((uint64_t)(b)[6] << 8) | \
((uint64_t)(b)[7] << 0))

Function:

static inline uint64_t read64(unsigned char const *b) {
return ((uint64_t)b[0] << 56) | ((uint64_t)b[1] << 48) |
((uint64_t)b[2] << 40) | ((uint64_t)b[3] << 32) |
((uint64_t)b[4] << 24) | ((uint64_t)b[5] << 16) |
((uint64_t)b[6] << 8) | ((uint64_t)b[7] << 0);
}

If your compiler is not too smart, this might be better:

static inline uint64_t read64(unsigned char const *b) {
return ((uint64_t)(((uint32_t)b[0] << 24) | ((uint32_t)b[1] << 16) |
((uint32_t)b[2] << 8) | b[3]) << 32) |
(((uint32_t)b[4] << 24) | ((uint32_t)b[5] << 16) |
((uint32_t)b[6] << 8) | b[7]);
}

 

[Army1987]

Hi all,

I wrote a macro to read 8 bytes from a byte stream. Am not
sure if it is working ok. Can anyone please point out if there looks
to be a problem!

#define READ64(b) ( (uint64_t(*b)) << 56) + ((uint64_t(*(b+1))) << 48)
+ ((uint64_t(*(b+2))) << 40) +((uint64_t(*(b+3))) << 32)+((uint64_t(*(b
+4))) << 24) + (( uint64_t(*(b+5))) << 16) + (( uint64_t(*(b+6))) <<
8) + ( *(b+7))

Here b is a pointer to an unsigned char.

1. The syntax of the cast is (type-name)cast-expression, e.g.
(uint64_t)*b, not uint64_t(*b). Maybe you were thinking about C++?
2. What's wrong with good ol' b[0], b[1] etc.? They're somewhat
clearer than (*(b+1)).

Try:
#define READ64(b) ( ((uint64_t)(b)[0] << 56) + ((uint64_t)(b)[1] << 48) +\
((uint64_t)(b)[2] << 40) + etc... )
(Also, if you're ever going to port this to somewhere CHAR_BIT > 8
you should mask each addend with & 0xFF, and if you aren't you
could add #if CHAR_BIT != 8
#error something
#endif
somewhere.)

 

[Army1987]

Lots of problems:

The endianness of the machine will have
an impact, certainly.

I think he did that to have the same format on all machines.
Otherwise he could use memcpy().

 

[Thad Smith]

Lots of problems:

The endianness of the machine will have
an impact, certainly.

The macro, as written, constructs an unsigned 64-bit integer from a
big-endian data stream of octets, regardless of the endedness of the
processor.

What happens if CHAR_BIT != 8? (If CHAR_BIT == 9,
you can't fit 8 chars into a 64 bit quantity.)

Either the input stream is defined as a series of octets or the code
must be rewritten for different widths of char. Since the data is
probably read from a file in a defined format, and the format is likely
a stream of octets, the code works on the majority of existing
processors and also works on those that read each octet to a single
unsigned char.

What if a caller wants chars 2 through 9 from a buffer
and calls the macro as:
uint64_t x = READ64( b+2 );

It works properly. It would, however, fail with arguments such as b+1<<2.

If you want to read 8 bytes from a stream, I highly
recommend fread().

Yes, that is good advice. The macro supplied doesn't read data from a
stream, of course, but rather converts it from a big endian stream of
octets to a value stored in a variable.

It's not clear how you intend to use this macro. In
what way does it benefit your code?

Having used similar conversion macros, it is clear to me: conversion
from a specified external format to a binary value in native format.

 

[Chris Torek]

You SHOULD use a function for this, possibly an inline function.

Indeed, in some cases it turns out to be faster to call a regular
(non-inline) function than to do the code in line (either with a
macro, or with an inline function). (As always, the way to find
out is to measure it. If you do a lot of these conversions, it
may be worth looking at your compiler's output. Many of today's
ILP32-LL64 compilers produce poor 64-bit shift-and-mask code, so
this sort of micro-optimization is occasionally worthwhile.)

Function:

static inline uint64_t read64(unsigned char const *b) {
return ((uint64_t)b[0] << 56) | ((uint64_t)b[1] << 48) |
((uint64_t)b[2] << 40) | ((uint64_t)b[3] << 32) |
((uint64_t)b[4] << 24) | ((uint64_t)b[5] << 16) |
((uint64_t)b[6] << 8) | ((uint64_t)b[7] << 0);
}

If your compiler is not too smart, this might be better:

static inline uint64_t read64(unsigned char const *b) {
return ((uint64_t)(((uint32_t)b[0] << 24) | ((uint32_t)b[1] << 16) |
((uint32_t)b[2] << 8) | b[3]) << 32) |
(((uint32_t)b[4] << 24) | ((uint32_t)b[5] << 16) |
((uint32_t)b[6] << 8) | b[7]);
}

I find that applications that do this tend also to do 16 and/or
32-bit numbers, so for the second case, consider, e.g.:

static inline uint32_t read32(const unsigned char *b) {
return ((uint32_t)b[0] << 24) | ((uint32_t)b[1] << 16) |
((unsigned)b[2] << 8) | b[3];
}
static inline uint64_t read64(const unsigned char *b) {
return ((uint64_t)read32(b) << 32) | read32(b + 4);
}

One might also consider also using "unsigned long" and "unsigned
long long" instead of the (possibly overspecified) uint32_t and
uint64_t aliases; and read32 can call read16() twice, and read16()
can use plain "unsigned int" (as the code in read32 above shows).

 

[Charlie Gordon]

It works properly. It would, however, fail with arguments such as b+1<<2.

No, you are mistaken: it does not work for b+2 either. It compiles, but
does not behave as intended. The MSB is read from b[0] and offset by 2.

 

[Charlie Gordon]

It works properly. It would, however, fail with arguments such as
b+1<<2.

No, you are mistaken: it does not work for b+2 either. It compiles, but
does not behave as intended. The MSB is read from b[0] and offset by 2.

Actually, it might compile as C++, but one has to fix the cast syntax for it
to compile as C ;-)

Complete answers have been posted on a different thread.

 

[Thad Smith]

William Pursell wrote:
On 30 Sep, 10:26, (e-mail address removed) wrote:
I wrote a macro to read 8 bytes from a byte stream. Am not
sure if it is working ok. Can anyone please point out if there looks
to be a problem!

#define READ64(b) ( (uint64_t(*b)) << 56) + ((uint64_t(*(b+1))) << 48)
+ ((uint64_t(*(b+2))) << 40) +((uint64_t(*(b+3))) << 32)+((uint64_t(*(b
+4))) << 24) + (( uint64_t(*(b+5))) << 16) + (( uint64_t(*(b+6))) <<
8) + ( *(b+7))

Here b is a pointer to an unsigned char.
What if a caller wants chars 2 through 9 from a buffer
and calls the macro as:
uint64_t x = READ64( b+2 );
It works properly. It would, however, fail with arguments such as
b+1<<2.

No, you are mistaken: it does not work for b+2 either. It compiles, but
does not behave as intended. The MSB is read from b[0] and offset by 2.

Actually, it might compile as C++, but one has to fix the cast syntax for it
to compile as C ;-)

Thanks for correcting my blunders!