...
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
only checks for equality only with 1.
Compliant Solution
Because most functions guarantee a return value of nonzero only for true, the preceding code is better written by checking for inequality with 0 (false), as follows:
Code Block | ||||
---|---|---|---|---|
| ||||
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(); } |
...
In noncompliant code, function status can typically be indicated by returning −1
returning −1 on failure or any nonnegative number on success. This is a common convention in the standard C library, but it is discouraged in ERR02-C. Avoid in-band error indicators.
Although failures are frequently indicated by a return value of zero, some common conventions may conflict in the future with code where 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 foo()
to return an error code or −1 rather than 0 to indicate a failure (all of which are also common conventions).
...
Recommendation | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
EXP20-C | medium | probable | low | P12 | L1 |
Related Guidelines
Bibliography
[StackOvflw 2009] | "Should I |
---|
...
Return TRUE/FALSE |
---|
...
Values from a C |
---|
...
Function?" |
---|
...