Issue with a function reversing byte order

Question

I'm working on CAN Frames, in order to decode their payload, I have to reverse their order. The payload is a uint8_t payload[8] (8 bytes in the payload) read in the can frame with an external function that works properly. I have this function :

static inline uint64_t reverse_byte_order(uint64_t x)
{
    x = (x & 0x00000000FFFFFFFF) << 32 | (x & 0xFFFFFFFF00000000) >> 32;
    x = (x & 0x0000FFFF0000FFFF) << 16 | (x & 0xFFFF0000FFFF0000) >> 16;
    x = (x & 0x00FF00FF00FF00FF) << 8  | (x & 0xFF00FF00FF00FF00) >> 8;
    return x;
}

So basically i just call reverse_byte_order(*(uint64_t*)payload), but after some tests we couldn't get the right messages after decoding, and realized the problem came from this function.

One test that doesn't work is putting payload[0]=0x40 and everything else to 0x00. The output is 0, whereas it should be 0x 40 00 00 00 00 00 00 00.

Any helped would be appreciated.

Edit : Here is a MCVE you can copy paste into your IDE if you want to do some testing :

#include <stdint.h>

static inline uint64_t reverse_byte_order(uint64_t x)
{
    printf("\nInitial x : %016x", x);
    x = (x & 0x00000000FFFFFFFF) << 32 | (x & 0xFFFFFFFF00000000) >> 32;
    printf("\nFirst x : %016x", x); //in our example here it doesn't work anymore as x is 0
    x = (x & 0x0000FFFF0000FFFF) << 16 | (x & 0xFFFF0000FFFF0000) >> 16;
    printf("\nSecond x : %016x", x);
    x = (x & 0x00FF00FF00FF00FF) << 8  | (x & 0xFF00FF00FF00FF00) >> 8;
    printf("\nFinal x : %016x", x);
    return x;
}

int main(int argc, char const *argv[])
{
    uint8_t data[8];
    data[0]=0x11;
    data[1]=0x00;
    data[2]=0x00;
    data[3]=0x00;
    data[4]=0x00;
    data[5]=0x00;
    data[6]=0x00;
    data[7]=0x00;


    uint64_t indata = reverse_byte_order(*(uint64_t*)data);
}

Answer 1

If you use memcpy() to copy your array of uint8_t values to a uint64_t value, your byte swap function works without any possible alignment issues:

#include <stdio.h>
#include <string.h>
#include <inttypes.h>

static inline uint64_t reverse_byte_order(uint64_t x) {
  x = (x & 0x00000000FFFFFFFF) << 32 | (x & 0xFFFFFFFF00000000) >> 32;
  x = (x & 0x0000FFFF0000FFFF) << 16 | (x & 0xFFFF0000FFFF0000) >> 16;
  x = (x & 0x00FF00FF00FF00FF) << 8 | (x & 0xFF00FF00FF00FF00) >> 8;
  return x;
}

int main(void) {
  uint8_t data[8];

  _Static_assert(sizeof(data) == sizeof(uint64_t), "You have a weird CPU");

  data[0] = 0x11;
  data[1] = 0x00;
  data[2] = 0x00;
  data[3] = 0x00;
  data[4] = 0x00;
  data[5] = 0x00;
  data[6] = 0x00;
  data[7] = 0x00;

  uint64_t num;
  memcpy(&num, data, sizeof num);
  uint64_t indata = reverse_byte_order(num);

  printf("%#" PRIx64 " reversed is %#" PRIx64 "\n", num, indata);

  return 0;
}

Compiling and running this version prints 0x11 reversed is 0x1100000000000000 on a little-endian system.

Note using the <inttypes.h> macros to get the right format specifiers for printf(). If you're using %x for printing uint64_t values on a system where unsigned int is 32 bits, that's got a lot to do with you seeing the wrong numbers. Your compiler should be warning you about such things. If not, turn up the options (-Wall -Wextra is a good set for gcc and clang).

Answer 2

The failure in the test code shown can be entirely explained by the fact that uint64_t values are printed using an x conversion specifier, which is for an unsigned. The resulting behavior is not defined by the C standard, but converting just the low 32 bits of the uint64_t, and thus printing zero for a value with zeros in the low 32 bits, is a natural result.

The solution for this is to use a correct conversion specifier:

• Include the header <inttypes.h>.
• In place of x inside the quoted string, end the quoted string and use PRIx64. This is a macro that expands to a quoted string with the correct conversion specifier for a uint64_t to be converted to hexadecimal, and the quoted string will be concatenated with neighboring strings. For example, change "\nInitial x : %016x" to "\nInitial x : %016" PRIx64.

Additional problems in the code shown include:

• The behavior of converting a pointer to uint64_t * is not defined by the C standard if the alignment is not correct for a uint64_t *.
• Using memory defined as an array of uint8_t as if it were a uint64_t violates the aliasing rules in C 2018 6.5 7, and the resulting behavior is not defined by the C standard.
• <stdio.h> is not included.
• Lines should be printed with a trailing \n rather than a preceding \n. This is because \n causes a line-buffered stream to be flushed: When there is a trailing \n, the output is printed immediately, whereas, with only preceding \n, the output may be held in a buffer indefinitely.

A defined way to reinterpret memory as another type is to copy the bytes:

uint64_t x;
memcpy(&x, data, sizeof x);

After the above, x can be passed to reverse_byte_order as a uint64_t. (Since memcpy is declared in <string.h>, that header should be included.)

Also be sure to enable warnings in your compiler and to pay attention to them. Most compilers would have warned about the mismatch between the conversion specifier in the format string and the argument type, as well as the missing <stdio.h> header.