Is GOTO considered harmless when jumping to cleanu

Question

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 concentrate 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.

Answer 1

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.

Answer 2

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.

Answer 3

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.

Answer 4

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;

Answer 5

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.

Answer 6

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.