How can I correctly handle malloc failure in C, especially when there is more than one malloc?

Question

Suppose this is a part of my code:

 int foo()  {       char *p, *q ;     if((p = malloc(BUFSIZ)) == NULL) {         return ERROR_CODE;     }     if((q = malloc(BUFSIZ)) == NULL) {         free(p)         return ERROR_CODE;     }     /* Do some other work... */      free(p);     free(q);    } 

Since it's possible that the first malloc is successful but the second one fails, I use free(p) in the second "error handler". But what if there are more malloc's and what if I want to modify the code (adjusting their orders, adding or deleting some malloc)?

I know in C++ there are things like RAII and exception safe, etc. But in general, what is the correct way to handle malloc failure in C? (maybe using some goto?)

Answer 1

Your code is fine, but for lots of variables, I'd prefer:

int foo() {     char *p = NULL;     char *q = NULL;     int ret = 0;      if (NULL == (p = malloc(BUFSIZ)))     {         ret = ERROR_CODE;         goto error;     }      // possibly do something here      if (NULL == (q = malloc(BUFSIZ)))     {         ret = ERROR_CODE;         goto error;     }      // insert similar repetitions      // hopefully do something here    error:     free (p);     free (q);     return ret; } 

Note that freeing NULL is defined as a no-op.

This avoids n levels of indent for n variables. You can clean up filehandles etc. similarly (though you'll have to put a condition around the close()).

Now, if you know you can allocate them all at once, then dasblinkenlight has a good answer, but here's another way:

int foo() {     int ret = 0;     char *p = malloc(BUFSIZ);     char *q = malloc(BUFSIZ);     char *r = malloc(BUFSIZ);     if (!p || !q || !r)     {         ret = ERROR_CODE;         goto exit;     }      // do something    exit:     free(p);     free(q);     free(r);     return ret; } 

Final possibility: if you actually want to exit the program on malloc fail, consider using mallopt's M_CHECK_ACTION option. This makes malloc() faults get checked, and calls abort(), possibly printing a helpful message.

From the man page:

NAME

mallopt - set memory allocation parameters

SYNOPSIS

  #include <malloc.h>    int mallopt(int param, int value); 

DESCRIPTION

The mallopt() function adjusts parameters that control the behavior of the memory-allocation functions (see malloc(3)). The param argument specifies the parameter to be modified, and value specifies the new value for that parameter.

The following values can be specified for param:

M_CHECK_ACTION

Setting this parameter controls how glibc responds when various kinds of programming errors are detected (e.g., freeing the same pointer twice). The 3 least significant bits (2, 1, and 0) of the value assigned to this parameter determine the glibc behavior, as follows:

Bit 0: If this bit is set, then print a one-line message on stderr that provides details about the error. The message starts with the string "*** glibc detected ***", followed by the program name, the name of the memory-allocation function in which the error was detected, a brief description of the error, and the memory address where the error was detected.

Bit 1: If this bit is set, then, after printing any error message specified by bit 0, the program is terminated by calling abort(3). In glibc versions since 2.4, if bit 0 is also set, then, between printing the error message and aborting, the program also prints a stack trace in the manner of backtrace(3), and prints the process's memory mapping in the style of /proc/[pid]/maps (see proc(5)).

Bit 2: (since glibc 2.4) This bit has an effect only if bit 0 is also set. If this bit is set, then the one-line message describing the error is simplified to contain just the name of the function where the error was detected and the brief description of the error.

Answer 2

Since it is perfectly OK to pass NULL to free(), you could allocate everything that you need in a "straight line", check everything in a single shot, and then free everything once you are done, regardless of whether or not you have actually done any work:

char *p = malloc(BUFSIZ); char *q = malloc(BUFSIZ); char *r = malloc(BUFSIZ); if (p && q && r) {     /* Do some other work... */ } free(p); free(q); free(r); 

This works as long as there are no intermediate dependencies, i.e. you do not have structures with multi-level dependencies. When you do, it is a good idea to define a function for freeing such a structure, without assuming that all memory blocks are non-NULL.