Is there a best practice for allocation/deallocating multiple, dynamic arrays in C?

Question

I was wondering what would be the best approach to allocate/deallocate multiple one-dimensional, dynamic arrays in C. This seems easy at first, however, for me it turned out to be problematic. Consider the following example program, which illustrates my problem:

#include <stdio.h>
#include <stdlib.h>

#define N 10

int my_func(size_t n);


int main(void)
{   
    int ret = my_func(N);
    printf("%d\n", ret); 
    return 0;
}


int my_func(size_t n)
{

    double *x = malloc(n * sizeof (double));
    if (x==NULL) return 0;

    double *y = malloc(n * sizeof (double));
    if (y==NULL) { free(x); return 0; }

    double *z = malloc(n * sizeof (double));
    if (z==NULL) { free(x); free(y); return 0;}

    /* some computations */

    free(x); free(y); free(z);
    return 1;
}

Allocation and checking of x, is straightforward. However, if the allocation of y fails, one has to free x. If there is a further array, like z in the example, and its allocation fails, one has to care about x and y. And as always, all memory has to be freed at the end of the function. Generally, checking for a failed allocation somewhere in the program requires to take care of all previously allocated memory blocks.

I thought it would be "easier" to automate the deallocation of all previously allocated memory blocks, so I came up with the following implementation that uses a "memory registry" to store pointers to the allocated memory blocks.

#include <stdio.h>
#include <stdlib.h>

#define N 10
#define N_BLOCKS 3

double *mem_reg[] = { NULL, NULL, NULL };

void clean_up(void);
int my_func(size_t n);


int main(void)
{   
    int ret = my_func(N);
    printf("%d\n", ret); 
    return 0;
}


int my_func(size_t n)
{
    double *x = malloc(n * sizeof (double));
    if (x!=NULL) mem_reg[0] = x; else goto fail;

    double *y = malloc(n * sizeof (double));
    if (x!=NULL) mem_reg[1] = y; else goto fail;

    double *z = malloc(n * sizeof (double));
    if (x!=NULL) mem_reg[2] = z; else goto fail;

    /* some computations */

    clean_up();
    return 1;

fail:
    clean_up();
    return 0;
}

void clean_up(void)
{
    for (size_t i=0; i<N_BLOCKS; i++)
    {
        if (mem_reg[i] != NULL)
            free(mem_reg[i]);
    }
}

The advantage of the second example is that there is one single exit point in my_func defined under the label fail. So even if I had to check for some errors in the computations (not shown in the examples), I could simply goto fail in order to easily clean up all dynamic allocations. The disadvantage is that the code of the second example is not easier than the code of the first example.

You may have recognized that I am not so much experienced in C, hence this question. Is there an advantage in one of the both examples, that I didn't see? Then which one is to be preferred. Or is there even a better way?

Answer 1

Serious answer: use C++ so that you can benefit from RAII. You can mix C and C++ freely within your codebase, and keep C compatibility by declaring your functions as extern "C". But C++ may not be viable for various reasons.

In that case, return to this almost-outdated advice: a function should have one entry point and one exit point only. I.e., exactly one return. Any resources are allocated in a nice linear fashion, and released in reverse order before this return. That means that where your existing code returns directly, it should set a variable to the return value and goto the cleanup section.

Is goto harmful? Not here, where it is necessary to implement behaviour that is unavailable using the normal utilities of the language. (But note thst C++'s RAII is exactly such an utility that makes this use of goto unnecessary.)

int my_func(size_t n)
{
  double *x = NULL;
  double *y = NULL;
  double *z = NULL;
  int ok = 1;

  x = malloc(n * sizeof(*x));
  if (x == NULL) { ok = 0; goto cleanup; }

  y = malloc(n * sizeof(*y));
  if (y == NULL) { ok = 0; goto cleanup; }

  z = malloc(n * sizeof(*z));
  if (z == NULL) { ok = 0; goto cleanup; }

  /* some computations */

cleanup:
  free(z); free(y); free(x);
  return ok;
}

Answer 2

Calling free(NULL) is harmless in any correct implementation of C.

So the simplest solution is to malloc all three arrays. Check that all three are not null before performing the calculation. Then free all three pointers at the end.

Answer 3

Your method is doing too much things. You:

allocate memory
perform operations
handle errors
deallocate memory
return a result

Imagine your code if every method did all this. It's not maintainable, so you split it up. Allocating memory is not trivial to wrap, so leave that in place. But the operations and error handling needs to go. Write a separate method that takes in three double*s and performs operations on them, returning a meaningful result (so either the literal result of the calculation, or a success/error code).

// keep your method
int my_func(size_t n) {
    // allocation will always be ugly in C
    double *x = malloc(n * sizeof (double));
    double *y = malloc(n * sizeof (double));
    double *z = malloc(n * sizeof (double));

    // operations should have their own methods
    int foo = my_computations(x, y, z);

    // with allocation comes cleaning duty
    free(x);
    free(y);
    free(z);

    // methods should return a result
    return foo;
}

// this method now assumes correct allocation
int my_computations(double* x, double* y, double* z) {
    if (x==NULL || x==NULL || x==NULL) { return NOT_OK; }

    /* some computations */

    return OK;
}

This is a small change, but it keeps your requirements separate from the computation. It allows you to freely expand the computation method, use it from other methods, write tests for it, etc. All without having to deal with the allocation and deallocation part. It also solves your problem, and protects you from it in the future.

Answer 4

The second approach almost gets you to the right place. Take a look at your code and you'll notice a repeating pattern that amounts to "allocate the next block and free any previously-allocated blocks on failure." That's an algorithm ripe for turning into a general-purpose function. You're not the first to bump into this problem, and I happen to have a one in my toolbox that covers it (and beats the inevitable wordy expanation):

// Allocate multiple blocks of memory, making sure no allocated
// block remains unfreed in the event of a failure.  Returns 0
// on success, nonzero otherwise.  Nothing placed in 'results'
// should be considered valid on failure.

int multi_malloc(
  size_t blocks,        // Number of blocks
  size_t *block_sizes,  // Size of each block
  void **results        // Where to put the allocated pointers
  )
{
  assert(blocks > 0);
  assert(block_sizes != NULL);
  assert(results != NULL);

  size_t block;

  for ( block = 0 ; block < blocks; ++block ) {
    void *allocated = malloc(block_sizes[block]);
    if ( allocated == NULL ) {
      break;
    }
    results[block] = allocated;
  }

  // Declare success if all blocks were allocated
  if ( block == blocks ) {
    return 0;
  }

  // Otherwise, free allocated memory and declare failure.
  while ( block > 0 ) {
    --block;
    assert(results[block] != NULL);
    free(results[block]);
  }
  assert(block == 0);

  return 1;
}

Putting this activity into a function means not having to write the same code again once you have it correct. More importantly, this approach works for an arbitrarily-large number of blocks with an arbitrary number of different types without getting cumbersome.

Calling it requires a bit more in the way of gymnastics but is a decent trade-off for being able to have the entire set of allocations succeed or fail as a group.

#define ELEMENTS 100
#define LENGTH_OF(x) ( (sizeof(x)) / (sizeof(*(x))) )

int main(int argc, char **argv)
{
  // Size of each block with one differing type just for grins.
  size_t blocks[] = {
    (ELEMENTS * sizeof(double)),
    (ELEMENTS * sizeof(double)),
    (ELEMENTS * sizeof(int))
  };

  void *memory[LENGTH_OF(blocks)];

  if ( multi_malloc( LENGTH_OF(blocks), blocks, memory ) ) {
    puts("Allocation failed.");
    exit(1);
  }

  double *x = (double *)(memory[0]);
  double *y = (double *)(memory[1]);
  int    *z = (int    *)(memory[2]);

  // ...Computations...

  multi_free(LENGTH_OF(blocks), memory);
}

I'll leave writing multi_free() as an exercise for you.

Answer 5

Just to be a bit weird and different from the existing answers, when the types are homogeneous I would almost certainly do this:

double* xyz = malloc(n*3 * sizeof *xyz);
if (xyz)
{
    double* x = xyz;
    double* y = x + n;
    double* z = y + n;
    ...
    free(xyz);
    return success;
}
return error;

I might not bother if the types aren't homogeneous and have different alignment requirements, but I'd almost certainly do the above if they're all dynamic arrays of double and not only simplify the code a bit that way but also reduce the number of superfluous heap allocations and deallocations.

I use C these days in very small doses for small, very core-level parts of the system which often just deal with bits and bytes (ex: lowest-level memory allocator implementations which don't benefit much at all from type safety as provided in, say, C++, and where exception handling might complexify instead of simplify the implementation) and tend to be performance-critical, so I generally want to put in a little bit extra effort upfront to keep the heap allocations/deallocations to a minimum typically along with preserving spatial locality and so forth in the rare cases when I reach for C.

If they're mixed like:

int* x = malloc(...);
double* y = malloc(...);
char* z = malloc(...);

Then I tend to just do either like this:

if (x && y && z)
{
    ...
}
free(x);
free(y);
free(z);

That's not necessarily less efficient practically speaking if you can allocate those in advance since it's not necessarily doing any more work in the non-exceptional execution paths when all three allocations succeed.

... or use goto cleanup label at bottom (which I agree with many other answers is a reasonable practice in C to reduce probability of human error).

I thought it would be "easier" to automate the deallocation of all previously allocated memory blocks, so I came up with the following implementation that uses a "memory registry" to store pointers to the allocated memory blocks.

If you're tempted to generalize resource management this way, I would absolutely echo the suggestion to use C++.

Answer 6

If you can first allocate all you need, do the computations, and then release those resources, consider whether splitting it into two functions might help you:

int my_func(size_t n)
{
    double *x, *y, *z;
    int r = 0;
    if ((x = malloc(n * sizeof *x))) {
        if ((y = malloc(n * sizeof *y))) {
            if ((z = malloc(n * sizeof *z))) {
                r = my_func_impl(x, y, z, n);
                free(z);
            }
            free(y);
        }
        free(x);
    }
    return r;
}

You can simplify things by accepting the possible slight penalty for free(NULL):

int my_func(size_t n)
{
    double *x = 0, *y = 0, *z = 0;
    int r = 0;
    if ((x = malloc(n * sizeof *x))
    && (y = malloc(n * sizeof *y))
    && (z = malloc(n * sizeof *z)))
        r = my_func_impl(x, y, z, n);
    free(z);
    free(y);
    free(x);
    return r;
}

Or preferably, coalesce those allocations:

int my_func(size_t n)
{
    double *x = malloc(3 * n * sizeof *x);
    if (x) {
        double *y = x + n;
        double *z = y + n;
        int r = my_func_impl(x, y, z, n);
        free(x);
    }
    return r;
}

Naturally, if there are different types of resources, coalescing is impossible. Judicious use of goto is not a sin in C.

You might wonder why I abhor sizeof(TYPE). The problem is that that the assignee's type and the type its sized for might change independently. Using sizeof *pointer instead strongly couples them.

Answer 7

I had a similar problem when I was writing a program that had to allocate a lot of strings, do some processing, and then deallocate the strings.

I ended up writing a simple pooled memory allocator. I'd ask the allocator for a string of a particular size. The allocator would allocate a large block if necessary, and then give me a pointer to the appropriate location in the block, using that block for allocations until it was filled. The blocks were chained together, and at the end I had a single call to deallocate all of the blocks.

One side advantage was that I marked the start of free space each time I allocated, and checked that I hadn't had a buffer overrun on the next allocation. In this particular case I had a pretty good idea what size strings I would be dealing with so it was pretty straightforward to pick a block size that would enable me to allocate a bunch of strings.

It's a little more work to do a general solution but then it's in your toolbox for whenever you need it.