What are some good ways of handling errors (cleanu

Question

First of all, if someone can reword the question to make it clearer, please do.

A common occurrence in C programming is having several resources to be initialized/allocated in a specific order. Each resource is a prerequisite for the initialization of subsequent ones. If one of the steps fails, the leftover resources from previous steps must be de-allocated. Ideal pseudocode (utilizing a magically generic pure-unobtainium clean_up_and_abort() function) would look approximately as follows:

err=alloc_a()
if(err)
    clean_up_and_abort()

err=alloc_b()
if(err)
    clean_up_and_abort()

err=alloc_c()
if(err)
    clean_up_and_abort()

// ...

profit()

I have seen several ways of handling this, all of them seem to have significant drawbacks, at least in terms of what people tend to consider "good practice".

What is are the most readable and least error-prone ways of structuring the code when handling this situation? Efficiency is preferred, but a reasonable amount of efficiency can be sacrificed for the sake of readability. Please list advantages and drawbacks. Answers discussing multiple methods are welcome.

The goal is to hopefully end up with a set of several preferred methods for solving this problem.

I'll start with some of the methods I've already seen, please comment on them and add others.

Answer 1

The three most common methods I've seen so far:

1: Nested if-statements (without multiple returns for the SESE purists). With a long chain of prerequisites, this gets out of hand fast. IMO, even in simple cases this is a readability disaster and has no real advantages. I am including it because I see people do this (too) often.

uint32_t init_function() {
    uint32_t erra, errb, errc, status;
    A *a;
    B *b;
    C *c;

    erra = alloc_a(&a);
    if(erra) {
        status = INIT_FAIL_A;
    } else {

        errb = alloc_b(&b);
        if(errb) {
            dealloc_a(&a);
            status = INIT_FAIL_B;
        } else {

            errc = alloc_c();
            if(errc) {
                dealloc_b(&b);
                dealloc_a(&a);
                status = INIT_FAIL_C;
            } else {

                profit(a,b,c);
                status = INIT_SUCCESS;

            }
        }
    }
    // Single return.
    return status;
}

2: Multiple returns. This is my preferred method right now. THe logic is easy to follow but it's still dangerous because cleanup code has to be duplicated and it's easy to forget to deallocate something in one of the cleanup sections.

uint32_t init_function() {
    uint32_t err;
    A *a;
    B *b;
    C *c;

    err = alloc_a(&a);
    if(err) {
        return INIT_FAIL_A;
    }

    err = alloc_b(&b);
    if(err) {
        dealloc_a(&a);
        return INIT_FAIL_B;
    }

    err = alloc_c(&c);
    if(err) {
        dealloc_b(&b);
        dealloc_a(&a);
        return INIT_FAIL_C;
    }

    profit(a,b,c);
    return INIT_SUCCESS;
}

3: GOTO. Many people don't like goto on principle, but this is one of the standard arguments for a valid use of goto in C programming. The advantage is that it's hard to forget a cleanup step and there is no copypasting.

uint32_t init_function() {
    uint32_t status;
    uint32_t err;
    A *a;
    B *b;
    C *c;

    err = alloc_a(&a);
    if(err) {
        status = INIT_FAIL_A;
        goto LBL_FAIL_A;
    }

    err = alloc_b(&b);
    if(err) {
        status = INIT_FAIL_B;
        goto LBL_FAIL_B;
    }

    err = alloc_c(&c);
    if(err) {
        status = INIT_FAIL_C;
        goto LBL_FAIL_C;
    }

    profit(a,b,c);
    status = INIT_SUCCESS;
    goto LBL_SUCCESS;

    LBL_FAIL_C:
    dealloc_b(&b);

    LBL_FAIL_B:
    dealloc_a(&a);

    LBL_FAIL_A:
    LBL_SUCCESS:

    return status;
}

Anything else I did not mention?

Answer 2

4: Global variables, woohoo!!! Because everybody loves global variables, just like they love goto. But seriously, if you limit the scope of the variables to file scope (using the static keyword) then it's not that bad. Side note: the cleanup function takes/returns the error code unchanged, so as to declutter the code in the init_function.

static A *a = NULL;
static B *b = NULL;
static C *c = NULL;

uint32_t cleanup( uint32_t errcode )
{
    if ( c )
        dealloc_c(&c);
    if ( b )
        dealloc_b(&b);
    if ( a )
        dealloc_a(&a);

    return errcode;
}

uint32_t init_function( void )
{
    if ( alloc_a(&a) != SUCCESS )
        return cleanup(INIT_FAIL_A);

    if ( alloc_b(&b) != SUCCESS )
        return cleanup(INIT_FAIL_B);

    if ( alloc_c(&c) != SUCCESS )
        return cleanup(INIT_FAIL_C);

    profit(a,b,c);
    return INIT_SUCCESS;
}

5: Faux OOP. For those who can't handle the truth (that global variables are actually useful in C programs), you can take the C++ approach. C++ takes all of the global variables, puts them into a structure, and calls them "member" variables. And somehow that makes everybody happy.

The trick is to pass a pointer to the structure to all of the functions, as the first argument. C++ does this behind the scenes, in C you have to do it explicitly. I call the pointer that so as to avoid conflicts/confusion with this.

// define a class (uhm, struct) with status, a cleanup method, and other stuff as needed
typedef struct stResources
{
    char *status;
    A *a;
    B *b;
    C *c;
    void (*cleanup)(struct stResources *that);
}
stResources;

// the cleanup method frees all resources, and frees the struct
void cleanup( stResources *that )
{
    if ( that->c )
        dealloc_c( &that->c );
    if ( that->b )
        dealloc_b( &that->b );
    if ( that->a )
        dealloc_a( &that->a );

    free( that );
}

// the init function returns a pointer to the struct, or NULL if the calloc fails
// the status member variable indicates whether initialization succeeded, NULL is success
stResources *init_function( void )
{
    stResources *that = calloc( 1, sizeof(stResources) );

    if ( !that )
        return NULL;

    that->cleanup = cleanup;

    that->status = "Item A is out to lunch";
    if ( alloc_a( &that->a ) != SUCCESS )
        return that;

    that->status = "Item B is never available when you need it";
    if ( alloc_b( &that->b ) != SUCCESS )
        return that;

    that->status = "Item C is being hogged by some other process";
    if ( alloc_c( &that->c ) != SUCCESS )
        return that;

    that->status = NULL;  // NULL is success
    return that;
}

int main( void )
{
    // create the resources
    stResources *resources = init_function();

    // use the resources
    if ( !resources )
        printf( "Buy more memory already\n" );
    else if ( resources->status != NULL )
        printf( "Uhm yeah, so here's the deal: %s\n", resources->status );
    else
        profit( resources->a, resources->b, resources->c );

    // free the resources
    if ( resources )
        resources->cleanup( resources );
}