Problem with MISRA C 2012 Rule 13.5

[jbrookley]

I am using Parasoft's C++ test and I'm getting a line flagged. The code is as follows:

Code:

if((padcValue1 = 4902)||(padcValue2 = 4902))  /* Error on this line here */
{
    LogFailure(FlagStartupDACTest);
    StartupFail = 1;
}

The error says "Do not use expressions with side effects in the right-hand operand of a logical operator". Is it saying I can't use multiple or cases or is the concern about making it coded so it's easier to test for MC/DC? I can break it up further but it just seems like an inefficient way to code it (unless there's an added benefit or I'm missing the intent of the code).

Any help you can give me would be greatly appreciated! Thanks!

[Steve Montgomery]

The C language standard basically says that (a) evaluation of logical AND and OR operators proceeds left-to-right and (b) evaluation of a logical operator stops when the result can be determined. So, when evaluating A || B, if A is true, B isn't evaluated. If evaluation of B would results in a side-effect, then that side-effect may or may not occur, depending on whether A evaluates to true or not.

I'm guessing that either or both of pacdValue1 and padcValue2 in your example are volatile-qualified, maybe because they read directly from an ADC register and therefore give rise to a side-effect when evaluated. Suppose for example that padcValue1 is volatile. The potential problem is that if padcValue1 > 4725 then padcValue1 will be evaluated twice, i.e. the register will be read twice and two side-effects will occur. But if padcValue1

[jbrookley]

Steve,

Thanks for your response!

You are correct that padcvalue1 and 2 are volatiles. The interesting thing is those padcvalue variables ARE the storage of the ADC info and this portion of the code is outside of the main code. Could the concern be that the interrupt might updating this variable in the ISR in between it being tested in the main code? If so, that does make sense to put this into a separate variable. That actually makes a lot of sense now . . . Thanks a lot!

[Steve Montgomery]

I think the concern is that any side-effect on the RHS of a logical operator may produce an unexpected result because of the uncertainty as to whether that side-effect will occur or not.

For example, if i has the value 0, and the following is executed:

Code:

if ((x == y) && (i++ == 10))

does i have value 0 or 1? It depends on the values of x and y. Now maybe the programmer who wrote this was perfectly aware of the way in which the operands of logical operands are evaluated. But maybe they weren't and might be surprised to discover that i only increments if x and y have the same value.

In the same way, a programmer may or may not be surprised to find that a volatile access may or may not occur on the RHS of a logical operator. An added problem for volatile accesses is that the nature of the side effect is going to depend on the target processor, the compiler, the design of the program (e.g. using volatile to qualify things that might be accessed from different interrupt levels) and so on. Since a volatile access could, in theory, cause any other volatile object to change its value and/or could cause an output to change state, the best the MISRA rules really can do is to draw attention to the potential issue and suggest, as in the case of this rule, a way of avoiding it.

[misra-c]

The MISRA-C Working Group agrees with the previous responses and has no further comments.