First off, Jean-François is absolutely right: you cannot assume any particular bit widths for the built-in types, short
, int
, long
, etc. Use the types defined in stdint.h
that have explicit bit widths to ensure that the code is correct and portable.
Otherwise, your code looks pretty good, and this is a reasonable implementation. But…
Is this a good way to implement the macros or is there a better way?
There is indeed a better way: to use inline functions rather than macros. :-)
After an optimizing compiler finishes, the resulting code will be equivalent, but the advantages of functions are numerous: type safety, the ability to use expressions as arguments, no insidious parenthesization bugs, etc.
You will still need some preprocessor magic to avoid generating code when it is not necessary, but this is still much cleaner than implementing the whole header as a mess of macros.
Define a couple of inline helper functions that do the conversion, like so:
inline uint16_t Reverse16(uint16_t value)
{
return (((value & 0x00FF) << 8) |
((value & 0xFF00) >> 8));
}
inline uint32_t Reverse32(uint32_t value)
{
return (((value & 0x000000FF) << 24) |
((value & 0x0000FF00) << 8) |
((value & 0x00FF0000) >> 8) |
((value & 0xFF000000) >> 24));
}
Although it is not strictly necessary, I have kept your explicit masking because I think it increases the clarity of the code, both for human readers and for the compiler. Others may feel that simpler is better. For example, Reverse16
could simply be implemented as return ((value << 8) | (value >> 8));
.
However, I've chosen to format your code slightly differently for nice vertical alignment. I think this makes it easier to read, and easier to audit for correctness at a glance.
On most compilers, there are byte-swapping intrinsics that you could have used to implement the body of these functions. For example, GNU compilers (including GCC and Clang) have __bswap_32
and __bswap_16
macros in the <byteswap.h>
header, and Microsoft's compiler offers the _byteswap_ushort
and _byteswap_ulong
intrinsics in the <intrin.h>
header.
While intrinsics can often result in better code than writing out the C code long-form, all compilers I tested here are exceptionally smart: GCC, Clang, and ICC all recognize the bit-twiddling code used above and compile it to identical object code as if we had used the intrinsic—a single BSWAP
instruction on the x86 architecture! Microsoft's compiler makes this optimization for the 32-bit version, but not for the 16-bit version. However, its output either way is still perfectly reasonable, and if you're seeking portable code, there is no compelling interest in using the intrinsics. For once, you don't need them to get optimal code!
Now that we have those helper functions, we need to define some more inline functions that work like your macros, except that the conditional logic will be encapsulated within the body of the functions, rather than surrounding the macro definitions.
Naturally, since these are functions, not macros, we'll use a different (title-case) naming convention. We don't need SCREAMING_CASE because they're not scary anymore:
inline uint16_t LittleEndian16(uint16_t value)
{
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
return value;
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
return Reverse16(value);
#else
# error unsupported endianness
#endif
}
inline uint16_t BigEndian16(uint16_t value)
{
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
return Reverse16(value);
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
return value;
#else
# error unsupported endianness
#endif
}
inline uint32_t LittleEndian32(uint32_t value)
{
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
return value;
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
return Reverse32(value);
#else
# error unsupported endianness
#endif
}
inline uint32_t BigEndian32(uint32_t value)
{
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
return Reverse32(value);
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
return value;
#else
# error unsupported endianness
#endif
}
The complete code is slightly longer than the macro-based version, but the safety and other benefits of functions more than justify this expanded length. The compiler doesn't care, and it only takes a few minutes longer to write. You will more than make up for it later in the time you don't have to spend debugging macros used in expressions.
You'll use them in basically the same way as the macros. Instead of LITTLE_ENDIAN_SHORT(value)
, you'd call LittleEndian16(value)
. Note that I've also used the explicit bit-width in the function's names, instead of the ambiguous short
and long
type names.
0xFF000000U
(and for good measure, you can addU
to the other bitmasks as well). While most compilers do the right thing, you are technically overflowing a signed integer here. \$\endgroup\$