Many functions require the allocation of multiple resources. Failing and returning somewhere in the middle of this function without freeing all of the allocated resources could produce a memory leak. It is a common error to forget to free one (or all) of the resources in this manner, so a goto-chain is the simplest and cleanest way to organize exits when order is preserved.
Noncompliant Code Example
In this noncompliant example, exit code is written for every instance in which the function can terminate prematurely. Notice how failing to allocate obj2
produces a memory leak and fails to close the opened file.
typedef struct object { // A generic struct -- The contents don't matter int propertyA, propertyB, propertyC; } object_t; errno_t do_something(void){ FILE *fin; object_t *obj1, *obj2; errno_t ret_val; fin = fopen("some_file", "r"); if (fin == NULL){ return errno; } obj1 = malloc(sizeof(object_t)); if (obj1 == NULL){ ret_val = errno; fclose(fin); return ret_val; } obj2 = malloc(sizeof(object_t)); if (obj2 == NULL){ ret_val = errno; fclose(fin); return ret_val; // Forgot to free obj1 !! } // ... more code ... }
This is also just a small example; in much larger examples, errors like this would be even harder to detect.
Compliant Solution
In this revised version, we have used a goto-chain in replacement of each individual return segment. If there is no error, control flow will fall through to the SUCCESS
label, release all of the resources and return NOERR
. In the case of an error, the return value will be set to errno
, control flow will jump to the proper failure label, and the appropriate resources will be released before returning.
Please note that these examples assume errno_t
and NOERR
to be defined, as requested by [[DCL09-C. Declare functions that return an errno error code with a return type of errno_t]]. An equivalent compatible example would define errno_t
as an int
and NOERR
as zero.
// ... assume the same struct as above ... errno_t do_something(void){ FILE *fin; object_t *obj1, *obj2; errno_t ret_val = NOERR; // Initially assume a successful return value fin = fopen("some_file", "r"); if (fin == NULL){ ret_val = errno; goto FAIL_FIN; } obj1 = malloc(sizeof(object_t)); if (obj1 == NULL){ ret_val = errno; goto FAIL_OBJ1; } obj2 = malloc(sizeof(object_t)); if (obj2 == NULL){ ret_val = errno; goto FAIL_OBJ2; } // ... more code ... SUCCESS: // Free everything free(obj2); FAIL_OBJ2: // Otherwise, free only the objects we allocated free(obj1); FAIL_OBJ1: fclose(fin); FAIL_FIN: return ret_val; }
The benefits of this method are that the code is cleaner and we prevent the rewriting of similar code upon every function error.
Risk Assessment
Failure to free allocated memory or close opened files results in a memory leak and possibly unexpected results.
Recommendation |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
---|---|---|---|---|---|
MEM12-C |
low |
probable |
medium |
P3 |
L3 |
References
[[ISO/IEC 9899:1999]] Section 7.20.3, "Memory management functions"
[[ISO/IEC 9899:1999]] Section 7.19.5, "File access functions"
[[Seacord 05]] Chapter 4, "Dynamic Memory Management"