CODEWITHSUNDEEP
cendianness
Ohunter
Ohunter

Reputation: 363

little endian to uint with undetermined numbers of bytes

I was trying to write a function that took in N bytes of little endian hex and made it into an unsigned int.

unsigned int endian_to_uint(char* buf, int num_bytes)
{
    if (num_bytes == 0)
            return (unsigned int) buf[0];

    return (((unsigned int) buf[num_bytes -1]) << num_bytes * 8) | endian_to_uint(buf, num_bytes - 1);
}

however, the value returned is approx ~256 times larger than the expected value. Why is that?

If I needed to do use it for a 4 byte buffer, normally you'd do:

unsigned int endian_to_uint32(char* buf)
{
    return (((unsigned int) buf[3]) <<   24)
         | (((unsigned int) buf[2]) <<   16)
         | (((unsigned int) buf[1]) << 8)
         | (((unsigned int) buf[0]));
}

which should be reproduced by the recursive function I wrote, or is there some arithmetic error that I haven't caught?

Upvotes: 0

Views: 232

Answers (2)

Lundin
Lundin

Reputation: 213989

There is absolutely no reason to use recursion here: bit shifts is among the fastest operations available, recursion is among the slowest. In addition, recursion is dangerous, hard to read and gives nasty peak stack consumption. It should be avoided in general.

In addition, your function is not a general one, since you return unsigned int, making the function inferior to the shift version in every single way.

To actually write a generic-size little endian conversion function, you can do like this:

void little_endian (size_t bytes, uint8_t dest[bytes], const uint8_t src[bytes])
{
  for(size_t i=0; i<bytes; i++)
  {
    dest[i] = src[bytes-i-1];
  }
}

Working example:

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


void little_endian (size_t bytes, uint8_t dest[bytes], const uint8_t src[bytes])
{
  for(size_t i=0; i<bytes; i++)
  {
    dest[i] = src[bytes-i-1];
  }
}

int main (void)
{
  uint8_t data [] = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88};
  uint32_t u32;
  uint64_t u64;

  little_endian(4, (uint8_t*)&u32, data);
  little_endian(8, (uint8_t*)&u64, data);

  printf("%"PRIx32"\n", u32);
  printf("%"PRIx64"\n", u64);

  return 0;
}

Upvotes: 0

Santosh A
Santosh A

Reputation: 5351

The below code snippet would work.

unsigned int endian_to_uint(unsigned char* buf, int num_bytes)
{
    if (num_bytes == 0)
        return (unsigned int) buf[0];

    return (((unsigned int) buf[num_bytes -1]) << (num_bytes -1) * 8) | endian_to_uint(buf, num_bytes - 1);
}

Change 1:
Modified the function argument data type from char* to unsigned char *
Reason:
For a given buf[] = {0x12, 0x34, 0xab, 0xcd};
When you are trying to read buf[3] i.e here buf[num_bytes -1] will give you 0xffffffcd instead of just 0xcd because of sign extension. For more info on sign extension refer Sign Extension

Change 2:
Use num_bytes-1 when calculating the shift position value. This was a logical error in calculation of the number of bits to be shifted.

Upvotes: 0

Related Questions