0

Observing below Misra warning for below part of code.

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

Not sure what could be the fix here to get rid of this warning.

#define C_BYTE unsigned char
C_BYTE SessionStatus;
#define DISCONNECTED   0x10

if((!(SessionStatus & (C_BYTE) DISCONNECTED)))
{
   //Do something
}

I tried a few chances but didn't work as below.

1)

if((~(SessionStatus & (C_BYTE) DISCONNECTED)))
{
       //Do something
}

2)

if((!(SessionStatus & (C_BYTE) DISCONNECTED)) != 0u)
{
       //Do something
}
Lundin
  • 195,001
  • 40
  • 254
  • 396
kapilddit
  • 1,729
  • 4
  • 26
  • 51
  • 1
    `~` (first fix) is definitely not what you want. I don't see anything wrong, technically or morally, with the first version. I don't think your second fix would do anything to fix things because the issue is on the `!`, which is before the comparison. Try dropping the `!` in your second fix, and making it an `==` comparing against `(C_BYTE) 0`. I don't know enough about the MISRA(ble) rules to be sure. – Thomas Jager Aug 19 '19 at 13:31
  • @ThomasJager The reason why MISRA-C doesn't allow `SessionStatus & DISCONNECTED` would be something like this `(SessionStatus & DISCONNECTED) << n`, which is dangerous code. Because of integer promotion, we've gotten a result that's of signed `int` and now, depending on the data and `n`, we are potentially shifting data into the sign bit of that `int`. Undefined behavior. – Lundin Aug 19 '19 at 13:46
  • @Lundin I'm not disagreeing with you, I don't know nearly enough to do that, but I don't see what disallowing `SessionStatus & DISCONNECTED` has to do with not permitting `!` in this case. Is it something like C++ compilers having an "error-type", and then getting errors on operators that use that "error-value"? – Thomas Jager Aug 19 '19 at 13:58
  • 1
    @ThomasJager Basically they disallow all forms of questionable, implicit type conversions. Enforcing a stronger type system upon C, if you will. To compare with, C++ the `!` operator would expect a bool operand and it returns a bool. Boolean logic in C however, results in `int`, which is potentially dangerous because it is a signed type. – Lundin Aug 19 '19 at 14:04
  • 1
    The double negative in `if((!(SessionStatus & (C_BYTE) DISCONNECTED)) != 0u)` is not necessary (or, IMO, desirable) — you could use: `if ((SessionStatus & (C_BYTE) DISCONNECTED) == 0u)`, could you not? It's even easier to understand than any of the alternatives using negation: "is the 'disconnected' bit zero?" – Jonathan Leffler Aug 19 '19 at 15:36
  • 1
    @JonathanLeffler Yeah that solution works, or better yet ensure that all integer constants are unsigned and then you can do `if ((SessionStatus & DISCONNECTED) == 0u)`. But the question was why the `!` operator specifically had an unpermitted operand. – Lundin Aug 20 '19 at 11:05

1 Answers1

4

The reason for the warning is that MISRA-C rule 10.1 expects the operand to the ! && || operators to be "essentially boolean". In your case it is actually an int, because of implicit type promotion rules.

Your 2nd example almost solved it, but you must convert to essentially boolean before applying !. That is:

if(!( (SessionStatus & (C_BYTE)DISCONNECTED) != 0u )) 

This is ok since the result of the != operator is to be regarded as essentially boolean. So that code is MISRA-C compliant, but a bit hard to read. I would instead recommend this:

#define DISCONNECTED   0x10u // u suffix makes this essentially unsigned
...

bool disconnected = (bool) (SessionStatus & DISCONNECTED);
if(!disconnected)

By adding the u suffix to the integer constant, it is essentially unsigned, same type category as unsigned char. Therefore the & operation is valid without using any casts. However, we are not allowed to implicitly convert from essentially unsigned to essentially boolean, therefore add the cast to bool.

EDIT

Since SessionStatus & DISCONNECTED is a "composite expression", MISRA doesn't allow the result to be assigned or cast to a different or wider essential type. The rationale is that they fear incompetent programmers who believe that the calculation in for example (uint32_t)(u16a + u16b) is carried out with uint32_t because of the cast, which is of course nonsense. (It would be better to educate the programmers about basic C than to come up with artificial rules, but anyway...)

I'd advise to ignore this rule entirely, but if you can't for whatever reason, here's an alternative fix:

#define DISCONNECTED   0x10u
...

unsigned char disconnected = SessionStatus & DISCONNECTED;
if(!(bool)disconnected)

But of course this is worse code than my first example.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • After trying this option, observing the warning: Impermissible cast of composite expression (different essential type categories) [MISRA 2012 Rule 10.8, required] – kapilddit Aug 20 '19 at 10:13
  • 1
    @kapilddit Oh right, well that's a silly rule. The rationale is that they are afraid that the programmer thinks that the type of the cast affects the type used for the calculation inside the parenthesized expression. If you know how implicit type promotions work, you can safely ignore this rule, permanently. But I'll add a fix to this answer. – Lundin Aug 20 '19 at 10:53