Memory cleanup in functions with many return points

Question

I often have functions which allocate memory for a bunch of variables. Each malloc call may fail, and checking for each return value causes my code to get too long. Here's an example:

int some_func(size_t len1, size_t len2, size_t len3)
{
       char *a;
       struct someStruct *p1;
       struct otherStruct *p2;

       /* Just to avoid values of 0 in malloc */
       if ((len1 == 0) || (len2 == 0) || (len3 == 0)) {
               printf("Must give positive parameters.");
               return -1;
       }

       a = malloc(len1);

       if (a == NULL) {
               printf("malloc failed.");
               return -1;
       }

       p1 = malloc(len2);

       if (p1 == NULL) {
               printf("malloc failed.");
               free(a);
               return -1;
       }

       p2 = malloc(len2);

       if (p2 == NULL) {
               printf("malloc failed.");
               free(a);
               free(p1);
               return -1;
       }

       /* ... do actually useful stuff ... */

       free(a);
       free(p1);
       free(p2);
       return 0;
} 

Here, every single time I call malloc, I also need to add the if-statement and free every variable which was previously allocated. Keeping track of this eventually becomes tedious, and many lines of code get repeated (all the previous free() calls are repeated in the next malloc() error block).

I would think this is a common problem, so how do people usually deal with this? The other solution I see is using a "goto" to some cleanup code that frees everything. But this may lead to complications if some variables get freed before that, during normal execution.

Answer 1

I do it like something like this:

int some_func(size_t len1, size_t len2, size_t len3)
{
   int rCode=0;
   char *a = NULL;
   struct someStruct *p1 = NULL;
   struct otherStruct *p2 = NULL;

   /* Just to avoid values of 0 in malloc */
   if ((len1 == 0) || (len2 == 0) || (len3 == 0)) {
      printf("Must give positive parameters.");
      rCode=(-1);
      goto CLEANUP;
   }

   a = malloc(len1);
   if (a == NULL) {
      printf("malloc failed.");
      rCode=(-1);
      goto CLEANUP;
   }

   p1 = malloc(len2);
   if (p1 == NULL) {
      printf("malloc failed.");
      rCode=(-1);
      goto CLEANUP;
   }

   p2 = malloc(len2);
   if (p2 == NULL) {
      printf("malloc failed.");
      rCode=-1;
      goto CLEANUP;
   }

   /* ... do actually useful stuff ... */


CLEANUP:

   if(a)
      free(a);

   if(p1)
      free(p1);

   if(p2)
      free(p2);

   return(rCode);
} 

But perhaps, more like this:

int some_func(size_t len1, size_t len2, size_t len3)
   {
   int                 rCode = 0;
   char               *a     = NULL;
   struct someStruct  *p1    = NULL;
   struct otherStruct *p2    = NULL;

   /* Just to avoid values of 0 in malloc */
   if((0 == len1) || (0 == len2) || (0 == len3)) 
      {
      printf("Must give positive parameters.");
      rCode=EINVAL;
      goto CLEANUP;
      }

   errno=0;
   a = malloc(len1);
   if(NULL == a) 
      {
      rCode=errno?errno:ENOMEM;
      printf("malloc failed.  errno:%d", errno);
      goto CLEANUP;
      }

   errno=0;
   p1 = malloc(len2);
   if(NULL == p1) 
      {
      rCode=errno?errno:ENOMEM;
      printf("malloc failed.  errno:%d", errno);
      goto CLEANUP;
      }

   errno=0;
   p2 = malloc(len2);
   if(NULL == p2) 
      {
      rCode=errno?errno:ENOMEM;
      printf("malloc failed.  errno:%d", errno);
      goto CLEANUP;
      }

   /* ... do actually useful stuff ... */


CLEANUP:

   if(a)
      free(a);

   if(p1)
      free(p1);

   if(p2)
      free(p2);

   return(rCode);
   } 

BONUS Mahonri's list of rules for writing maintainable code