Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

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 while preserving the order of freed resources.

Noncompliant Code Example (POSIX)

In this noncompliant example, exit code is written for every instance in which the function can terminate prematurely. Notice how failing to close fin2 produces a resource leak, leaving an open file descriptor.

Please note that these examples assume errno_t and NOERR to be defined, as recommended in DCL09-C. Declare functions that return errno with a return type of errno_t. An equivalent compatible example would define errno_t as an int and NOERR as zero.

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. Take care when reading errno for more details.

Code Block
bgColor#FFCCCC
langc
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
bgColor#CCCCFF
langc
/* ... 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 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)

...

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

P4

L3

Automated Detection

Tool

Version

Checker

Description

Klocwork
Include Page
Klocwork_V
Klocwork_V
MLK.MIGHT
MLK.MUST
MLK.RET.MIGHT
MLK.RET.MUST
RH.LEAK

LDRA tool suite
Include Page
LDRA_V
LDRA_V
50 DPartially implemented
Parasoft C/C++test
Include Page
Parasoft_V
Parasoft_V

CERT_C-MEM12-a

Ensure resources are freed

PC-lint Plus

Include Page
PC-lint Plus_V
PC-lint Plus_V

429

Assistance provided

Polyspace Bug Finder

Include Page
Polyspace Bug Finder_V
Polyspace Bug Finder_V

CERT C: Rec. MEM12-C


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"

...


...

Image Modified Image Modified Image Modified