Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Parasoft C/C++test 2023.1

Perform explicit tests to determine success, true/false, and equality to improve the readability and maintainability of code and for compatibility with common conventions.

In particular, do Do not default the test for non-zero.
Essentiallynonzero. For instance, suppose a foo() function returns 0 to indicate failure or a nonzero value to indicate success. Testing for inequality with 0,

Code Block
bgColor#ccccff
langc

#define FAIL 0

if (foo() != FAIL0) ...

is preferable to

Code Block
bgColor#ffcccc
langc

#define FAIL 0

if (foo()) ...

despite the fact convention that C considers 0 to be false. The first will be beneficial if it is later decided that a failure return should be -1 rather than 0.

Performing explicit tests to determine success, true/false, and equality makes code both maintainable and compliant with common conventions.
In the case that the return value will never change, an explicit test is still preferable to clearly communicate the numeric, rather than boolean, value of the test.

0 indicates failure. Explicitly testing for inequality with 0 benefits maintainability if foo() is later modified to return −1 rather than 0 on failure.

This recommendation is derived from and considers the implications of the following common conventions:(1)

  1. Functions return 0 if false and nonzero if true [StackOvflw 2009].
  2. Function failures can typically be indicated by

...

  1. −1 or any nonzero number.
  2. Comparison functions (such as the standard library function strcmp()

...

  1. , which has a trinary return value) return 0 if the arguments are equal and

...

  1. nonzero otherwise (see strcmp function).

Noncompliant Code Example

In this noncompliant code example, is_banned() returns 0 if false and nonzero if true:

Code Block
bgColor#ffcccc
langc
LinkedList bannedUsers;

int is_banned(User usr) {
  int x = 0;

  Node cur_node = (bannedUsers->head);

  while (cur_node != NULL) {
    if(!strcmp((char *)cur_node->data, usr->name)) {
      x++;
    }
    cur_node = cur_node->next;
  }

  return x;
}

void processRequest(User usr) {
  if(is_banned(usr) == 1) {
    return;
  }
  serveResults();
}

If a banned user is listed twice, the user is granted access. Although is_banned() follows the common convention of returning nonzero for true, processRequest checks for equality only with 1.

Compliant Solution

Because most functions guarantee a return value of nonzero only for true, the preceding code is better written by checking for inequality with 0 (false), as follows:

Code Block
bgColor#CCCCFF
langc
LinkedList bannedUsers;

int is_banned(User usr) {
  int x = 0;

  Node cur_node = (bannedUsers->head);

  while(cur_node != NULL) {
    if (strcmp((char *)cur_node->data, usr->name)==0) {
      x++;
    }
    cur_node = cur_node->next;
  }

  return x;
}

void processRequest(User usr) {
  if (is_banned(usr) != 0) {
    return;
  }
  serveResults();
}

Noncompliant Code Example

In noncompliant code, function status can typically be indicated by returning −1 on failure or any nonnegative number on success. This is a common convention in the standard C library, but it is discouraged in ERR02-C. Avoid in-band error indicators.

These conventions are not entirely non-controversial and is a point of contention for many programmers during code reviews. Here is a typical discussion among programmers on the subject of convention (1).

Noncompliant Code Example

In regards to (1):

Although failures are frequently indicated by a return value of zero (which C considers to be false), there are 0, some common conventions that may conflict in the future with code where in which the test for non-zero nonzero is not explicit. In this case, defaulting the test for non-zero nonzero welcomes bugs if and when a developer modifies foo validateUser() to return an error code or -1 −1 rather than 0 to indicate a failure (all of which are also common conventions). 

Code Block
bgColor#ffcccc
langc
int 

validateUser(User usr)
 {
  if(listContains(validUsers, usr))
  {
    return 1;
  }

  return 0;
}

void processRequest(User usr, Request request)
 {
  if(!validateUser(usr))
  {
    return "invalid user";
  }
  else
  {
    serveResults();
  }
}

The Although the code above will work as intended. However, it is very feasible possible that a future modification will result in the following.:

Code Block
bgColor#ffcccc
langc
errno_t 
validateUser(User usr)
 {
  if(list_contains(allUsers, usr) == 0)
  {
    return 303;  //* userUser not found error code */
  }
  if(list_contains(validUsers, usr) == 0)
  {
    return 304; //* invalidInvalid user error code */
  }

  return 0;
}

void processRequest(User usr, Request request)
 {
  if(!validateUser(usr))
  {
    return "invalid user";
  }
  else {
    serveResults();
  }
}

In this code, the programmer intended to add error code functionality to indicate the cause of a validation failure. The new code, however, validates any invalid or non-existing nonexisting user. Because there is not an no explicit test in processRequest(), the logical error is not obvious (the implied "if not validated" seems correct) and seems correct by certain conventions.

Compliant Solution

The following This compliant code is preferable for code improved maintenance. By defining what constitutes a failure and explicitly testing for it, the behavior is clearly implied, and future modifications are more likely to preserve it. If there is a future modification like the one aboveis made, such as in the previous example, it is immediately obvious that the if statement in processRequest() does not correctly utilize the specification of validateUser correctly().

Code Block
bgColor#CCCCFF
langc
int 
validateUser(User usr)
 {
  if(list_contains(validUsers, usr))
  {
    return 1;
  }

  return 0;
}

void processRequest(User usr, Request request)
 {
  if(validateUser(usr) == 0)
  {
    return "invalid user";
  }
  else
  {
    serveResults();
  }
}

Noncompliant Code Example

In regards to (2):

Comparison functions (such as the standard library strcmp() function) return 0 if the arguments are equal and nonzero otherwise.

Because many comparison functions return 0 for equality and nonzero for inequality, they can cause confusion when used to test for equality. If someone were to switch the following strcmp() call with a function testing for equality, but the programmer did not follow the same convention as strcmp(), the programmer might instinctively just replace the function name. Also, when quickly reviewed, the code could easily appear to test for inequality.The following is fairly common yet ignores the convention that most functions in C only guarantee a non-zero return value to indicate True/Yes/etc..

Code Block
bgColor#ffcccc
langc
void login(char *usr, char *pw
LinkedList bannedUsers;

is_banned(User usr) {
  intUser xuser = 0;

  Node cur_node = (bannedUsers->headfind_user(usr);

  while(cur_node != NULL) {
    if(strcmp((char *)cur_node->data, usr->nameif (!strcmp((user->password),pw)) {
      x++grantAccess();
    }
    cur_node = cur_node->next;else {
  }

  return x;
}

processRequest(User usr)
{
  if(is_banned(usr) == 1)
  {
    returndenyAccess("Incorrect Password");
  }
  serveResults();
}

If a banned user is listed twice, he gets access. Although check_user follows the common convention of returning non-zero for true, processRequest only checks for equality with 1.

Compliant Solution

Because most functions only guarantee a return value of non-zero for "true," the code above is better written by checking for inequality with 0 ("false") as follows.

The preceding code works correctly. However, to simplify the login code or to facilitate checking a user's password more than once, a programmer can separate the password-checking code from the login function in the following way:

Code Block
bgColor#ffcccc
langc
int check_password(User *user, char *pw_given) {
  if (!strcmp((user->password),pw_given)
Code Block
bgColor#CCCCFF

LinkedList bannedUsers;

is_banned(User usr) {
  int x = 0;

  Node cur_node = (bannedUsers->head);

  while(cur_node != NULL) {
    if(strcmp((char *)cur_node->data, usr->name) {
    return  x++1;
  }
  return 0;

}

void login(char *usr,  cur_node = cur_node->next;char *pw) {
  }

User user return x;
}

processRequest(User usr)
{= find_user(usr);
  if (is!check_banned(usr) != 0)
  password(user, pw)) {
    returngrantAccess();
  }
  else {
  serveResults(  denyAccess("Incorrect Password");
  }
}

Noncompliant Code Example

In regards to (3):

Because comparison functions (like strcmp) return 0 for equality and non-zero for inequality, they can cause confusion when used to test for equality. If someone were to switch the following strcmp call with an equals function, they might instinctively just replace the function name. Also, when quickly reviewed, the code could easily appear to test for inequality.

Code Block
bgColor#ffcccc

char *a, *b;

if(\!strcmp(a,b))
{
  return "strings are equal";
}

However, doing so would produce incorrect behavior. As a resultIn an attempt to leave the previous logic intact, the developer just replaces strcmp() with a call to the new function. However, doing so produces incorrect behavior. In this case, any user who inputs an incorrect password is granted access. Again, two conventions conflict and produce code that is easily corrupted when modified. To make code maintainable and to avoid these conflicts, such a result should never be defaulted.

Compliant Solution

The following approach to This compliant solution, using a comparison function for this purpose, is the preferred approach. By performing an explicit test, any programmer who wishes to modify the equality test can clearly see the implied behavior and convention that is being followed.

Code Block
bgColor#ccccff#CCCCFF
langc
void login(char *usr, char *pw) {
  User user = find_user(usr);
  if
\#define STREQ(str1, str2) (strcmp((str1user->password), (str2)pw) == 0) {

char *inputstring, *somestring;

if ( STREQ( inputstring, somestring ))
    grantAccess();
  }
  else {
  return  denyAccess("stringsIncorrect are equalPassword");
  }
}

By defining a macro to adapt the comparison function to have a return value compliant with convention (3), the code clearly illustrates its intent and agrees with implied behavior.

Risk Assessment

Code which that does not conform to the common practices presented will be presented is difficult to maintain. Bugs may can easily arise when modifying helper functions which that evaluate true/false or success/failure. Bugs may can also easily arise when modifying code that tests for equality using a comparison function that obeys the same conventions as standard library functions like such as strcmp.

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

EXP20-C

Medium

Probable

Low

P12

L1

Automated Detection

Tool

Version

Checker

Description

Astrée
Include Page
Astrée_V
Astrée_V

Supported indirectly via MISRA C:2004 Rule 13.2.
Axivion Bauhaus Suite

Include Page
Axivion Bauhaus Suite_V
Axivion Bauhaus Suite_V

CertC-EXP20Fully implemented
Helix QAC

Include Page
Helix QAC_V
Helix QAC_V

C3344, 
C4116

LDRA tool suite
Include Page
LDRA_V
LDRA_V
114 SPartially implemented
Parasoft C/C++test
Include Page
Parasoft_V
Parasoft_V

CERT_C-EXP20-a
CERT_C

medium

probable

medium

P8

L2

-EXP20-b

Avoid comparing values with TRUE macro/enum constant using equality operators ("==", "!=")
Tests of a value against zero should be made explicit, unless the operand is effectively Boolean

PC-lint Plus

Include Page
PC-lint Plus_V
PC-lint Plus_V

697

Partially supported: reports comparisons of Boolean values to constants other than 0

Bibliography

[StackOvflw 2009]"Should I Return TRUE/FALSE Values from a C Function?"

...

Image Added Image Added Image Added