[SOLVED] Good practice - lot of to do at the function termination

Good practice - lot of to do at the function termination - goto alternative

Possible Duplicate:
Valid use of goto for error management in C?

Recently I've encountered C code like this:

if( !condition1){
    goto failure;
}

some_stuff_that_needs_manual_undoing();

if( !condition2){
    goto failure;
}

// And many more lines such as the ones above
return 0;

failure:
// Do a lot of stuff here, free, file closing and so on
return -1;

To sum up the situation:

I have a long function that do several things in a row (let's say open a file, then allocate memory based on file contents, then connect do database and so so on). Of course I want to free resources correctly, but there are lot of places which can cause function to end prematurely (and all of them need clean up).

The Question: How to do this correctly?

Although goto doesn't seem to be that bad practice it also doesn't seem to be good solution.

I though of following:

Use a macro which would do the job, e.g.:

#define CLEANUP if( memory != NULL)free(memory); \
                if( fp != NULL) fclose(fp);\
                // ...

if( !condition1){
    CLEANUP
    return -1;
}

if( !condition2){
    CLEANUP
    return -2;
}

// ...

This will result in duplicate assembly, but the cleanup code would be in one place.

Encapsulate function into another function

int _some_stuff_do_work(void **memory, FILE **file, ...){
    // Would just return on error
}

int some_stuff() {
    void *memory = NULL;
    FILE *file = NULL;

    _some_stuff_do_work( &memory, &file, ...);
    if( fp) fclose(fp);
}

This could get really ugly if there will be more than 3-5 things that will need cleaning up (the function would then take many arguments and that always calls for a problems).

OOP - Destructor

typedef struct {
    void *memory;
    FILE *fp;
} LOCAL_DATA;

// Destructor
void local_data_destroy( LOCAL_DATA *data)
{
    if( data->fp){
        free(data->fp);
        data->fp = NULL;
    }
}

But this could lead to many functions (and structures) that would be used only once in whole application and it seems like it could create a hell of a mass.

Loop and break statements

while(1){
    if( !condition1){
       break;
    }

    // ...

    break;
}

if( fp) fclose(fp);

I found this on may places but using one iteration loop? I don't know, it seems to be completely un-intuitive.

Solution

Goto is the way to go. First understand why exactly "goto is bad" and then you will see that in situation you're describing goto is not really bad. Avoiding goto at all costs is just wrong and comes from shallow understanding of good programming principles.

This is way to go (Linux kernel source): http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a%3Dblob;f%3Dfs/block_dev.c;h%3D38e721b35d45388cb0febed8021a02277a4a2f1b;hb%3DHEAD#l887