2

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;
Improve this question
9
  • *data is effectively equivalent to data[0]. Clearly that won't work. Commented May 25, 2016 at 7: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;. Commented May 25, 2016 at 7:15
  • You probably want to write speed=*(uint32_t*)data instead of speed=( uint32_t)*data. Commented May 25, 2016 at 7:16
  • 1
    @GuillaumeGeorge This is dangerous. According endianness of microcontroller, results can be different Commented May 25, 2016 at 7:27
  • 1
    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... Commented May 25, 2016 at 10:53

2 Answers 2

Reset to default
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
Improve this answer
Sign up to request clarification or add additional context in comments.

11 Comments

std::uint32_t is c++, isn't it... uint32_t alone looks more like C to me
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.
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 Does my edit correct errors you identified ?
@Garf365 No. You should never use bit shift signed integers, it is simple as that.
|
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?

Improve this answer

10 Comments

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.
(Though note that if volatile for some reason is important to preserve during the operation, the casts should use a volatile qualified type too)
For my personal culture, why addition is bad in this case in comparison of bit-or ?
@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.
Ok, thanks :) I avoid to use "tricks" to optimize in place of compiler and I prefer explicit code. I will remember this
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.