2

In the below C code, while checking the if condition, i am getting the Misra warning as

The operand of the opeartor '=='do not have same essential type category: one is 'Boolean' and other is 'unsigned'

In the header file file1.h

#define timer_4sec       (uint8)160
#define bool_rolltime    (roll_time_count < timer_4sec)

in the source file , file1.c

static uint8 roll_time_count = timer_4sec

void function()
{
  if(bool_rolltime == True)
  {
     printf("rolling ..\n");
  }
}
void function2()
{
    //update the roll_time_count status , increment and decrement static timer variable.
}

The problem is here if(bool_rolltime == True). I have understood that roll_time_count and timer_4sec are uint8 variables. So i have tried to fix this warning like this way

#define bool_rolltime    (bool)(roll_time_count < timer_4sec)

Is this code correct method to avoid the Misra error? If not please suggest how to avoid such warnings ?

user2986042
  • 1,098
  • 2
  • 16
  • 37
  • 1
    Make `True` an `int` of value `1` ... or omit the `== True` part (`if (bool_rolltime) /*stuff*/;`) – pmg Jul 22 '21 at 07:06
  • OT: regarding: `if(bool_rolltime == True)` True actually has the value of 1, The macro might not return 1. Suggest: `if(bool_rolltime != False)` or even better: `if(bool_rolltime)` – user3629249 Jul 22 '21 at 07:08
  • 3
    `if(bool_rolltime)` – ikegami Jul 22 '21 at 07:08
  • @user3629249: all relational operators yield an `int` with value of `0` or `1`. ([C11 6.5.8p6](http://port70.net/~nsz/c/c11/n1570.html#6.5.8)) – pmg Jul 22 '21 at 07:09
  • 4
    Why are you even comparing it to `True`? It's already a truth value. You just need `if (x)`, not `if (x == True)`. – Tom Karzes Jul 22 '21 at 07:18
  • A `0` value is false, all other values are true. For example `if(apples) { eat(1); }` – Weather Vane Jul 22 '21 at 07:22
  • i want to know , `if(bool_rolltime == False)` is a valid statement ? – user2986042 Jul 22 '21 at 07:26
  • 1
    Not according to MISRA. You are comparing apples with oranges. Consider `if(bool_rolltime == True)`. When `bool_rolltime == 2` and `True == 1` then the equality test fails. – Weather Vane Jul 22 '21 at 07:29
  • 1
    You have to remember that C (at least pre-C2x) has a real bodge as a boolean... a uint8_t with values 0 and 1 pretending to be a boolean. Never check `== TRUE` as this misses 254 valid true values! Always check `!= FALSE` – Andrew Jul 22 '21 at 07:30
  • @Andrew would MISRA warn about that too? – Weather Vane Jul 22 '21 at 07:33
  • 3
    But I'm puzzled as to why you are hiding your check in a macro... `if ( roll_time_count < timer_4sec )` is much clearer... although `if ( roll_time_count < ROLL_TIME_EXPIRY )` would be better, with `#DEFINE ROLL_TIME_EXPIRY TIMER_4SEC` – Andrew Jul 22 '21 at 07:33
  • 1
    @Andrew No, `x != FALSE` is not enough, you need at least `(x != FALSE) != FALSE`, but I recommend `((x != FALSE) != FALSE) != FALSE` just to be super extra sure. This should cover 98% of all your boolean checking needs. – n. m. could be an AI Jul 22 '21 at 07:33
  • :-) N1... for booleans, you can of course just do `!x` - but this should only be done with booleans, and not non-booleans... – Andrew Jul 22 '21 at 07:35
  • @Andrew "this should only be done with booleans" is incorrect. – Weather Vane Jul 22 '21 at 07:45
  • @andrew and @weather Vane `#define bool_rolltime (roll_time_count < timer_4sec)` will this return either `true` or `false` ? or `1` or `other values` since it is integers. which one is correct understanding ? if i make `bool temp = (roll_time_count < timer_4sec) ` then `temp` will return `true` or `false` . right ? – user2986042 Jul 22 '21 at 07:55
  • 2
    Yes... but why do you need the intermediate step? Just do `if (roll_time_count < timer_4sec) ` – Andrew Jul 22 '21 at 07:56
  • @WeatherVane Of course, C allows boolean operations for non-boolean values, but MISRA does not allow this, and a static code analysis with MISRA rules (or a compiler with integrated MISRA checks) will complain. – Bodo Jul 22 '21 at 07:56
  • 1
    The C Standard allows lots of things that are not necessarily a good idea ;-) – Andrew Jul 22 '21 at 07:58
  • @Bodo yes thanks. I thought the comment was a more general statement. – Weather Vane Jul 22 '21 at 07:58
  • 1
    Please show us the definition of `True`. – the busybee Jul 22 '21 at 08:17
  • Does the code use `#include `? This would define a type `bool` and the constants `true` and `false`, not `True`. Where does the definition of `True` come from? If your intention is to define your own Boolean type and constant values, your MISRA checker might know only the definitions in standard include files. – Bodo Jul 22 '21 at 10:06

1 Answers1

2

This essentially the same question you asked yesterday. As I answered then, you need to tell your static analyser which types that are boolean. Such as bool, false and true. Why you are using some home-brewed standard True, I don't know, but it's not a good idea as you noticed. Similarly, drop home-brewed uint8 and use standard C uint8_t.

In addition, you get MISRA violations because of smelly macros hiding simple expressions. That's plain bad practice, MISRA or no MISRA. You shouldn't invent some mysterious macro language. Just keep it simple and readable:

#define timer_4sec       160u // no need for cast but you need the u suffix for MISRA-C

static uint8_t roll_time_count = timer_4sec;

void function (void) // empty parenthesis is obsolete C, write (void)
{
  if(roll_time_count < timer_4sec)
  {
     printf("rolling ..\n");
  }
}

Now that we kept the code simple and readable, it automatically turned MISRA-C compliant too. Both operands of < are of the same "unsigned" essential type category thanks to the 'u' suffix.

Again, like I already answered yesterday, you need to study the rules for essential type in the MISRA-C guidelines or you won't understand a thing of what the tool keeps telling you.

Lundin
  • 195,001
  • 40
  • 254
  • 396