Where to free multiple mallocs

Question

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.

This is a bit of an opinions based question. But IMHO that use of `goto` to jump to common clean up is one of the few cases where it is conventionally acceptable. — kaylum, Feb 07 '22 at 21:43
"the use of `goto` is discouraged" -- discouraged by whom? This is actually a pretty common idiom in C, and is probably worth considering unless your coding standard specifically forbids it. — Fred Larson, Feb 07 '22 at 21:43
goto is typical here https://stackoverflow.com/questions/570317/what-is-the-best-way-to-free-memory-after-returning-from-an-error — Passerby, Feb 07 '22 at 21:43
But don't forget to initialize all the variables to `NULL` at the beginning. — Barmar, Feb 07 '22 at 21:43
If you're exiting the program when an error happens, you could simply not bother, since all memory is freed when the program exits. — Barmar, Feb 07 '22 at 21:44

dbush · Accepted Answer · 2022-02-07T21:56:55.353

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();
    }
}

When using this technique, be careful not to jump over the initialization of a local variable and then use that variable, it would be easy to accidentally mix up the labels. Another way to err on the side of caution is to declare and initialize to null all the pointers at the start; and only have the single cleanup label at the end — M.M, Feb 07 '22 at 22:39

score 0 · Answer 2 · answered Feb 07 '22 at 22:43

Another approach is to use a wrapper function, e.g.:

void f()
{
    int *a = malloc(n * sizeof *a);
    int *b = malloc(n2 * sizeof *b);
    int *c = malloc(n3 * sizeof *c);
    bool success = a && b && c;

    if ( success )
        do_stuff(a, b, c);

    free(c);
    free(b);
    free(a);

    if ( !success )
        report_error_and_exit();
}

Where to free multiple mallocs

2 Answers2