12
\$\begingroup\$

I have written a simple C header for converting the endianness of short integers and long integers. It uses the GCC macro __BYTE_ORDER__ to check the system's byte order and define the macros based on that.

The header creates the macros LITTLE_ENDIAN_SHORT(n), LITTLE_ENDIAN_LONG(n), BIG_ENDIAN_SHORT(n), BIG_ENDIAN_LONG(n) which convert the value n from host endianness to the endianness specified.

Here is the source for endian.h:

#ifndef ENDIAN_H
#define ENDIAN_H

#define REVERSE_SHORT(n) ((unsigned short) (((n & 0xFF) << 8) | \
                                            ((n & 0xFF00) >> 8)))
#define REVERSE_LONG(n) ((unsigned long) (((n & 0xFF) << 24) | \
                                          ((n & 0xFF00) << 8) | \
                                          ((n & 0xFF0000) >> 8) | \
                                          ((n & 0xFF000000) >> 24)))

#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
#    define LITTLE_ENDIAN_SHORT(n) (n)
#    define LITTLE_ENDIAN_LONG(n) (n)
#    define BIG_ENDIAN_SHORT(n) REVERSE_SHORT(n)
#    define BIG_ENDIAN_LONG(n) REVERSE_LONG(n)
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
#    define LITTLE_ENDIAN_SHORT(n) REVERSE_SHORT(n)
#    define LITTLE_ENDIAN_LONG(n) REVERSE_LONG(n)
#    define BIG_ENDIAN_SHORT(n) (n)
#    define BIG_ENDIAN_LONG(n) (n)
#else
#    error unsupported endianness
#endif

#endif

Is this a good way to implement the macros or is there a better way?

\$\endgroup\$
  • 1
    \$\begingroup\$ You may want to take a look at codereview.stackexchange.com/a/149751/4203 \$\endgroup\$ – forsvarir Dec 28 '16 at 14:29
  • 3
    \$\begingroup\$ A better way to implement the macros would be to replace them with inline functions. :-) \$\endgroup\$ – Cody Gray Dec 28 '16 at 18:24
  • \$\begingroup\$ It should be 0xFF000000U (and for good measure, you can add U to the other bitmasks as well). While most compilers do the right thing, you are technically overflowing a signed integer here. \$\endgroup\$ – Simon Richter Dec 29 '16 at 3:16
  • 1
    \$\begingroup\$ Is there some reason why you want to roll your own endian-conversion code rather than using the standard htonl()/ntohl() and htons()/ntohs() functions? The latter are much harder to mess up and easier for other programmers to read/understand/trust... \$\endgroup\$ – Jeremy Friesner Dec 29 '16 at 5:29
18
\$\begingroup\$

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 & 000xFF) << 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.

\$\endgroup\$
5
\$\begingroup\$

First problem: don't rely on short and long sizes being respectively 16 and 32 bit.

Use stdint.h include which defines uint16_t and uint32_t

Second problem: protect your term with parentheses because you could pass some expression to the macros.

For instance REVERSE_SHORT(4000<<3) would yield 0 because of operator precedence. What you cannot avoid is the multiple computation of n expression (maybe the optimizer can help but that's not guaranteed)

To sum it up I would write them as:

#include <stdint.h>
#define REVERSE_UINT16(n) ((uint16_t) ((((n) & 0xFF) << 8) | \
                                            (((n) & 0xFF00) >> 8)))
#define REVERSE_UINT32(n) ((uint32_t) ((((n) & 0xFF) << 24) | \
                                          (((n) & 0xFF00) << 8) | \
                                          (((n) & 0xFF0000) >> 8) | \
                                          (((n) & 0xFF000000) >> 24)))

Which doesn't mean it's safe to side-effect expressions like n++ or result from function calls, as someone noted in comments. Only the fact that it's written in uppercase could remind users to avoid side-effect expressions.

\$\endgroup\$
  • 1
    \$\begingroup\$ Protecting the parameter with parentheses is not enough! It will do with simple arithmetic expressions like REVERSE_UINT16(x+1) but won't suffice for expressions with side effects, like REVERSE_UINT16(x++) or REVERSE_UINT16(printf("%d",x)). The only way of 'protection' is converting the macro into an inline function, as Cody Gray describes in this answer. \$\endgroup\$ – CiaPan Dec 28 '16 at 20:11
  • \$\begingroup\$ so true! editing. Macros tend to do that, hence the UPPERCASE ;) \$\endgroup\$ – Jean-François Fabre Dec 28 '16 at 20:49
  • 2
    \$\begingroup\$ This answer relies on int/unsigned being 32-bit or wider. Should code pass a int, unsigned short, char etc., these macros incur undefined a behavior due to shift into the sign bit. With 32-bit int, (n) & 0xFF) << 24 is UB. With 16 bit unsigned/int, ((n) & 0xFF) << 24) is UB. Better to use ((((uint32_t)(n)) & 0xFF) << 24) etc. \$\endgroup\$ – chux Dec 29 '16 at 4:49
3
\$\begingroup\$

I would suggest that, rather than thinking in terms of swapping endianness, it would be wiser to focus instead on writing functions to read or write 16- or 32-bit quantities with a given endianness from a given address (probably passed as a void*). In most contexts where it would be necessary to swap endianness, the data may have uncertain alignment; while some processors will handle unaligned memory accesses just fine, that's not a universal trait.

If you design functions like fetchu32le, which fetches a 32-bit little-endian value stored as 8 bits per char, code using such a function can be portable to machines which have stricter alignment requirements than the present hardware, even if things aren't always aligned, and will even be portable to platforms with different sizes of char provided that the data will be stored using an octet per char.

\$\endgroup\$
  • \$\begingroup\$ Agreed, this would make for a much cleaner overall design. But I'm not sure how passing a void* would compensate for the misalignment problems. If the data was misaligned on the recipient architecture, the moment you tried to read from it, you'd take an exception (or pay a very steep speed penalty). \$\endgroup\$ – Cody Gray Dec 29 '16 at 11:53
  • \$\begingroup\$ @CodyGray: In the absence of a defined macro indicating that the system supports word accesses with arbitrary alignment, the function could use a sequence of byte operations and shifts (which would, of course, be tolerant of any alignment). Alternatively, on some architectures, the best approach may be to call a function which selects one of four courses of action based upon the low bits of the address [a machine language routine for ARM7-TDMI would take 15 cycles plus call/return using byte loads; case-selecting logic would take 8-11 cycles depending upon alignment]. \$\endgroup\$ – supercat Dec 29 '16 at 15:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.