Versions Compared

Key

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

...

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

...

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:

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

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.

...

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

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.

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-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 Modified Image Modified Image Modified