Given OpenSSL's int return style error handling, how can I handle errors in OpenSSL functions without leading to memory leak?

Question

This question leans toward the "design" aspect of things, but I wanted to know how others might have dealt with this issue and might do so now considering current programming trends in C.

So OpenSSL likes to provide int returns for quite a lot if its functions; returning 1 on SUCCESS and 0 on FAILURE. In an attempt to keep error handling consistent in one of my projects I have more or less copied this return style in all of my functions. This results in subroutines that look like:

int foo (/*...*/) {
    int r;
    r = some_openssl_fn(/*...*/);
    if (!r) { handle_openssl_error(/*...*/); return 0; }
    /* ... */
    r = my_fn(/*...*/);
    if (!r) { handle_regular_error(/*...*/); return 0; }
    return 1;
}

Now this is fine if I don't have to work with pointers. However, I regularly need to use BIGNUM's, BN_CTX's, EC_POINT's, etc. These all require pointers and heap allocated memory and in this lies my quandary. Consider the following:

int bar (/*...*/) {
    int r;
    BIGNUM *bn;
    BN_CTX *ctx;

    ctx = BN_CTX_new();
    if (!ctx) { handle_openssl_error(/*...*/); return 0;}
    bn = BN_new();
    if (!bn) { handle_openssl_error(/*...*/); return 0;}
    r = BN_set_word(bn, 2ULL);
    if (!r) { handle_openssl_error(/*...*/); return 0; }
    r = BN_mul(bn, bn, bn, ctx);
    if (!r) { handle_openssl_error(/*...*/); return 0; }

    BN_free(bn);
    BN_CTX_free(ctx);
    return 1;
}

The way this code is now, upon error in any of the OpenSSL functions I will have failed to free bn or ctx, the function will return, and I will lose any references I could use to free either of them. So initially I began freeing before the returns like so:

int bar (/*...*/) {
    int r;
    BIGNUM *bn;
    BN_CTX *ctx;

    ctx = BN_CTX_new();
    if (!ctx) { handle_openssl_error(/*...*/); return 0;}
    bn = BN_new();
    if (!bn) { 
        BN_CTX_free(ctx);
        handle_openssl_error(/*...*/);
        return 0;
    }
    r = BN_set_word(bn, 2ULL);
    if (!r) { 
        BN_free(bn);
        BN_CTX_free(ctx);
        handle_openssl_error(/*...*/);
        return 0;
    }
    r = BN_mul(bn, bn, bn, ctx);
    if (!r) { 
        BN_free(bn);
        BN_CTX_free(ctx);
        handle_openssl_error(/*...*/);
        return 0;
    }

    BN_free(bn);
    BN_CTX_free(ctx);
    return 1;
}

But as you can see, it requires this unwieldy cascading of redundant free statements that is horrid to read, write, and scale to more than even a few error producing functions or heap allocations. To combat this I saw that quite a few people use goto statements to jump to the freeing section of code and though gotos seem to get a bad rap my code morphed into the following:

int bar (/*...*/) {
    int r;
    BIGNUM *bn;
    BN_CTX *ctx;

    ctx = BN_CTX_new();
    if (!ctx) { handle_openssl_error(/*...*/); goto err; }
    bn = BN_new();
    if (!bn) { handle_openssl_error(/*...*/); goto err; }
    r = BN_set_word(bn, 2ULL);
    if (!r) { handle_openssl_error(/*...*/); goto err; }
    r = BN_mul(bn, bn, bn, ctx);
    if (!r) { handle_openssl_error(/*...*/); goto err; }
    
    BN_free(bn);
    BN_CTX_free(ctx);
    return 1;
err:
    BN_free(bn);
    BN_CTX_free(ctx);
    return 0;
}

This cleaned things up somewhat, but kept some redundant frees (though I admit there may be a cleverer place to put the err label and preclude the need for the redundant frees). However it leads to the problem that should ctx fail to initialize we jump to the err section which calls free on bn which hasn't been allocated memory yet. This would mean I need to create another label and swap the order in which I free bn and ctx which I don't think is good practice because it seems from many OpenSSl examples that ctx needs to be freed after any BIGNUMs (possibly to prevent double frees?).

So I'm left with my pants down and hands up in the air somewhat. I have basically given in to the fact that to use OpenSSL in this way with this style of error handling I will inevitably incur either memory leak (due just not freeing the memory) or ugly redundant code (by freeing it).

1. I am curious what better ways there may be (with/without goto's)?
2. What is a clean way to include these error handling if statements without incurring memory leak?
3. What is a better way to do error handling in general?
4. Should I just not check for a bunch of these errors?
5. Does OpenSSL's error handling practice force this upon me or is there a way around it?

Answer 1

Some things to note:

1. Using a "trick" do { if (error) break; } while (0); can simplify the code.
2. If we initialize the pointers to NULL at the start, we can have a common/single exit/return point.
3. Maintaining a separate variable for the return value can allow things to be tighter.

Here is the refactored code:

int
bar( /* ... */ )
{
    BIGNUM *bn = NULL;
    BN_CTX *ctx = NULL;
    int r = 0; // 0=err, 1=okay

    do {
        ctx = BN_CTX_new();
        if (!ctx)
            break;

        bn = BN_new();
        if (!bn)
            break;

        r = BN_set_word(bn, 2ULL);
        if (!r)
            break;

        r = BN_mul(bn, bn, bn, ctx);
        if (!r)
            break;
    } while (0);

    if (!r)
        handle_openssl_error( /* ... */ );

    if (bn)
        BN_free(bn);
    if (ctx)
        BN_CTX_free(ctx);

    return r;
}

---

UPDATE:

Interesting trick, seems to be about the same as goto but with a bit more cumbersome readability, but I'll put in it in the bag o' tricks for later. I think it could be worth it for you to add this trick as an answer to the question M.M linked in the comment above. Or even as an edit to Dmitri's answer giving another option. – z.karl

Actually, I've been using this trick since the 1980's in professional settings. I've only had one time where the response was "What???". The usual response has been "Wow! Neat trick--I'm going to use that in my own code"

I was going to add the following links, but couldn't find them at first. For more info on the "trick", see my answers:

1. Are there range operators in the C programming language?
2. How to properly make a counting algorithm to count from file?
3. How to make a binary to decimal program in C?
4. C/C++ use a switch case match all alphabet This shows how to use a C computed goto (e.g. goto &&label;) and the do/while/0 to replace a switch/case