4

I have written the following piece of code which MISRA does not like:

UartPtr->C &= ((uint8_t)(~SIO_C2_SBK));

with

#define SIO_C2_SBK ((uint8_t)0x01u)

and UartPtr is defined as

UartPtr = (UartStruct*) 0x12345678; /* I know that this is also a violation of MISRA */

with the underlying datastructure:

typedef volatile struct UartStructTag
{
  uint8_t      BDH;
  uint8_t      BDL;
  uint8_t      C1;
  uint8_t      C2;
} UartStruct;

My Misra checker complains about the first line and states, that

An integer constant expression with negative value is being converted to an unsigned type.

However, the following line does not yield into a problem with MISRA:

UartPtr->C |= ((uint8_t)(SIO_C2_SBK));

So the problem comes from the bitwise negation. But as all operations are directly casted to uint8_t, i do not get the violation of the MISRA standard. Who wants to help me here?

m47h
  • 1,611
  • 2
  • 18
  • 26

2 Answers2

6

In any arithmetic expression, values of types smaller than int are implicitly converted to int before they are processed. The C language cannot do arithmetic on types smaller than int. Thus, your code actually behaves like this:

UartPtr->C &= ((uint8_t)(~(int)(uint8_t)0x01u));

which is just

UartPtr->C &= ((uint8_t)(~1));

where ~1 has the value -2 on two's complement architectures.

To fix this issue, convert to unsigned or any other unsigned type larger than int before applying bitwise not:

UartPtr->C &= ((uint8_t)(~(unsigned)SIO_C2_SBK));
fuz
  • 88,405
  • 25
  • 200
  • 352
  • So is the reason for this rule that if e.g. SIO_C2_SBK would be a negative signed constant, the operation would eventually not yield the desired result and the cast would hide this? – m47h Dec 02 '15 at 10:54
  • 2
    @m47h The rule is rather trying to protect you from situations like `if(~SIO_C2_SBK > 0)` which would not behave as expected. That's one really nasty bug to track down. – Lundin Dec 02 '15 at 10:57
5

The ~ operator, like most C operators, will do an implicit integer conversion of the operand before the operator is applied.

#define SIO_C2_SBK ((uint8_t)0x01u)

So the above macro is the issue, because you are forcing the literal down from unsigned int type into a small integer type, which will get implicitly promoted. Instead of uint8_t you end up with int, before ~ is applied.

This violates rule 10.1 of MISRA-C:2004, which doesn't allow implicit conversions that yield a type of different signedness (such conversions are dangerous, so it is a very good rule).

  • If you don't need this macro to give uint8_t, then simply drop the (uint8_t cast and that will solve the problem.

  • If this macro for some reason must give uint8_t, then change the code to this (MISRA compliant):

    UartPtr->C &= (uint8_t) ~(uint32_t)SIO_C2_SBK;
    

    where uint32_t corresponds to the size of int on the given platform.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • I am not terribly familiar with MISRA, but does it not require masking with `& 0xFF` before casting with `uint8_t`? – user694733 Dec 02 '15 at 10:52
  • 1
    @user694733 Nope. Masking with `& 0xFF` is essentially the same thing as casting to `uint8_t`. However, MISRA 10.5 explicitly requires you to cast the result of the `~` operator to the intended type. – Lundin Dec 02 '15 at 10:55