Versions Compared

Key

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

...

Code Block
bgColor#ccccff
langc
if (foo() != 0) ...

is preferable to

Code Block
bgColor#ffcccc
langc
if (foo()) ...

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

...

  1. Functions return 0 if false and nonzero if true [StackOvflw 2009 AA. Bibliography#StackOvflw 09].
  2. Function failures can typically be indicated by −1 or any nonzero number.
  3. Comparison functions (such as the standard library function strcmp(), which has a trinary return value) return 0 if the arguments are equal and nonzero otherwise (see strcmp function).

...

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.

...

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(); }
Code Block
bgColor#CCCCFF
langc

Noncompliant Code Example

...

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();
  }
}

Although the code will work as intended, it is 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; /* User not found error code */
  }
  if(list_contains(validUsers, usr) == 0) {
    return 304; /* Invalid 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 BB. Definitions#validation failure. The new code, however, validates any invalid or nonexisting user. Because there is no explicit test in processRequest(), the logical error is not obvious and seems correct by certain conventions.

...

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

...

Code Block
bgColor#ffcccc
langc
void login(char *usr, char *pw) {
  User user = find_user(usr);
  if (!strcmp((user->password),pw)) {
    grantAccess();
  }
  else {
    denyAccess("Incorrect Password");
  }
}

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:

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(); } else { denyAccess("Incorrect Password"); } }
Code Block
bgColor#ffcccc
langc

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

...

void login(char *usr, char *pw) { User user = find_user(usr); if (strcmp((user->password),pw) == 0) { grantAccess(); } else { denyAccess("Incorrect Password"); } }
Code Block
bgColor#ccccff
langc

Risk Assessment

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

...

Tool

Version

Checker

Description

CERT C Rules implemented in the LDRA tool suite
Include Page
LDRA_V
LDRA_V
114 SPartially Implemented

PRQA QA-C

Include Page
PRQA QA-C_v
PRQA QA-C_v

3344
4116

 

Bibliography

[StackOvflw 2009AA. Bibliography#StackOvflw 09]"Should I Return TRUE/FALSE Values from a C Function?"

...

Image Modified