...
These examples also assume that errno
is set if fopen()
or malloc()
fail. These are guaranteed by POSIX but not by C11. See ERR30-C. Set errno to zero before calling a library function known to set errno, and check errno only after the function returns a value indicating failureTake care when reading errno for more details.
Code Block | ||||
---|---|---|---|---|
| ||||
typedef struct object { /* Generic struct: contents don't matter */ int propertyA, propertyB, propertyC; } object_t; errno_t do_something(void){ FILE *fin1, *fin2; object_t *obj; errno_t ret_val; fin1 = fopen("some_file", "r"); if (fin1 == NULL) { return errno; } fin2 = fopen("some_other_file", "r"); if (fin2 == NULL) { fclose(fin1); return errno; } obj = malloc(sizeof(object_t)); if (obj == NULL) { ret_val = errno; fclose(fin1); return ret_val; /* Forgot to close fin2!! */ } /* ... More code ... */ fclose(fin1); fclose(fin2); free(obj); return NOERR; } |
This is just a small example; in much larger examples, errors like this are even harder to detect.
Compliant Solution (POSIX, Nested Ifs)
This compliant solution uses nested if statements to properly close files and free memory in the case that any error occurs. When the number of resources to manage is small (3 in this example), nested if statements will be simpler than a goto chain.
Code Block | ||||
---|---|---|---|---|
| ||||
/* ... Assume the same struct used previously ... */ errno_t do_something(void) { FILE *fin1, *fin2; object_t *obj; errno_t ret_val = NOERR; /* Initially assume a successful return value */ if ((fin1 = fopen("some_file", "r")) != NULL) { if ((fin2 = fopen("some_other_file", "r")) != NULL) { if ((obj = malloc(sizeof(object_t))) != NULL) { /* ... More code ... */ /* Clean-up & handle errors */ free(obj); } else { ret_val = errno; } fclose(fin2); } else { ret_val = errno; } fclose(fin1); } else { ret_val = errno; } return ret_val; } |
Compliant Solution (POSIX, Goto Chain)
Occasionally, the number of resources to manage in one function will be too large to permit using nested ifs to manage them.
In this revised version, a a goto
chain replaces chain replaces each individual return segment. If no error occurs, control flow falls through to the the SUCCESS
label label, releases all of the resources, and returns returns NOERR
. If an error occurs, the return value is set to to errno
, control flow jumps to the proper failure label, and the appropriate resources are released before returning.
...
This method is beneficial because the code is cleaner, and the programmer does not need to rewrite similar code upon every function error.
Note that this guideline does not advocate more general uses of goto, which is still considered harmful. The use of goto in these cases is specifically to transfer control within a single function body.
Compliant Solution (copy_process()
from Linux kernel)
...
Tool | Version | Checker | Description | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Klocwork |
| MLK.MIGHT MLK.MUST MLK.RET.MIGHT MLK.RET.MUST RH.LEAK | |||||||||||
LDRA tool suite |
| 50 D | Partially implemented | ||||||||||
Parasoft C/C++test |
| BD-RES-LEAK | CERT_C-MEM12-a | Ensure resources are freed | |||||||||
PC-lint Plus |
| 429 | Assistance provided | Implemented||||||||||
Polyspace Bug Finder | R2016a | Memory allocated dynamically not freed Lock function without unlock function File stream not closed before |
...
| Checks for memory leak and resource leak (rec. partially covered) |
Bibliography
Dijkstra, Edgar, "Go To Statement Considered Harmful.", 1968 | |
Linux Kernel Sourcecode (v2.6.xx) | 2.6.29, kernel/fork.c , the copy_process() Function |
[Seacord 2013] | Chapter 4, "Dynamic Memory Management" |
...