You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 12 Next »

Do not default the test for non-zero.
Essentially,

#define FAIL 0

if (foo() != FAIL) ...

is preferable to

#define FAIL 0

if (foo()) ...

despite the fact 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.

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

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

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

if(foo())
{
  return 0;
}

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.

\#define FAIL 0

if(foo()==FAIL) // explicit test for failure
{
  return 0;
}

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

\#define TRUE 1

if(foo() == TRUE) // Will evaluate to False if foo() returns 2, though that may logically imply a result of True.
{
  return true;
}

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.

\#define FALSE 0

if(foo() \!= FALSE) // Given convention (2), this will always yield the intended behavior.
{
  return true;
}

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.

char *a, *b;

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

However, doing so would produce incorrect behavior. As a result, such a result should never be defaulted.

Compliant Solution

The following approach to using a comparison function for this purpose is preferred.

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

char *inputstring, *somestring;

if ( STREQ( inputstring, somestring ))
{
  return "strings are equal";
}

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 does not conform to the common practices presented will be difficult to maintain. Bugs may easily arise when modifying helper functions which evaluate true/false or success/failure. Bugs may 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

medium

P8

L2

  • No labels