CODEWITHSUNDEEP
cloopserror-handlinggoto
user3758232
user3758232

Reputation: 882

Free memory in a loop regardless of return code

I have the following C pseudo code:

int main()
{
    int rc = 0;

    for (int i = 1; i < 10; i++) {
        char *data = malloc(i); // Each iteration has a different alloc size

        rc = do_something(data);
        if (rc != 0) goto cleanup;

        rc = do_something_else();
        if (rc != 0) goto cleanup;

cleanup:
        free(data);
        if (rc != 0) break;
    }
    return rc;
}

I want to emulate Python's try...finally pattern by breaking out of a loop if a called function returns an error, but only after doing some necessary cleanup work.

So far the code looks OK to me but I'm new to C. Is there a different pattern that avoids the repeated rc != 0 test after free? (I am aware that some people consider goto unconditionally wrong but I see it as the cleanest solution I found for this case.)

Upvotes: 2

Views: 172

Answers (4)

Adrian Mole
Adrian Mole

Reputation: 51864

In the specific code/case you have shown, you can remove the need for any sort of per-loop clean-up by using the realloc function instead of malloc. This will simply replace the memory allocated on the previous loop (if there is one) with a new block (of different size). You can then simply defer any clean-up (i.e. calling free) to outside the loop.

You can still use the break statements to exit the loop whenever an error occurs; or, as mentioned in the comments, you could add an rc != 0 test to the loop's condition.

Here's some C code that will do as I have indicated (but, of course, you will need actual definitions of the two called fucntions for it to work):

#include <stdlib.h>
int do_something(char* data);
int do_something_else(void);

int main()
{
    int rc = 0;
    char* data = NULL;
    for (size_t i = 1; i < 10; i++) {
        char* temp = realloc(data, i); // If "data" is NULL (1st loop) -> same as malloc
        if (temp == NULL) { // Allocation failure...
            // Error message
            break;
        }
        data = temp; // Succesfull allocation, so update "data" pointer

        rc = do_something(data);
        if (rc != 0) break;

        rc = do_something_else();
    }
    free(data); // We now only have to do the cleanup once (calling with NULL is allowed).
    return rc;
}

Feel free to ask for further clarification and/or explanation.

Upvotes: 3

Barmar
Barmar

Reputation: 781741

You can declare the variable outside the loop. Then just null the variable whenever you free it, and check for this after the loop.

    char *data = NULL;
    for (int i = 1; i < 10; i++) {
        data = malloc(i); // Each iteration has a different alloc size

        rc = do_something(data);
        if (rc != 0) break;

        rc = do_something_else();
        if (rc != 0) break;

        free(data);
        data = NULL;
    }
    if (data) {
        free(data);
    }

Upvotes: 2

Alex Lop.
Alex Lop.

Reputation: 6875

It is OK to use such pattern (with goto) in cases when resource management is required and there is a set of checks to be performed while each failure requires to stop the flow and perform the required cleanup.

To make the code cleaner, in my opinion, would be to place the rc != 0 condition inside the for loop:

int rc = 1;
for (int i = 0; i < 10 && rc != 0; i++)
{
    ...

In such case you won't need the break at all as the loop would stop anyway once rc is compared to 0. I would also recommend check the resut of malloc:

int foo(void)
{
    int rc = 1;

    for (int i = 1; i < 10 && rc != 0; i++) {
        char *data = malloc(i); // Each iteration has a different alloc size
        if (!(rc = !!data)) goto cleanup;

        rc = do_something(data);
        if (rc != 0) goto cleanup;

        rc = do_something_else();
        if (rc != 0) goto cleanup;

cleanup:
        free(data);
    }

    return rc;
}

Note that it is completely legal to free a NULL pointer.

Upvotes: 1

TsRoe
TsRoe

Reputation: 136

From my experience there rarely is a truly simple solution to this kind of problem with error handling in C.

You could do something like this, but it is arguable if this really is better.

int main()
{
    int rc = 0;

    for (int i = 1; i < 10; i++) {
        char *data = malloc(i); // Each iteration has a different alloc size

        rc = do_something(data);
        if (rc != 0) goto cleanup;

        rc = do_something_else();
        if (rc != 0) goto cleanup;
        
        free(data);
        continue;

cleanup:
        free(data);
        break;
    }
    return rc;
}

Upvotes: 0

Related Questions