Is GOTO considered harmless when jumping to cleanup at the end of function?

The goto statement has been examined at great length in several SO discussions (see this and that), and I certainly don't want to revive those heated debates.

Instead, I'd like to conc开发者_StackOverflowentrate on a single use case of gotos and discuss its value and possible alternatives.

Consider the following code snippet, which is common in (at least my own) FSMs:

while (state = next_state()) {
        switch (state) {
                case foo:
                        /* handle foo, and finally: */
                        if (error) goto cleanup;
                        break;
                case bar:
                        /* handle bar, and finally: */
                        if (error) goto cleanup;
                        break;
                /* ...other cases... */
        }
}

return ok;

cleanup:
/* do some cleanup, i.e. free() local heap requests, adjust global state, and then: */
return error;

Swapping out the cleanup stuff in a separate function just in order to save the gotos seems awkward. On the other hand, we've been raised to condemn the use of gotos wherever possible.

My question: is my code example considered good style?

If not, are there feasible alternatives available?

Please adhere to the specific usage of goto described above. I don't want to delve into yet another discussion about the general use of goto.

Your usage of goto is ok. It doesn't break the 2 good ways to use goto.

1. gotos MUST go down (a few lines) in the source
2. The innermost block of goto labels MUST contain the goto statements

Instead of extracting the cleanup logic into its own function and calling that from different places, I would consider extracting the switch statement into a separate function and returning an error code from that. In your while loop you could the check the return code and do the cleanup and return if neccessary.

If you have several resources shared between the switch and cleanup logic then I think the goto would be preferrable to passing all this state around.

I have seen goto used in this manner in the OpenBSD kernel, particulary within ATA device drivers (one such example) and I personally feel that it is good style, as it helps illustrate exactly what is happening and how the code matches up to the corresponding FSM. When trying to verify functionality of an FSM against a specification, this use of goto improves clarity somewhat.

Goto is not needed when you have switch. Using both switch and goto just adds complication.

while (state) {
        switch (state) {
                case cleanup:
                        /* do some cleanup, i.e. free() local heap requests, adjust global state, and then: */
                        return error;
                case foo:
                        /* handle foo, and finally: */
                        if (error) { state = cleanup; continue; }
                        break;
                case bar:
                        /* handle bar, and finally: */
                        if (error) { state = cleanup; continue; }
                        break;
                /* ...other cases... */
        }
        state = next_state();
}

return ok;

I'd say if the cleanup code can't be generalized, i.e. it's specific for the function it's being used in, the goto is a nice and clean way to do it.

Looking at Ben Voigt's answer gave me an alternative answer:

while (state = next_state()) {
        switch (state) {
                case foo:
                        /* handle foo, and finally: */
                        /* error is set but not bothered with here */ 
                        break;
                case bar:
                        /* handle bar, and finally: */
                        /* error is set but not bothered with here */
                        break;
                /* ...other cases... */
        }

        if (error) {
                /* do some cleanup, i.e. free() local heap requests, */
                /* adjust global state, and then: */
                return error;
        }
}

return ok;

The downside of this is that you have to remember, after it process a state, it might clean up if there's an error. It looks like the if structure could be an if-else chain to handle different types of errors.

I haven't taken a formal class on FSMs, but it looks to me like the code you posted has the same behavior.

If all your init code is done before entering the while loop, then your gotos are useless, you can do the cleanup when exiting the loop. If your state machine is all about bringing up stuff in the correct order, then why not, but since you have a state machine, why not use it to do the cleanups ?

I am not against goto when initializing several thing together, and having a simple error handling code, as discussed here. But if you go through the trouble of setting up a state machine, then I can't see a good reason to use them. IMO, the question is still too general, a more practical example of state machine would be useful.

A general principle I like to follow is that one should when possible try to write code whose flow and design fits that of the problem domain ("what the program needs to do"). Programming languages include control structures and other features which are good fits for most, but not all, problem domains. Such structures should be used when they match the program's requirements. In cases where a program's requirements are not a good match for a language's features, I prefer to focus on writing code which expresses what the program needs to do, than to contort the code to fit patterns which, while they meet the requirements of other applications, don't really meet the requirements for the program being written.

In some cases, a very natural way of translating a state machines into code, in cases where a routine will be able to not have to "exit" until a state machine has run to some form of conclusion, is to have a goto label represent each state, and use use if and goto statements for the state transitions. If the required state transitions would be a better fit for other control structures (e.g. while loops), using such loops would be better than goto statements, and using switch statements may make certain kinds of "adaptations" (e.g. having a routine perform state transition each time it's executed, rather than requiring it to immediately run to completion) much easier. On the other hand, since a switch statement is really just a "goto" in disguise, it may be cleaner to simply use a goto directly than use a switch statement to imitate one.

The use of goto to cleanup the code by breaking multiple nesting for loop is very convenient, instead of setting flags and checking it in each nesting. For example, if you are unable to open a file and you discover it deep in a nesting, you can simply goto the cleanup segment and close the files and free resources. You can see such goto examples in the coreutilities tools sources.

If you just need some cleanup code to be able to be called from multiple places in your procedure and it needs to access local resources, maybe use a statement lambda instead. Define it before your switch logic and just call it where you need to clean up. I like the idea for a couple reasons: 1) It's cooler than a goto (and this is always important) 2) You get the clean encapsulation of logic without having to create an external method and pass a bunch of parameters since the lambda can access the same local variables withing the closure.