Strange behavior using goto for error handling in C

Question

I am developing a basic application in C, using the OP-TEE (TrustZone Secure OS) libraries. I am running the code in QEMU. Here is the code in which the strange behavior occurred:

void prepare_rsa_operation(TEE_OperationHandle *handle, uint32_t alg, TEE_OperationMode mode, TEE_ObjectHandle key) {
    TEE_Result ret = TEE_SUCCESS;   
    TEE_ObjectInfo key_info;

    ret = TEE_GetObjectInfo1(key, &key_info);
    if (ret != TEE_SUCCESS) {
        EMSG("TEE_GetObjectInfo1: %#" PRIx32, ret);
        goto err;
    }

    ret = TEE_AllocateOperation(handle, alg, mode, key_info.keySize);
    if (ret != TEE_SUCCESS) {
        EMSG("Failed to alloc operation handle : 0x%x", ret);
        goto err;
    }
    DMSG("========== Operation allocated successfully. ==========");

    ret = TEE_SetOperationKey(*handle, key);
    if (ret != TEE_SUCCESS) {
        EMSG("Failed to set key : 0x%x", ret);
        goto err;
    }
    DMSG("========== Operation key already set. ==========");

err:
    TEE_FreeOperation(handle);
    return 1;
}

Problem that occurred:
Both successful messages were being printed (for operation allocated and key setting), but the err label was being reached even though: the TEE_FreeOperation(handle); should be written TEE_FreeOperation(*handle);. I fixed this and removed the return, since my function returns void. Now, the code is working fine, but, in my understanding, the err label should be reached only if one of the conditional tests (if's) fail, since the goto command is just inside them.

Am I wrong with this understanding? Can anyone explain me why the err label was being reached even though no error occurred before?

Add a `return;` just before the `err:` BTW your function is defined to return void, you still attempt to `return 1;` — joop, Aug 16 '18 at 14:49
You reach the `err` label regardless because the normal execution of your code gets to it. Labels don't stop code that reaches them naturally. — Christian Gibbons, Aug 16 '18 at 14:52
Yes, @joop . As I said in the question, I removed the `return`; but, still, I thought this should be identified in compilation time, not just in runtime (in my humble opinion). — Dalton Cézane, Aug 16 '18 at 15:04
Good, @Christian . So, this is the normal flow, since I did not stop the program before the label in any part, right? — Dalton Cézane, Aug 16 '18 at 15:06
@DaltonCézane my gcc warns me, even without the `-Wall` or `-pedantic` flags. — joop, Aug 16 '18 at 15:10

score 3 · Answer 1 · answered Aug 16 '18 at 14:52

The err: label is reached either if you goto it or after executing DMSG("========== Operation key already set. ==========");. Meaning you get a clean-up no matter if the function was successful or not. That's the whole reason for using "on error goto" patterns to begin with.

A more readable alternative to the goto, is to return upon error and leave the clean-up in an external wrapper function.

score 2 · Accepted Answer · answered Aug 16 '18 at 14:58

There's no special logic that prevents code from progressing past a label.

By convention, goto is typically used in C for this type of error handing but it doesn't have to be that way. Labels for goto can be freely placed anywhere in a function. You could for example do this:

void f()
{
    int i;
start:
    printf("i=%d\n", i);
    i++;
    if (i < 10) {
        goto start;
}

But please don't.

score 1 · Answer 3 · answered Aug 16 '18 at 15:18

A labeled statement doesn't get skipped over if it's reached through normal flow of execution. IOW, given code like

if ( condition )
{
  goto err;
}

err:
  // error handling code

// regular code

If condition evaluates to false, the code following err still gets executed because it follows the if statement. You can avoid it by using second label and goto:

if ( condition )
{
  goto err;
}
goto normal;

err:
  // error handling code

normal:
  // regular code

but figuring out a goto-less way to handle the problem is better.

"but figuring out a goto-less way to handle the problem is better." Not necessarily: https://koblents.com/Ches/Links/Month-Mar-2013/20-Using-Goto-in-Linux-Kernel-Code/ — alx - recommends codidact, Jan 31 '19 at 20:01

score 1 · Answer 4 · answered Aug 16 '18 at 15:55

Much like Case labels in a switch-case statement, a label will just fall through to the next instruction when reached through the normal flow of code. It is simply a place that can be jumped to. This functionality is taken advantage of when doing clean-up after an error. For example, if you're mallocing a bunch of things and an error occurs, you can jump to different sections of the clean up code depending on when you hit your error:

int func(void) {
    int ret = -1;
    int *x = malloc(sizeof(*x));
    if (/* some error condition */) {
        goto CLEANUP1;
    }
    int *y = malloc(sizeof(*y));
    if (/* some error condition */) {
        goto CLEANUP2;
    }
    int *z = malloc(sizeof(*z));
    if (/* some error condition */) {
        goto CLEANUP3;
    }
    ret = 0;
    /* do whatever successful operations you want here */

    CLEANUP3:
    free(z);
    CLEANUP2:
    free(y);
    CLEANUP1:
    free(x);
    return ret;
}

So with the above snippet, with normal error-free execution, all of the malloc'd variables get freed before leaving the function. If there is an error after mallocing x, you jump to the CLEANUP1 label and free x. if there is an error after mallocing z, then you've also malloc'd x and y, so you jump to the CLEANUP3 label which will free z and then fall through to the other two frees.

Strange behavior using goto for error handling in C

4 Answers4