clintside-effectsmisrapc-lint

Both sides have side effects?


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: enter image description here

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


Solution

  • 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.