Portable conversion of data endianness using the Linux kernel's API

2
votes

How can I improve the following code, that is, make it more robust with respect to type safety and endianness using the functions and macros in the Linux kernel's API? For instance, in the following example src_data is an array of two 16-bit signed integers (typically stored in little endian order) and is to be sent out via UART in big endian byte order.

s16 src_data[2] = {...}; /* note: this is signed data! */
u8 tx_data[4];

u8* src_data_u8 = (u8*)src_data;

tx_data[0] = src_data_u8[1];
tx_data[1] = src_data_u8[0];
tx_data[2] = src_data_u8[3];
tx_data[3] = src_data_u8[2];

I think the functions cpu_to_be16 and cpu_to_be16p should play a role in doing this conversion. Although I'm not sure how I can use them in a way that is safe and robust to endianness.

4
linux-kernelendiannessmemory-alignment
What exactly is the issue you want to solve, that the comments in the header file you link do not help you with? Make your question more specific. - Michael Foukarakis
If you write code like this, careful with the de-serialization of incoming protocols. You cant go from lets say u8 rx_data[4] to s16 [2] by using pointer conversions - that would be a strict aliasing violation. You get away with it here because you go from "any type" to character type, which is a special allowed case. - Lundin
@MichaelFoukarakis The functions/macros in the header file don't seem safe since they only accept unsigned arguments... Furthermore, it is not clear to me how I can safely store the intermediate result before putting the converted data into my u8 tx_data array... - mallwright
Thanks for the input @0andriy, the protocol is fixed as big endian, and the target machine (an AVR) is little endian. As I understand, in practice, pretty much all Linux based machines are also little endian, but nonetheless, I would like to know how to achieve this conversion in a clean, platform independent way. - mallwright
@0andriy It is fairly clear that you have never written a single line of hardware-related programming in your life. Sorry, but TCP/IP sockets is not hardware, it is layer 3 and 4 or so. I write drivers for embedded systems everyday and have done so for the past 15 years, but thanks for sharing your wisdom about something you have never worked with. - Lundin

4 Answers

0
votes

As I understand, two 16-bit words, to be sent, one after another, after converting each into bigendian format. I think following should be ok.

s16 src_data[2] = {...}; /* note: this is signed data! */
s16 tx_data[2];
tx_data[0] = cpu_tp_be16(src_data_u8[0]);
tx_data[1] = cpu_to_be16(src_data_u8[1]);

s16 src_data[2] = {...}; /* note: this is signed data! */
s16 tx_data[2];

tx_data[0] = cpu_tp_be16(src_data_u8[0]);
tx_data[1] = cpu_to_be16(src_data_u8[1]);
-1
votes

Your issue with safety seems to be that the htons(x) function/macro expects an unsigned integer, but you possess a signed one. Not an issue:

union {
    int16_t signed_repr;
    uint16_t unsigned_repr;
} data;

data.signed_repr = ...;

u16 unsigned_big_endian_data = htons(data.unsigned_repr);

memcpy(tx_data, &unsigned_big_endian_data,
       min(sizeof tx_data, sizeof unsigned_big_endian_data));

PS. Type-punning via unions is perfectly well-defined.

-1
votes

I believe the following is one of the best answers to my question. I have used the links provided by @0andriy to existing examples in the kernel source code.

Converting a signed 16-bit value for transmitting

s16 src = -5;
u8 dst[2];
__be16 tx_buf;
*(__be16*)dst = cpu_to_be16(src);

Converting multiple signed 16-bit values for transmitting

s16 src[2] = {-5,-2};
u8 dst[4];
s16* psrc = src;
u8* pdst = dst;
int len = sizeof(src);

for ( ; len > 1; len -= 2) {
    *(__be16 *)pdst = cpu_to_be16p(psrc++);
    pdst += 2;
}

A quick disclaimer, I still need to check if this code is correct / compiles.

Overall, I'm a bit unsatisfied with the solution for copying and converting the endianness of multiple values since it is prone to typos and could easily be implemented into a macro.

-2
votes

If the Linux machine will always be little endian, and the protocol will always be big endian, then the code works fine and you don't need to change anything.

If you for some reason need to make the Linux code endian-independent, then you'd use:

tx_data[0] = ((unsigned int)src_data[0] >> 8) & 0xFF;
tx_data[1] = ((unsigned int)src_data[0] >> 0) & 0xFF;
tx_data[2] = ((unsigned int)src_data[1] >> 8) & 0xFF;
tx_data[3] = ((unsigned int)src_data[1] >> 0) & 0xFF;

Where the cast is there to ensure that the right shifts are not carried out on a signed type, which would invoke non-portable implementation defined behavior.

The advantage of bit shifts compared to any other version is that they work on an abstraction level above the hardware and endianess, letting the specific compiler generate the instructions for the underlying memory access. Code such as u16 >> 8 always means "give me the least significant byte" regardless of where that byte is stored in memory.