Converting bytes array to integer

Question

I have a 4-byte array (data) of type uint8_t, which represents a speed data integer. I'm trying to cast this array to uint32_t integer (speed), multiply this speed by 10 and then restore it back to the 4-byte array (data). The data format is clear in the code below. I always get the error:

"assignment to expression with array type"

The code:

volatile uint8_t data[4] = {0x00 , 0x00, 0x00, 0x00};
volatile uint32_t speed;
speed=( uint32_t)*data;
speed=speed*10;
data=(uint8_t*)speed;

`*data` is effectively equivalent to `data[0]`. Clearly that won't work. — Tom Karzes, May 25 '16 at 07:15
`volatile uint8_t data[4]` is a static array, when you use the name of this array in an expression it will decay into a pointer of its type which is a `prvalue` which means you can't assign to it like `data=(uint8_t*)speed;`. — Biruk Abebe, May 25 '16 at 07:15
You probably want to write speed=*(uint32_t*)data instead of speed=( uint32_t)*data. — Guillaume George, May 25 '16 at 07:16
@GuillaumeGeorge This is dangerous. According endianness of microcontroller, results can be different — Garf365, May 25 '16 at 07:27
You want `speed = *(uint32_t*)data;`. Keep in mind the result of this depends on the CPU's endianness, though. And after you multiply `speed` by 10, you can't really use the `=` operator to copy the bytes back into the array; instead, do this: `memcpy(data, (uint8_t*)data, 4);`. — user4520, May 25 '16 at 07:41
@shamsalsham no, `speed=( uint32_t)data;` result to an assignment of pointer to integer. You will get at least a warning, and certainly an unexpected behavior — Garf365, May 25 '16 at 07:42
Well, it would actually be `(uint32_t *) data`. Except that will quite likely fail since it ignores alignment. If you just want to alias the array to a `uint32_t`, then put it in a union. — Tom Karzes, May 25 '16 at 07:46
Apparently, getting this right is hard even for intermediately experienced programmers. Just ignore all advise given in the above comments, most of it is incorrect or leads to non-portable code... — Lundin, May 25 '16 at 10:53

score 6 · Answer 1 · edited May 23 '17 at 12:31

6

To be safe according endianess, portable and secure, you should recreate your data:

speed = ((uint32_t)data[0]) << 24 
      | ((uint32_t)data[1]) << 16 
      | ((uint32_t)data[2]) << 8 
      | ((uint32_t)data[3]);

or

speed = ((uint32_t)data[3]) << 24 
      | ((uint32_t)data[2]) << 16 
      | ((uint32_t)data[1]) << 8 
      | ((uint32_t)data[0]);

Choose solution according position of most significant byte

You get an "assignment to expression with array type" error because you can't assign directly an array: data=(uint8_t*)speed; is totally forbidden in C, you definitively can't have an array for lvalue. You have to do inverse operation:

data[0] = (uint8_t)((speed >> 24) & 0x00FF);
data[1] = (uint8_t)((speed >> 16) & 0x00FF);
data[2] = (uint8_t)((speed >> 8) & 0x00FF);
data[3] = (uint8_t)(speed & 0x00FF);

or, according position of most significant byte:

data[3] = (uint8_t)((speed >> 24) & 0x00FF);
data[2] = (uint8_t)((speed >> 16) & 0x00FF);
data[1] = (uint8_t)((speed >> 8) & 0x00FF);
data[0] = (uint8_t)(speed & 0x00FF);

EDIT

Don't use cast or memcpy as mention in commentaries and original answer: in addition of non portability issues, you will have security issues, according alignment restrictions and aliasing rules on some platform, compiler can generate incorrect code - thanks to user694733 | see here - thanks to Lundin

    speed = *((uint32_t *)data); // DANGEROUS NEVER USE IT
    *((uint32_t *)data) = speed; // DANGEROUS NEVER USE IT

edited May 23 '17 at 12:31

Community

1
1

answered May 25 '16 at 07:28

Garf365

3,619
5
29
41

std::uint32_t is c++, isn't it... uint32_t alone looks more like C to me – Stian Skjelstad May 25 '16 at 08:09
`speed = *((uint32_t *)data);` fails on platforms with sufficient alignment restrictions and also breaks strict aliasing rules (which means compiler can generate incorrect code). Never use it, use `memcpy` instead. – user694733 May 25 '16 at 09:08
Your answer has lots of poorly-defined behavior because you bit shift on signed integer types. This `speed = data[0] << 24 + data[1] << 16 + data[2] << 8 + data[3];` is dangerous code, implicit promotions to `int` may cause havoc. – Lundin May 25 '16 at 09:23
@Lundin Does my edit correct errors you identified ? – Garf365 May 25 '16 at 09:29
@Garf365 No. You should never use bit shift signed integers, it is simple as that. – Lundin May 25 '16 at 09:31
Looks ok now, but I really don't see why you must mention the unsafe or non-portable methods. There is never a reason to use any of them. – Lundin May 25 '16 at 09:43
@Lundin I mention them because there are the starting point of question – Garf365 May 25 '16 at 09:46
The OP actually didn't mention them, it was just various confused people leaving very bad advise in comments to him. – Lundin May 25 '16 at 09:59
@Lundin edit according your comments, really thanks for every informations you give me! – Garf365 May 25 '16 at 10:12
@Garf365 Your answer is full and right, but I have a little doubts: volatile suggests it has something to do with interface/data source which has defined endianess. With my experience in such a situation it is better to use functions from (be32toh or le32toh). Then it is gains more portability. – Marcin Kajzler May 25 '16 at 12:08
I don't know `` but it seams to be specific form Unix world. So I doubt it exists everywhere. – Garf365 May 25 '16 at 12:23

Lundin · Accepted Answer · 2016-05-25T12:29:29.060

5

Your code doesn't work because during data=(uint8_t*)speed; you don't get a "lvalue" for data, you just get an array type which can't be used in assignment or any form of arithmetic. Similarly, speed=( uint32_t)*data; is a bug because that only gives you the first item in the array.

The only correct way you should do this:

volatile uint8_t data[4] = {0x00 , 0x00, 0x00, 0x00};
volatile uint32_t speed;

speed = (uint32_t)data[0] << 24 |
        (uint32_t)data[1] << 16 |
        (uint32_t)data[2] <<  8 |
        (uint32_t)data[3] <<  0;

speed=speed*10;

data[0] = (uint8_t) ((speed >> 24) & 0xFFu);
data[1] = (uint8_t) ((speed >> 16) & 0xFFu);
data[2] = (uint8_t) ((speed >>  8) & 0xFFu);
data[3] = (uint8_t) ((speed >>  0) & 0xFFu);

This is 100% portable and well-defined code. No implicit promotions take place. This code does not rely on endianess or other implementation-defined behavior. Why write code that does, when you can write code that doesn't?

edited May 25 '16 at 12:29

answered May 25 '16 at 09:31

Lundin

195,001
40
254
396

Please note that other present answers suggest dangerous methods that lead to bugs or that aren't portable. There's no reason why you should ever do anything except the above method. Forget about structs, unions, memcpy, addition, pointer conversions and other nonsense. – Lundin May 25 '16 at 09:32
(Though note that if `volatile` for some reason is important to preserve during the operation, the casts should use a volatile qualified type too) – Lundin May 25 '16 at 09:39
For my personal culture, why addition is bad in this case in comparison of bit-or ? – Garf365 May 25 '16 at 09:44
@Garf365 Because it doesn't express the intent. Bitwise OR is self-documenting code. It is true that addition and bitwise OR are equivalent, the compiler should hopefully not generate slower code because of addition. But addition could be confused with for example adding the value of the individual bytes and storing the result in a uint32_t. You shouldn't use it for the same reason as you shouldn't use `speed / 256` instead of `speed >> 8`. – Lundin May 25 '16 at 09:57
Ok, thanks :) I avoid to use "tricks" to optimize in place of compiler and I prefer explicit code. I will remember this – Garf365 May 25 '16 at 10:03
@Lundin *"...you just get an array type which can't be used in arithmetic"*,is assignment an arithmetic operation or is that a typo? – Biruk Abebe May 25 '16 at 11:59
@bkVnet Well, there's really no such thing as arithmetic operations in C. The standard speaks of _arithmetic operands_ or arithmetic types, which are all integer, bool and float types. Though I guess that sentence doesn't make sense, since the assignment operators work on other types than the arithmetic types. I'll edit. – Lundin May 25 '16 at 12:29
@Ludin Thank you for the clarification, but i am still a little confused. for example,`*(data+1)` is performing a pointer arithmetic, from this my understanding was you can perform arithmetic using the name of an array(which in this context decays to a pointer value and is an arithmetic operand). May be i have the wrong understanding about this in C. – Biruk Abebe May 25 '16 at 12:55
@bkVnet Briefly: the rule of "array decay" (6.3.2.1/3) specifically says that: "an expression that has type ‘‘array of type’’ is converted to an expression with type ‘‘pointer to type’’ that points to the initial element of the array object and is not an lvalue." The key here is that this isn't a "lvalue" so it cannot be used by the assignment operator. The result of the unary `*` is however a lvalue. Anyway, please ask a separate question about this, since it is not related to the topic in this post. – Lundin May 25 '16 at 13:52
@Lundin Sorry if i took your time but i didn't think this was a separate question. I was just asking about the statement in your answer that says the array name can't be used in *"any form of arithmetic"*. Thanks anyway. – Biruk Abebe May 25 '16 at 13:57

Converting bytes array to integer

2 Answers2

Linked