Page History

...

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

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

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):

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

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

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

In regards to (2):

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

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 code above will work as intended. However, it is very feasible that a future modification will result in the following.

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";
  }

  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 user. Because there is not an 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 is preferable for code 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.

errno_t 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):

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

...

...

(

...

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.

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) {
      x++;
    }
    cur_node = cur_node->next;
  }

  return x;
}

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

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.

...

\#define STREQ(str1, str2) (strcmp((str1), (str2)) == 0)

char *inputstringstr1, *somestringstr2;

if ( STREQstrcmp( inputstring, somestring )str1, str2) == 0 )
{
  return "strings are equal";
}

...