Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

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

...

  1. Functions return 0 if false, and non-zero if true [1].
  2. Function failures can typically be indicated by one of the following return values: -1 or any non-zero number.
  3. Comparison functions return 0 if the arguments are equal and non-zero otherwise (such as the standard library function strcmp(), which has a trinary return value) [2]).

Noncompliant Code Example

In this noncompliant code example, is_banned() return 0 returns zero if false , and non-zero if true.

Code Block
bgColor#ffcccc
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 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 above code is better written by checking for inequality with 0 (false) as follows:

Code Block
bgColor#CCCCFF
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) != 0) {
    return;
  }
  serveResults();
}

Noncompliant Code Example

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

...

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 no explicit test in processRequest(), the logical error is not obvious and seems correct by certain conventions.

Compliant Solution

The following compliant code sample is preferable for 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 above, it is immediately obvious that the if statement in processRequest() does not utilize the specification of validateUser() correctly.

Code Block
bgColor#CCCCFF
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

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

Because many comparison functions 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 a function testing for equality, but the programmer did not following follow the same convention as strcmp(), they the programmer might instinctively just replace the function name. Also, when quickly reviewed, the code could easily appear to test for inequality.

...

The code above works correctly. However, in order to simplify the login code or to facilitate checking a user's password more than once, a programmer might can separate the password checking code from the login function . They might do so in the following way:

Code Block
bgColor#ffcccc
int check_password(User *user, char *pw_given) {
  if (!strcmp((user->password),pw_given)) {
    return 1;
  }
  return 0;
}

void login(char *usr, char *pw) {
  User user = find_user(usr);
  if (!check_password(user, pw)) {
    grantAccess();
  }
  denyAccess("Incorrect Password");
}

In an attempt to leave the previous logic intact, the developer just replaces the strcmp with a call to their the new function. However, doing so produces incorrect behavior. In the case above, 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 using a comparison function for this purpose is preferred. 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
void login(char *usr, char *pw) {
  User user = find_user(usr);
  if (strcmp((user->password),pw_given) == 0) {
    grantAccess();
  }
  denyAccess("Incorrect Password");
}

Risk Assessment

Code which that does not conform to the common practices presented will be difficult to maintain. Bugs may can easily arise when modifying helper functions which 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 strcmp.

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

EXP20-C

medium

probable

low

P8

L2

Related Guidelines

ISO/IEC 9899:1999

Bibliography

[StackOvflw 2009] "Should I return TRUE / FALSE values from a C function?

...