Ygthkb

I have a 16 bits unsigned variable. I need to split it in 8 bits chunks.

Is doing the following enough:

chunk_lsb = (uint8)variable;

chunk_msb = (uint8)(variable >> 8);

Or should I use a mask:

chunk_lsb = (uint8)(variable & 0xFFu);

chunk_msb = (uint8)((variable >> 8) & 0xFFu);

I know that both approaches work, I'm just looking for the best way to do it, if there is one. Maybe there's none and just use the cast to reduce calculations is the best way? What do you guys think?

Since uint8 is unsigned, you don't have to do the masking:

6.3.1.3 Signed and unsigned integers

1. When a value with integer type is converted to another integer type other than _ Bool , if the value
   can be represented by the new type, it is unchanged.


2. Otherwise, if the new type is unsigned, the value is converted by repeatedly adding or subtracting
   one more than the maximum value that can be represented in the new type until the value is in the
   range of the new type. ⁶⁰⁾
   


3. Otherwise, the new type is signed and the value cannot be represented in it; either the result is
   implementation-defined or an implementation-defined signal is raised.

However, most likely both will result in the same compiler output. I usually add the mask because it makes clear what code is supposed to do, and makes the cast unnecessary.

It isn't clear what type variable is. Without that specified, we can only speculate.

But in general, you should avoid bit shifting on signed integer types, as that leads to various forms of poorly-defined behavior. This in turn means that you have to be careful with small integer types too, because they get promoted to signed int. See Implicit type promotion rules.

The specific case of (uint8)((variable >> 8) & 0xFFu); is safe if variable is unsigned. Otherwise it is unsafe, since right-shifting a negative value leads to implementation-defined behavior (arithmetic or logical shift).

variable << 8 will invoke undefined behavior on 16 bit systems in case variable is a small integer type, or an int16_t.

The safest, most portable way no matter left/right shift is therefore this:

chunk_lsb = variable;

chunk_msb = ((unsigned int)variable >> 8);

Though you might want to be overly explicit in order to silence all compiler warnings:

chunk_lsb = (uint8_t) (variable & 0xFFu);

chunk_msb = (uint8_t) ( (unsigned int)variable>>8 & 0xFFu );

If chunk_lsb is an 8 bit object, use cast or mask (not both). Useful in quieting pedantic warnings about range reduction. I prefer the mask - unless the compiler is picky.

uint8_t chunk_lsb = (uint8_t) variable;

// or 

uint8_t chunk_lsb = variable & 0xFFu;

Otherwise use the mask.

unsigned chunk_lsb = variable & 0xFFu;

Maybe there's none and just use the cast to reduce calculations is the best way?

General speaking, the asm code will be the same, so in terms of speed, it does not matter which one you use:

• masking: https://godbolt.org/z/Olrw3x
  


• casting: https://godbolt.org/z/EEdvQZ
  

What do you guys think?

IMO, the first one is clearer, in terms of readability, but I cannot figure out a coding standard or guideline which supports my preference. Anyway, I would use a const variable if your preference is the second one, to remove magic numbers and make clearer that the purpose is masking (assuming that you have chosen a correct name for the const variable).