1

I have a simple macro function which will check the condition and return an Boolean value. Below is the snippet of code

assume: #define boolean bool

Test.h file

#define CHECK_STATE(X)            (boolean)(STATE_VALUE == (uint8)(X) )

Test.c file

enum MY_STATE{

    STATE_0,  
    STATE_1,     
    STATE_2    
};

static uint8 STATE_VALUE  = (uint8)STATE_0; //This value update in the code dynamically from state 0 to 2

if(CHECK_STATE(STATE_1))  /*MISRA warning: Controlling expression is not an 'essentially Boolean' expression. */
{
  /*If condition pass*/
}
else
{
  /*If condition failes*/
}

In the above code CHECK_STATE function will return a Boolean value. So if loop can check either true or false. So why MISRA is throwing the warning "not an 'essentially Boolean' expression" ? Any suggestion to fix this type of warnings?

user2986042
  • 1,098
  • 2
  • 16
  • 37
  • 1
    `(STATE_VALUE == (uint8)(X))` gives boolean value – 0___________ Aug 02 '23 at 18:57
  • Does `if(CHECK_STATE(STATE_1) == true)` fix the problem? Check https://stackoverflow.com/q/52780276/1606345 – David Ranieri Aug 02 '23 at 19:03
  • 1
    It might be clearer to the MISRA checker if you used a switch statement to deal with each of the three possible values of enum MY_STATE. – Jim Rogers Aug 02 '23 at 19:03
  • 3
    Remove the `(boolean)` cast from the macro, it might be confusing the checker – Eugene Sh. Aug 02 '23 at 19:29
  • OT why is `STATE_VALUE` type `uint8` and not `int`? You're having to cast everything in sight. – Weather Vane Aug 02 '23 at 19:35
  • 3
    "*assume: `#define boolean bool`*" -- I suppose we are then also expected to assume that `stdbool.h` has also been `#include`d? This seems needlessly indirect. If you feel you must have `boolean` for a type name, then why not define it as built-in type `_Bool`, which does not depend on any headers. With that said, however, the outer cast in your `CHECK_STATE()` macro is kinda ridiculous. I would drop it. – John Bollinger Aug 02 '23 at 20:23
  • The cast `(boolean)` is useless, redundant and confusing, especially with a custom `boolean` definition. – chqrlie Aug 04 '23 at 20:10
  • From a MISRA perspective, the result of `==` and similar logic/relational operators is already of type "essentially boolean". – Lundin Aug 07 '23 at 08:49
  • OT and partially echoing @WeatherVane - why not use ``static enum MY_STATE STATE_VALUE`` and avoid the casts? – Andrew Aug 07 '23 at 09:11

2 Answers2

0

In your analyzer you should be able to specify your custom bool type, e.g. in PClint:

-strong(B,boolean) /* custom bool */

And define it as:

typedef uint8_t boolean;

I also prefer custom bool since in many embedded libraries/HAL define their own bool, which can cause collision.

Eva4684
  • 21
  • 3
0

Of course, MISRA prefers you to use inline-functions to function-like macros, to allow better type matching... so your tool is missing a D.4.9 violation! But to the matter in hand...

Your tool is not correctly configured to treat you custom boolean as a boolean-type; this is compounded by your code being unnecessarily verbose.

There are two ways around this:

1. Simplify Use the MISRA essential types, and use the enumeration type

Test.h file

#define CHECK_STATE(X) ( STATE_VALUE == (X) )  // No casts needed!

Test.c file

enum MY_STATE {
    STATE_0,  
    STATE_1,     
    STATE_2    
};

static enum MY_STATE STATE_VALUE  = STATE_0; // No cast needed

if ( CHECK_STATE( STATE_1 ) ) 
{
  /*If condition pass*/
}
else
{
  /*If condition fail*/
}

2. Configure the tool See @Eva4684's answer for a hint

IMHO the KISS principle should apply here

As an aside, why are you defining the macro in a header file, when both the variable are the enumeration are module scope?

Andrew
  • 2,046
  • 1
  • 24
  • 37