Search code examples
cmallocfreegoto

Where to free multiple mallocs


I have the following piece of code:

void f(size_t n) {
    int *a = malloc(n * sizeof *a);
    if(a == NULL) {
        report_error_and_exit();
    }

    int *b = malloc(n * sizeof *a);

    if(b == NULL) {
        free(a);
        report_error_and_exit();
    }

    int *c = malloc(n * sizeof *a);
    if(c == NULL) {
        free(a);
        free(b);
        report_error_and_exit();
    }

    /*use a, b, c*/
}

or something along those lines. Basically, I need multiple mallocs, and all of them mustn't fail.

I was wondering where should I free some of the allocated memory. As the function gets longer and bigger checking for malloc fails gets more messy.

A possible solution I was thinking was doing something like the following:

void f(size_t n) {
    int *a = malloc(n * sizeof *a);
    if(a == NULL) {
        goto malloc_fail;
    }

    /*...*/

malloc_fail:
    free(a);
    free(b);
    /*...*/
    report_error_and_exit();
}

however the use of goto is discouraged.

I am wondering what is the appropriate solution to this.


Solution

  • This is actually a classic example of a proper use of goto in C.

    While it is possible to abuse goto (and badly), one of the cases where it does make sense is in error handling code such as this. It only jumps forward, makes the code easier to read, and puts all of the error handling in one place so it's less error prone.

    One thing you have to watch however is that if you jump over the initialization of a variable, then that variable is uninitialized. Given your example, if the first call to malloc which initializes a fails, then your code as currently written will call free on b and c which are not initialized.

    This can be fixed by having a different jump destination for each failure and doing the cleanup in the reverse order.

    void f(size_t n) {
        int success = 0;
    
        int *a = malloc(n * sizeof *a);
        if(a == NULL) {
            goto malloc_fail_a;
        }
        int *b = malloc(n * sizeof *b);
        if(b == NULL) {
            goto malloc_fail_b;
        }
        int *c = malloc(n * sizeof *c);
        if(c == NULL) {
            goto malloc_fail_c;
        }
    
        /*...*/
    
        success = 1;
    
    malloc_fail_c:
        free(c);
    malloc_fail_b:
        free(b);
    malloc_fail_a:
        free(a);
        if (!success) {
            report_error_and_exit();
        }
    }