Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

MISRA 2012 violation - Type mismatch (Rules 10.1, 10.4)

I'm facing MISRA C 2012 violation that I can't understand. Following is the code:

#define I2C_CCRH_FS      ((uint8_t)0x80)
#define I2C_CCRH_DUTY    ((uint8_t)0x40)
#define I2C_CCRH_CCR     ((uint8_t)0x0F)

typedef struct I2C_struct
{
  volatile uint8_t CR1;
  volatile uint8_t CR2;
  volatile uint8_t CCRL;
  volatile uint8_t CCRH;
} I2C_TypeDef;

#define I2C_BaseAddress         0x5210
#define I2C ((I2C_TypeDef *) I2C_BaseAddress)

I2C->CCRH &= ~(uint8_t)((I2C_CCRH_FS | I2C_CCRH_DUTY) | I2C_CCRH_CCR);

In the previous code, PC-Lint complains that:

Unpermitted operand to operator '|' [MISRA 2012 Rule 10.1, required]

Mismatched essential type categories for binary operand [MISRA 2012 Rule 10.4, required]

Rule 10.1 states that there should be no problem ORing unsigned ints. (PC-Lint passes the first OR operation and complains about the second one!!)

Rule 10.4 states that the operands of the operation shall have the same essential type.

I can't understand why these violations exist although all of the operands are declared as uint8_t?

I've tried puting parentheses around each two of the ORed constants. I've also tried casting all of them to uint8_t and to volatile uint8_t. Non solved the violation.

I checked these two posts (1, 2) but they don't answer my question.

like image 238
Salahuddin Avatar asked Jun 07 '18 08:06

Salahuddin


2 Answers

I2C_CCRH_FS | I2C_CCRH_DUTY is MISRA-compliant by itself. Both operands are essentially unsigned so that sub-expression is fine. However, there is still an implicit conversion of each operand to int. The result in practice is of type int.

In pseudo code: when you do (result as int) | I2C_CCRH_CCR, the operands before implicit promotion have the types int | uint8_t. The uint8_t will get integer promoted to int here too. You have operands of different signedness.

(I guess that the tool complains about 10.4 since the integer promotions are part of the usual arithmetic conversiosn, which is what 10.4 is about.)

This whole expression doesn't cause any problems in practice so the warning is mostly pedantic. But imagine if you had done ~(I2C_CCRH_FS | I2C_CCRH_DUTY) | I2C_CCRH_CCR) with no casts - you'd end up with a negative number, something along the lines of 0xFFFFFFxx expressed in 2's complement. That would potentially be dangerous.

To fix this, you have two options:

  • For every operation, cast the result back to the intended type. This is often the spirit of MISRA-C.
  • Cast the operands to a large unsigned type before the operation. Usually more readable IMO.

Note also that the ~ operator should not be used with a signed operand! This is a violation of rule 10.1. The cast back to uint8_t should be done last.


TL;DR. How to get the code MISRA compliant:

You'd either have to do something semi-awful like this:

I2C->CCRH &= (uint8_t) ~ (uint8_t) ((uint8_t)(I2C_CCRH_FS | I2C_CCRH_DUTY) | I2C_CCRH_CCR)

That's a bit of a mess. I would instead cast in advance. Assuming 32 bit CPU:

I2C->CCRH &= (uint8_t) ~( (uint32_t)I2C_CCRH_FS    |  // comment explaining FS
                          (uint32_t)I2C_CCRH_DUTY) |  // comment explaining DUTY
                          (uint32_t)I2C_CCRH_CCR   ); // comment explaining CCR

The above style is useful when dealing with MCU registers and similar. This code is acceptable, but could be simplified further.

If it is possible to change the defines to #define I2C_CCRH_FS 0x80u, then you get:

I2C->CCRH &= (uint8_t) ~(I2C_CCRH_FS | I2C_CCRH_DUTY | I2C_CCRH_CCR);

and it would still be MISRA compliant, because of the handy little u suffix that MISRA likes.

like image 196
Lundin Avatar answered Oct 11 '22 16:10

Lundin


When you perform the bitwise operation (I2C_CCRH_FS | I2C_CCRH_DUTY) the result gets promoted to an integer. See Integer Promotion rules Here

Therefore the mismatch is occurring between the result of the above operation and the next bitwise OR | I2C_CCRH_CCR

To fix this you need to add a cast to the result of BOTH Bitwise OR operations.

The first cast is required to cast the result of the ~ operator from int back to unsigned

I2C->CCRH &= (uint8_t)~(uint8_t)((uint8_t)(I2C_CCRH_FS | I2C_CCRH_DUTY) | I2C_CCRH_CCR);

To Explain

I2C->CCRH &= (uint8_t)~    // Required to cast result of ~ operator to uint8_t
             (uint8_t)     // Casts the result of the 2nd OR to uint8_t
             ((uint8_t)    // Casts the result of the 1st OR    
              (I2C_CCRH_FS | I2C_CCRH_DUTY)  // 1st OR
              | I2C_CCRH_CCR);      // 2nd OR
like image 2
Rishikesh Raje Avatar answered Oct 11 '22 15:10

Rishikesh Raje