2
votes

I got a MISRA-C warning of Rule 10.8: A composite expression of 'essentially unsigned' type (unsigned char) is being cast to a wider unsigned type, 'unsigned short'.

This warning was detected with the following code.

void Fucntion(unsigned char isSwitchOn) {
    unsigned short switchMask = 0U;

    switchMask |= (unsigned short)(isSwitchOn & 1U) << 1;

Why was a warning detected? Also, does this code cause problems?

I think that the expression (isSwitchOn & 1U) is converted to an int type and calculated, and the result is truncated and stored as an unsigned short type. Thinking like that, the warning seems unnatural to me.

3
Not your question, but in your code, switchMask |= ... accesses an uninitialized variable.Stephan Lechner
And How can I avoid the warning?Khen
Sorry, this is snip of actual code. I edited code.Khen
Observe parenthesis.... (unsigned short)(isSwitchOn & 1U) << 1 vs (unsigned short)((isSwitchOn & 1U) << 1);...Myst

3 Answers

1
votes

Why was a warning detected?

Let us look at

void Fucntion(unsigned char isSwitchOn) {
  unsigned short switchMask = 0U;
  switchMask |= (unsigned short)(isSwitchOn & 1U) << 1;
  1. isSwitchOn, being lower rank than 1U, goes through the usual arithmetic conversion (C11 §6.5.10 3) to the type unsigned to match the type of 1U.

  2. isSwitchOn & 1U is calculated, the result type is unsigned.

  3. Cast (unsigned short) is applied to the unsigned result - this step seems strange. This is the MISRA-C warning of Rule 10.8. There is no need for the cast. The composite type unsigned is unnecessarily being narrowed to unsigned short.

  4. The (unsigned short) result is prepared for shifting and integer promotions are performed on each of the operands of the <<. So the (unsigned short) is promoted to int or possibly unsigned if USHRT_MAX > INT_MAX. Let us assume int.

  5. Now the shift of an int occurs. Result is type int.

  6. The int is applied to a unsigned short.


Also, does this code cause problems?

I do not see in this case, the cast causing an issue other than it is WET programing. Had the result been unsigned switchMask, yes then the shifted out bit would have been lost.

It makes more sense to cast the result after the shift. @Myst

switchMask |= (unsigned short)((isSwitchOn & 1U) << 1);

Or avoid other potential warnings with

switchMask = (unsigned short)(switchMask | ((isSwitchOn & 1U) << 1));

I think that the expression (isSwitchOn & 1U) is converted to an int type and calculated

No, the expression (unsigned short)(isSwitchOn & 1U) is converted to an int type and calculated.


Note: It is dubious that unsigned char isSwitchOn and unsigned short switchMask are not the same type.

0
votes

regarding:

unsigned short switchMask = 0U;

this statement is assigning a unsigned short int using a unsigned int

regarding:

switchMask |= (unsigned short)(isSwitchOn & 1U) << 1;

this statement is anding a unsigned short int with a unsigned int

0
votes

Chux has posted an answer that explains what happens with the code and why it is wrong. As for how to fix it, you have to rewrite it. There are two problems:

  • You are using a character type as a boolean and then you also use that character type in arithmetic, casting the type after the promotions, instead of before them. Supposedly this is done to get rid of extra branches.
  • You are using the native integer types of C, which can have any size. This is not recommended practice and violates MISRA-C:2012 Dir 4.6. Get rid of these types and use stdint.h instead, which allows you to write deterministic, portable code.

Now there are two ways to deal with implicit type promotions: either you let them happen and cast afterwards - this is usually what MISRA prefers. Or you don't let them happen at all, by casting to a wide unsigned type before the operation is evaluated, eliminating the implicit promotion. Either should be fine as far as MISRA is concerned; I personally prefer the latter.

Fixed code would look like:

// ok if 8/16 bit system only
void Function (bool isSwitchOn) {
    uint16_t switchMask = ((uint16_t)isSwitchOn & 1U) << 1U;

or

// 32 bit system/fully portable code
void Function (bool isSwitchOn) {
    uint32_t shift = (uint32_t)isSwitchOn & 1U;
    uint16_t switchMask = (uint16_t) (shift << 1U);

Please note that these examples should generate the very same machine code. So you can use the fully portable version on a 8 bit system too. Both are MISRA-C compliant on the given systems - no implicit promotions take place anywhere.

Further info: Implicit type promotion rules.