Both sides have side effects?

Question

I ran static code analysis for MISRA 2004 and MISRA 2012 on the following C code:

BOOL_TYPE Strings_Are_Equal(const char *s1, const char *s2)
{
  BOOL_TYPE result = True;
  const char *str1 = s1;
  const char *str2 = s2;

  if (NULL == s1 || NULL == s2)
  {
    result = False;
  }
  else if (strlen(s1) != strlen(s2))
  {
    result = False;
  }
  else
  {
    while (*str1 != 0)
    {
      if(tolower(*str1++) != tolower(*str2++))
      {
        result = False;
        break;
      }
    }
  }

  return result;
}

and got the following findings from the PC-lint reports:

Can somebody please explain how is code at line 58 and 66 is suffering from side effects and how should I correct it?

Lundin · Accepted Answer · 2016-06-28T11:43:45.283

Calling a function could invoke a side effect, when using the formal definition of the C standard.

In the specific case of strlen(s1) != strlen(s2), there is nothing inside those functions that could cause harm. It wouldn't make sense to implement them with for example internal static variables. But if there were such internal variables present, the order of evaluation could give different results depending on which function call that was executed first. This is probably the rationale behind the warning.

In the case of tolower(*str1++) != tolower(*str2++) there are both two function call side effects and two variable assignment side effects from the ++ operators, a total of 4 in a single expression. Even though this particular case is safe, such code is dangerous, as it could depend on the order of evaluation, or even turn out completely unsequenced (like i=i++;) which would be a severe bug.

Solve this by storing the function results in temporary variables. And never mix ++ with other operators, because that's both dangerous, pointless and banned by another MISRA rule:

MISRA-C:2004 Rule 12.13

The increment (++) and decrement (--) operators should not be mixed with other operators in an expression.

Correct answer imo. I gave you an upvote since you pointed out what I forgot at first. (It must have been drive-by downvoting) — 2501, Jun 28 '16 at 11:39
Reason for down vote? Lack of quotes by the MISRA rules and C standard? I can't find any errors in the answer. — Lundin, Jun 28 '16 at 11:40

Andrew · Answer 2 · 2019-09-12T11:06:36.503

As an add-on to Lundin's excellent answer, the MISRA compliant approach would be:

    while (*str1 != 0)
    {
      // Condition should be a single sequence point
      // ... with no side effects
      if ( tolower(*str1) != tolower(*str2) )
      {
        result = false;
        break;
      }

      // Now increment pointers
      str1++;
      str2++;
    }

Thus you have clear distinction between your sequence points.

Both sides have side effects?

2 Answers2