CODEWITHSUNDEEP
cgoto
Curunir
Curunir

Reputation: 1288

C usage of goto

I have a state machine with some kind of callback mechanism were functions are invoked by a task runner. Behind all of that is a state machine which has 4 kinds of states, some excluding each other, others can be combined, which makes quite a complex rule set. One of the functions should show the user error messages if any illegal action is attempted (for sake of simplicity printf here):

static int state1 = 0;
static bool switch2 = 1;

void do_stuff(int value){
    int errorCode = 0;
    if(state1 == 1){ 
        errorCode = -1;
        goto ERROR;
    }
    if(state1 == 2 && switch2)
    {
        errorCode = 2;
        goto ERROR;
    }


    printf("No error!");
    return;
ERROR:
    printf("%d", errorCode);
}

This was the shortest and most concise way I can think of, but its always been hammered home that using goto is a bad thing. Is there any better way to solve this problem or is this the best way in regards of stability and maintenance?

Upvotes: 4

Views: 476

Answers (2)

Acorn
Acorn

Reputation: 26194

From what I can tell, you are using goto for error handling, not for a state machine.

Using goto for error handling is actually one of those cases where it is very useful and preferred vs. a convoluted chain of conditions. It allows you to perform manual RAII without repetition of code:

int do_stuff(...)
{
    ... = f1(...);
    if (...)
        goto ERROR_f1;

    ... = f2(...);
    if (...)
        goto ERROR_f2;

    ... = f3(...);
    if (...)
        goto ERROR_f3;

    // Success
    return ...;

ERROR_f3:
    undo_f2(...);

ERROR_f2:
    undo_f1(...);

ERROR_f1:
    return ...;
}

Upvotes: 3

cigien
cigien

Reputation: 60460

goto is rarely the right solution for control flow. While there are valid use cases for goto, in this particular function you could simply restructure the control flow into if-else branches like this:

void do_stuff(int value)
{
    int errorCode = 0;

    if (state1 == 1)
    { 
        errorCode = -1;
    }
    else if (state1 == 2 && switch2)
    {
        errorCode = 2;
    }
    else // unconditional case for no errors
    {
        printf("No error!");
        return;
    }
    printf("%s", errorCode);  // if control reaches here, print the error
}

Upvotes: 8

Related Questions