The right hand operand of a logical operator || has persistent side effects because of calling function (MISRA- C 2012Rule 13.5)

user_17436494 0
Viewed 311

The right hand operand of a logical operator || has persistent side effects because of calling function detectError().

if ( ( detect() == VALID ) || 
         ( detectError() == INVALID ) )
    {
        up( a,b );  
      }

typedef enum
{

C;

}E_name;

typedef struct
{
 
 E_name be:4;
  
}S_name;
S_name name;

persistent_side_effect: Expression name.be = C has persistent side effect: modifying non-local object okay.be = C.

sint16 detectError(void)
{
name.be = C;
}

I was able to solve logical operator &&, is there a solution for || operator?

misra c
2 Answers

Surely the simplest work around for this is:

whateverType detectFlag1 = detect();
whateverType detectFlag2 = detectError();

if ( ( detectFlag1 == VALID ) || ( detectFlag2 == INVALID ) )
{
   up( a,b );  
}

Simple, clear code, with no potential side effects?

user_16143190

Generally, code with MISRA-C quality concerns needs to be deterministic and there should exist at least one use-case where some part of the code gets executed (code coverage). In this case there is no telling if detectError() gets called or not, which may or may not be problematic depending on if that function contains any side effects.

Also, common sense doesn't sit well with "if detect is valid or detect error is invalid". What's that even supposed to mean, if detect failed but you couldn't detect errors, then wouldn't that leave your program in an undefined state?

Of course I have no idea what these functions are doing, but maybe at least consider better identifier naming. Maybe "detect error" should be called "get last error" or such.

Assuming the code is correct, then you can rewrite it in a clearer but otherwise equivalent manner like this:

if(detect() == VALID)
{
  up(a, b);
}
else if(detectError() == INVALID)
{
  up(a, b);
}
else
{
  ; // possibly handle this scenario or leave it blank
}

Note that the else is mandatory as per MISRA-C defensive programming/self-documenting code.

Other concerns:

  • Using bit-fields in a MISRA-C application is a very bad idea. MISRA-C doesn't prohibit them, but they are generally non-portable and poorly standardized, so they don't belong in a deterministic or portable program.
  • Don't come up with home-made garage standards for integers such as sint16. Use standard C sint16_t from stdint.h instead. If you are stuck with C90 then make typedefs corresponding to the stdint.h names.
user_5845180
Related