4

I have a typedef struct named Character.

typedef struct {
    unsigned int a : 1;
    unsigned int b : 1;
    unsigned int c : 1;
    unsigned int d : 1;
    unsigned int o : 1;
    unsigned int p : 1;
    unsigned int q : 1;
    unsigned int x : 1;
} Character;

static Character tempChar;

void writeVar(const uint8_t *pData)
{
    tempChar.a = pData[0] >> 5;
    ...
}

When I try to assign an uin8_t variable (with value 0 or 1) to one of these bitfields, I got a violation to MISRA rule 10.6 which states that:

The value of a composite expression shall not be assigned to an object with wider essential type

Is there a way to assign a bit-field to uint8_t without violating MISRA C?

Salahuddin
  • 1,617
  • 3
  • 23
  • 37

4 Answers4

4

Both operands in the expression pData[0] >> 5 will, if needed, be promoted to int (it will happen for pData[0]).

And the result of the expression is an int.

Both the promotion and the conversion from int to unsigned int, while perfectly valid and fine in normal cases, is enough for the very strict MISRA to complain.

Simple solution (as shown in comments) is to explicitly convert pData[0] to unsigned int using casting.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • Oh, simpler. Why did I think of `union` in the first place! :( – WedaPashi Jan 27 '20 at 10:13
  • 1
    Run the same code with `~pData[0] >> 5`. And then after that, `~(uint32_t)pData[0] >> 5`. Then complain about MISRA... unless of course you believe storing the value -1 in a `:1` bit-field is a good idea. – Lundin Jan 27 '20 at 15:43
1

The core problem here has nothing to do with MISRA, but with attempting to store a value at a specific slot in a bit-field. You cannot know how your bit-field layout actually ends up in memory, because that's not defined in the C standard.

Is your bit-field allocating 8 value bits in the MS byte or LS byte? Is it taking endianess in accordance or is it not? What is the bit order? Nobody knows. Step 1 is to get rid of the bit-field.

Step 2 is to get rid of anything unsigned int and use uint16_t/uint32_t.


As for MISRA-C 10.6 specifically, the rule against implicit conversion to a wider type was always rather misguided. The rationale MISRA used for this rule was to prevent people from writing code like uint32_t u32 = u16a + u16b; and thinking that the u32 operand of = somehow magically means that the operation will get carried out on 32 bits instead of 16. But on a 8/16 bit system, it is carried out with 16 bit arithmetic and there might be overflows/wrap-around.

Now as it happens, doing bit-shifts on signed types is always a very bad idea. pData[0] gets implicitly promoted to int which is signed. There are other MISRA rules dealing with this, rather than the one you quoted.

Regardless of MISRA, you should make a habit of always carrying out your shifts on unsigned types. "It's not dangerous in this case" is a bleak rationale. That means always write (uint32_t)pData[0] >> 5 and the cast should be applied before the shift not after it. This removes all uncertainties regarding undefined behavior left shifts and potentially arithmetic right shifts etc. Let the optimizer worry about the actual used size of the operands from there.

Lundin
  • 195,001
  • 40
  • 254
  • 396
0

I find MISRA C over-complicated for this very reason. Anyways, You didn't say you want to assign it directly. If that is the case, you can resort to the following:

typedef union {
    
    struct {
        unsigned int a : 1;
        unsigned int b : 1;
        unsigned int c : 1;
        unsigned int d : 1;
        unsigned int o : 1;
        unsigned int p : 1;
        unsigned int q : 1;
        unsigned int x : 1;
    };
    
    uint8_t u8Value;
    
} Character;

And setting those values by accessing tempChar.u8Value instead of the bit fields. For example,

tempChar.u8Value |= (1 << 0);

would set tempChar.a to 1.

That would still keep the neatness (readability) of code to the same extent. For example,

if(1 == tempChar.a)
{ 
    // Some code
}
WedaPashi
  • 3,561
  • 26
  • 42
0
tempChar.a = pData[0] >> 5;

In this 5 is a signed integer constant. You should use 5U for an unsigned constant

Also, the result of the right shift operation will be an int So you need to type cast the result back to unsigned int

tempChar.a = (unsigned int) (pData[0] >> 5U);
Rishikesh Raje
  • 8,556
  • 2
  • 16
  • 31