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.

CodePudding user response：

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

CodePudding user response：

I was wondering where should I free some of the allocated memory

It is a good question, but it is impossible for you to get a very answer here.

You must take a holistic view of your own code and define your own invariants. From my experience I would do so.

You define some invariants at some points (maybe also insert some assertions) and you know that at those points you are right. You also must provide a proof that the points you choose for invariants are correct.

however the use of goto is discouraged

The use of goto is not discouraged. It is commonly used in critical parts of many systems you use daily. The expression comes from Dijkstra, but he was not against goto. Without goto it would be hard to program many things.

CodePudding user response：

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