0

I'm working on an application that needs to convert any type of the variable from big to little-endian.

My system works with different variable types (16, 32, and 64 bits wide), and I need to be able to change the endianness with a single function. I wrote a function that manages to swap bytes in any variable however, I'm not happy with it. It works, but it requires dereferencing void pointers, which are prone to error with the double star...

  1. Is there any better way to approach the problem?
  2. Is there any way to avoid void pointers as return value? I was thinking about switch-case loop (eg. case 4 bytes -> return int32) however, I don't know how to write a function prototype for a function that returns different values.

My function:

void* swapBytes(void* number, int bytes_num){
    void* swapped;
    unsigned __int8* single_byte_ptr;
    swapped = malloc(bytes_num * sizeof(__int8));

    for (int i = 0; i<bytes_num; i++){
        single_byte_ptr =((unsigned __int8*)number)+i; //get current byte
        *( (__int8*)(swapped)+((bytes_num-1)-i)) = (unsigned __int8)*single_byte_ptr; //save the byte in new position
    }

    return swapped;
}

the way I call this function

__int64 big_number = 35169804487071;
big_number = *(__int64*)(swapBytes(&big_number, 8));
peter1
  • 59
  • 3
  • *My system works with different variable types...* - and how your system is getting the data in the "wrong" format? – Eugene Sh. Dec 15 '21 at 14:53
  • i am working on MPC5748G - a microcontroller from NXP - and I am reading the data from CAN, which is connected to a bus with many other devices. These devices use different data formats. – peter1 Dec 15 '21 at 14:56
  • 2
    Your code leaks memory also. – Fredrik Dec 15 '21 at 14:58
  • 2
    Then the most portable way would be to take the input as a byte stream and "manually" reinterpret is (like in `word = byte0 + (byte1 << 8) + (byte2 << 16) +...` – Eugene Sh. Dec 15 '21 at 14:58
  • 1
    Lots of suggestions in answers to [this question](https://stackoverflow.com/questions/2182002/convert-big-endian-to-little-endian-in-c-without-using-provided-func/2637138). – 500 - Internal Server Error Dec 15 '21 at 14:58
  • your solution is breaking strict aliasing rule. The effective type of the value returned from the function is `unsigned __int8` which is **not** compatible with `__int64`. Thus UB is invoked – tstanisl Dec 15 '21 at 15:04
  • Why are you using heap allocation on a safety/ASIL MCU? – Lundin Dec 15 '21 at 15:20
  • good point! ASIL has not been specified as a requirement for this project (I am a student in training). – peter1 Dec 15 '21 at 15:24
  • Don't use heap allocation in embedded systems in general and definitely don't use it in safety-related systems specifically. – Lundin Dec 15 '21 at 15:25
  • " I am reading the data from CAN, which is connected to a bus with many other devices. These devices use different data formats." It's the responsibility of every device to convert to the _network endianess_. Which in case of CAN is often big endian, same as MPC57xx. Though some application layer protocols like CANopen do use little endian for the payload, so it depends on which CAN protocol that is used. – Lundin Dec 15 '21 at 15:30

2 Answers2

3

One problem you have is that you're leaking memory. You return a pointer to malloc'ed memory, but you're not saving the pointer when you return.

Given that you're assigning the result back to the same value, you're better off updating the existing variable, swapping the current byte with a byte on the "opposite" side.

You also don't need to use a void * anyplace other than the parameter type. Inside of the function, just use a pointer to an unsigned char or unsigned __int8 to work through the bytes.

void swapBytes(void* number, int bytes_num)
{
    unsigned __int8* ptr = number;
    for (int i = 0; i<bytes_num/2; i++) {
        unsigned __int8 tmp = ptr[i];
        ptr[i] = ptr[bytes_num-1-i];
        ptr[bytes_num-1-i] = tmp;
    }
}

Then call it like this:

swapBytes(&big_number, sizeof(big_number));
dbush
  • 205,898
  • 23
  • 218
  • 273
1

Your solution is very over-engineered and also entirely unsuitable for embedded systems such as MPC57xx.

Any integer type can get safely iterated across using a pointer to character. Assuming uint8_t* is a character type for your compiler, it's as simple as this:

void little_to_big16 (uint8_t       big    [sizeof(uint16_t)], 
                      const uint8_t little [sizeof(uint16_t)])
{
  big[0] = little[1];
  big[1] = little[0];
}

Then write big_to_little16, big_to_little32 etc etc as needed. Such functions can and should probably be inlined too.

Example of use:

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

void little_to_big16 (uint8_t       big    [sizeof(uint16_t)], 
                      const uint8_t little [sizeof(uint16_t)])
{
    big[0] = little[1];
    big[1] = little[0];
}

int main (void)
{
  uint16_t little = 0xAABB;
  uint16_t big;
  little_to_big16((uint8_t*)&big, (uint8_t*)&little);
  printf("%"PRIx16, big);
}

Output on x86 little endian:

bbaa
Lundin
  • 195,001
  • 40
  • 254
  • 396