How to free previously allocated memory when errors occured?

Question

Given a function declaration like this:

int base_address(zval *object, int add_prefix, char **base_address TSRMLS_DC) {    
    int result;

    char *host;
    long port;
    char *prefix;  

    host = ... get host from object ...;
    port = ... get port from object ...;
    prefix = ... get prefix from object ...;

    result = SUCCESS;

    if (asprintf(base_address, "%s:%ld/%s", host, port, prefix) < 0) {
        result = FAILURE;
    }

    return result;
}

void my_func() {
    char *base_address;
    char *ping_url;

    if (base_address(getThis(), 0, &base_address TSRMLS_CC) == FAILURE) {
        MALLOC_ERROR();
    }

    if (asprintf(&ping_url, "%s/ping", base_address) < 0) {
        MALLOC_ERROR();
    }

   ... do some stuff with base address ...

    // release both, as everything worked
    free(base_address);
    free(ping_url);
}

If the first call to base_address succeeded and the second call to asprintf() failed, how do I cleanly skip to the end of the function in order to safely release allocated memory?

Is there some standard pattern how to avoid memory leaks in these situations where memory is allocated one after another (and each allocation might fail) without too much code duplication or goto statements?

What's wrong with goto? You could use a do while(0) construct, but that's essentially the same thing. — Luke, Sep 20 '11 at 20:43
@Dani: This isn't C++, so RAII is much more difficult here. :-( — Platinum Azure, Sep 20 '11 at 21:31
@PlatinumAzure: damn, some people still live in the stone age. — Daniel, Sep 20 '11 at 22:44

Jon Purdy · Answer 1 · 2011-09-20T20:51:00.363

Don't be afraid of goto. It's the simplest, cleanest, and most legible way of handling exceptions in C:

You don't repeat yourself. Duplicated code is error-prone.
You don't create deeply nested code. Deep nesting is illegible.
You don't hide behind do {...} while (0) and break. Good code says what it means.

Here's a basic example:

int operation() {

    int result = SUCCESS;

    if ((result = may_fail_first()) == FAILURE) {
        goto failed_first;
    }

    if ((result = may_fail_second()) == FAILURE) {
        goto failed_second;
    }

    // If your cleanup code doesn't ordinarily need to run.
    goto end;

failed_second:
    cleanup_second();

    // If you don't need to clean up everything.
    goto end;

failed_first:
    cleanup_first();

end:
    return result;

}

score 7 · Answer 2 · edited May 23 '17 at 09:59

7

This is one of the sane usages of goto for error handling:

if (base_address(getThis(), 0, &base_address TSRMLS_CC) == FAILURE) {
    goto end;
}

if (asprintf(&ping_url, "%s/ping", base_address) < 0) {
    goto release_address;
}

// do stuff

release_address:
free(base_address);
end:

This way you don't have to repeat the same releasing code in case you have many allocating calls which depend on each other.

You may want to refer to another of my answers here, which talks about the general case.

edited May 23 '17 at 09:59

Community

1
1

answered Sep 20 '11 at 20:44

Blagovest Buyukliev

42,498
14
94
130

2

If anyone down votes this because of goto, he can directly goto do something not programming. – Sep 20 '11 at 20:53

score 2 · Answer 3 · answered Sep 20 '11 at 20:40

2

in c, you are pretty much left to do if () in order to handle error handling

in your case

if ( base_address( ... ) )
{
  if (asprint( ... ))
  {
     /// do some stuff 

     free(ping_url);
  }
  free( base_address);
}

answered Sep 20 '11 at 20:40

AndersK

35,813
6
60
86

score 2 · Accepted Answer · 2011-09-20T21:03:36.600

If NULL is assigned to the pointer variables during declaration then free will be able to handle the cases where they were never malloced as well (it will simply do nothing). (Other guards can be used, such as -1 to refer to an invalid fd, for instance.)

I use this in conjunction with with the use goto for (additional) cleanup -- some of the other answers look far to "wordy" to me. I find that goto has to be used judiciously to avoid spaghetti-code and, in general, I find "gotos around gotos" just too hard to keep track of consistently. The additional assignment of NULL first also allows the variables themselves to be used as check-guards (during any possible cleanup or otherwise).

Happy coding.

Example:

void my_func() {
    char *base_address = NULL;
    char *ping_url = NULL;

    if (base_address(getThis(), 0, &base_address TSRMLS_CC) == FAILURE) {
        goto cleanup;
    }

    if (asprintf(&ping_url, "%s/ping", base_address) < 0) {
        goto cleanup;
    }

    // stuff... and assign return value (or return directly) if applicable,
    // assign NULL to variables which contain values that should
    // not be free'd, etc (use as guards!).
    // I prefer to add guards into the cleanup vs. "skip over it".
    // I vary rarely, if ever, have multiple cleanup sections -- in most
    // cases this would indicate the need for additional function(s) to me.

  cleanup:
    free(base_address);
    if (ping_url) {
       // perhaps need additional cleanup        
       free(ping_url);
    }

    // Return value, if applicable. (If a "direct return" above isn't
    // advisable for the given function due to cleanup rules.)
}

How to free previously allocated memory when errors occured?

4 Answers4