Copying unsigned char array to uint32_t, and vice versa

Question

I'm trying to iteratively copy an unsigned char array to a uint_32t variable (in 4 byte blocks), perform some operation on the uint_32t variable, and copy it back to the unsigned char array.

Here's my code:

unsigned char byteArray[len]
for (int i=0; i<len; i+=4) {
  uint32_t tmpInt = 0;
  memcpy(&tmpInt, byteArray+(i*4), sizeof(uint32_t));
  // do some operation on tmpInt here
  memcpy((void*)(byteArray+(i*4)), &tmpInt, sizeof(uint32_t));
}

It doesn't work though. What's wrong, and how can I achieve what I want to do?

Can't reproduce an error. I set `byteArray[len] = {1, 2, 3, 4};`, then in the middle I do `tmpInt++` and in the end if I print the chars and I get `2 2 3 4`. — Blaze, Dec 06 '18 at 10:12
How is the data stored in the 8 bit array, e.g. how is the value 2000 stored? — Rishikesh Raje, Dec 06 '18 at 10:13
What exactly doesn't work? (And why 4 and sizeof(uint32_t) in the same line?) — n. m. could be an AI, Dec 06 '18 at 10:13
*It doesn't work though.* **What** "doesn't work"? Does your computer explode? Does it clog your toilet? "Doesn't work" isn't a useful description. — Andrew Henle, Dec 06 '18 at 10:14

D.Go · Answer 1 · 2018-12-06T10:30:23.550

4

The problem is that you are adding 4 to i with each iteration and multiplying by 4. You should be using byteArray + i.

Also, as @WeatherVane pointed out below, your loop would be more consistent with a sizeof():

for (int i = 0; i < len; i += sizeof(uint32_t)).

edited Dec 06 '18 at 10:30

answered Dec 06 '18 at 10:17

D.Go

171
9

1

...and `i+=4` should be `i += sizeof(uint32_t)` to be consistent. – Weather Vane Dec 06 '18 at 10:20

score 2 · Accepted Answer · edited Jan 05 '21 at 09:43

As others pointed out you are doing too much by incrementing i as well as multiplying it by the size of your target.

On top of this

the code shown might run into a buffer overflow issue reading beyond the source array.
the sizeof operator evaluates to size_t not int.
the code repeats defining the size of the target independently several times.

Fixing all, the result might look like this:

  unsigned char byte_array[len];

  typedef uint32_t target_type;
  const size_t s = sizeof (target_type);

  for (size_t i = 0; i < (len/s)*s; i += s) {
    target_type target;
    memcpy(&target, byte_array + i, s);

    // do some operation on target here

    memcpy(byte_array + i, &target, s);
  }

To avoid the typedef just define the target outside of the for-loop:

  unsigned char byte_array[len];

  {
    uint32_t target;
    const size_t s = sizeof target;

    for (size_t i = 0; i < (len/s)*s; i += s) {
      memcpy(&target, byte_array + i, s);

      // do some operation on target here

      memcpy(byte_array + i, &target, s);
    }
  }

An equivalent to

byte_array + i

would be

&byte_array[i]

which might be more intuitively to read.

To avoid the "strange" (len/s)*s one could step away from using an index at all, but use a pointer instead:

for (unsigned char p = byte_array; p < byte_array + len; p += s) {
      memcpy(&target, p, s);

      // do some operation on target here

      memcpy(p, &target, s);
    }

In my opinion this is a more elegant solution.

Thanks, after debugging it for a while more, I came up with a solution that more or less looks like this. — user1118764, Dec 07 '18 at 08:24

Copying unsigned char array to uint32_t, and vice versa

2 Answers2