What is the correct way of freeing dynamically allocated memory if the program can exit early because of errors?

346 Views Asked by At

I have always learned that it is good to be generous with error checks in your code, and to provide an error message in case certain conditions are met. Consider a program of the following structure:

int main(const int argc, const char *argv[]) {

// read two integers M and N from input file 1 provided in argv[]
// read two integers K and L from input file 2 provided in argv[]

if (M*N < 1000000) {
    allocate array A
} else {
    printf("With your input values, the matrix will be too large!");
    return 1;
}

if (K*L < 1000000) {
    allocate array B
} else {
    printf("With your input values, the matrix will be too large!");
    return 1;
}

// multiply arrays elementwise
// free memory
return 0;
}

Ignore for the moment that this code could be reorganised, such that the validity of the input arguments is checked before allocations take place (which would be the straightforward solution in this simple example). My actual problem will be elaborated on down below for those who are interested.

If in the pseudocode above A is successfully allocated, but B is not because K*L exceeds the limit, the memory free statements at the end of the program are not reached, and a memory leak is associated with A. What would be the best way to avoid this issue if the code cannot be reorganised as described above? Removing the return 1; might cause other errors further down the line. The other two options I could think of are using the notorious goto statement (I did not dare), or indeed including all free calls inside the conditional as;

if (K*L < 1000000) {
    allocate array B
} else {
    printf("With your input values, the matrix will be too large!");
    free(A);
    return 1;
}

, but that would mean a lot of code repetition when one deals with a large number of dynamically allocated arrays. So: what program structure is recommended for a combination of intermediate error-checking and memory handling?

My actual case: I am working with CUDA, which (for those that are unfamiliar with it) allows one to write code to be executed on a GPU. All (or most) code associated with this API returns an error flag, and which it is recommended that it is checked before the program proceeds. In my case, at some point in my program I am allocating memory on the GPU like such (should be readable for those familiar with C):

double *dev_I;
cudaError_t err;
err = cudaMalloc(&dev_I, N*M, sizeof(*dev_I)); // note: cudaMalloc takes a double pointer
if (err != cudaSuccess) { printf("Error allocating dev_I.\n"); return 1; }

Here comes the important part: the next step is to copy memory from the host to a place where the GPU can access it. This looks somewhat like this:

err = cudaMemcpy(dev_I, host_I, M*N * sizeof(*host_I), cudaMemcpyHostToDevice);
if (err != cudaSuccess) { printf("Error copying dev_I"); return 1; }

And this is the part I am concerned about. Should the allocation be successful but the memory copy fail, the program will (should) exit, and I will be left with a memory leak. Hence my question how this is best dealt with.

2

There are 2 best solutions below

4
Kerrek SB On

General structured programming in C requires goto, as follows:

int f() {
  void *p = alloc_resource();
  if (p == NULL) goto error_1;

  void* q = alloc_resource();
  if (q == NULL) goto error_2;

  return g(p, q);  // g takes ownership

error_2:
  dealloc_resource(p);
error_1:
  return -1;  // some error code
}

You can see how this pattern generalizes to arbitrary numbers or ownership acquisitions: you basically have to keep a list of cleanup operations around and jump to the appropriate one when your control flow hits an error and all state that you've build up so far needs to be unwound.

Usually, one would try to avoid excessively complex functions by keeping each function small so that the cleanup can be done in a more ad-hoc way without gotos, but sometimes you need something structured like this (e.g. when you are composing multiple bits of ownership).

(Other languages offer language-level support for multiple exits, e.g. in C++ this logic is provided by so-called "destructors". They pretty much effect the same thing only in a more composable and reusable way with all the jumps generated by the compiler.)

1
chux - Reinstate Monica On

In addition to @Kerrek SB good answer, another approach is to assign/initialize all objects to some valid state, test the validity of all resources, use if ok, then free.

The key idea is at declaration of the object (e.g. bar *A = NULL), the object A is immediately given a valid value - some valid value.

int foo(void) {
  bar *A = NULL;
  bar *B = NULL;
  int error = 0;

  if (M*N < 1000000) {
    A = allocate(M, N);
  } else {
    printf("With your input values, the matrix will be too large!");
    error = 1;
  }

  if (K*L < 1000000) {
    B = allocate(K, L);
  } else {
    printf("With your input values, the matrix will be too large!");
    error = 1;
  }

  // Was resource allocation successful?
  if (A && B) {
   // multiply arrays
   mul(A,B);
  }

  // free resources
  free(A);
  free(B);  
  return error;
}

A pseudo-code outline

Get resources for A
Get resources for B
Get resources for C
If (OK(A) && OK(B) && OK(C)) {
  Perform the operation(A,B,C)
}
Return resources(C)
Return resources(B)
Return resources(A)

I find this approach easier to test.