Search code examples
cmisra

Why does MISRA-C:2004 throw an error here?


I seem to keep getting MISRA-C:2004 rule 10.1 and 10.3 errors for the lShift assignment in the following snippet and can't really see what more could be done to satisfy the requirement... why am I still getting an error?

#define ADC_INTSELxNy_LOG2_NUMBITS_PER_REG 3U
#define ADC_INTSELxNy_NUMBITS_PER_REG 8U
void foo (const bar_e intNumber) {
    uint_least8_t lShift = (uint_least8_t)(ADC_INTSELxNy_NUMBITS_PER_REG - (((((uint_least8_t)intNumber) + 1U) & 0x1U) << ADC_INTSELxNy_LOG2_NUMBITS_PER_REG));
    //...
}

Solution

  • That line is an unreadable mess. Consider splitting it up to increase readability. Depending on the width on int on your system, the code will look different. The code below assumes 32 bit integers.

    uint8_t        bit      = (uint8_t)( ((uint32_t)intNumber + 1U) & 0x1U );
    uint32_t       lShift32 = ((uint32_t)bit << ADC_INTSELxNy_LOG2_NUMBITS_PER_REG);
    uint_least8_t  lShift8  = (uint_least8_t)((uint32_t)ADC_INTSELxNy_NUMBITS_PER_REG - lShift);
    

    Now as for the reason why you got errors, rules 10.1 and 10.3 are concerned with implicit integer type promotions. If you split the code up as I did above, you'll get less confused about what's the "underlying type" of each sub-expression. What you did wrong was to add a cast to the underlying type before the operation, which does little good. You need to do it after each operation.

    The + operation needs an explicit cast to underlying type after the + operation, same thing with the & operation and the shift operation. It is not sufficient just to cast everything to the underlying type at the end, you have to consider each sub-expression individually.

    To explain my code above:

    The first row has an explicit cast to uint32_t to ensure that intNumber is of the same type as 1U. That way, there is no implicit conversion of the sub-expression intNumber + 1U and this is a wider type of same signedness as the underlying type uint8_t (meaning it is safe). The result of the addition is of type unsigned int. Again, unsigned int & unsigned int yields no implicit conversion. And finally the result is cast to uint8_t to satisfy a number of MISRA rules. The rest of the code goes on in the same manner.

    I always try to make a widening cast to a large integer type before the operation, to simplify everything, avoiding implicit promotions and avoiding numerous casts after each other.

    Note that the true purpose of rule 10.1 is to actually force you to educate yourself about implicit type promotions in C. It is a rather complex topic, but a frightening number of C programmers are oblivious to type promotions and all the dangers and bugs caused by them. More info about the type promotion rules here.