Is this a decent use-case for goto in C?

Question

I really hesitate to ask this, because I don't want to "solicit debate, arguments, polling, or extended discussion" but I'm new to C and want to gain more insight into common patterns used in the language.

I recently heard some distaste for the goto command, but I've also recently found a decent use-case for it.

Code like this:

error = function_that_could_fail_1();
if (!error) {
    error = function_that_could_fail_2();
    if (!error) {
        error = function_that_could_fail_3();
        ...to the n-th tab level!
    } else {
        // deal with error, clean up, and return error code
    }
} else {
    // deal with error, clean up, and return error code
}

If the clean-up part is all very similar, could be written a little prettier (my opinion?) like this:

error = function_that_could_fail_1();
if(error) {
    goto cleanup;
}
error = function_that_could_fail_2();
if(error) {
    goto cleanup;
}
error = function_that_could_fail_3();
if(error) {
    goto cleanup;
}
...
cleanup:
// deal with error if it exists, clean up
// return error code

Is this a common or acceptable use-case of goto in C? Is there a different/better way to do this?

Answer 1

The goto statement (and its corresponding labels) are a flow control primitive (along with conditional execution of a statement). By that, I mean that they are there to allow you to construct program flow control networks. You can think of them as modeling the arrows between the nodes of a flowchart.

Some of these can be optimized out immediately, where there is a direct linear flow (you just use a sequence of basic statements). Other patterns are best replaced with structured programming constructs where these are available; if it looks like a while loop, use a while loop, OK? The structured programming patterns are definitely at least potentially clearer of intent than a mess of goto statements.

Yet C does not include all possible structured programming constructs. (It's not clear to me that all relevant ones have been discovered yet; the rate of discovery is slow now, but I'd hesitate to jump to saying that all have been found.) Of the ones we know about, C definitely lacks the try/catch/finally structure (and exceptions too). It also lacks multi-level break-from-loop. These are the kinds of things which a goto can be used to implement. It's possible to use other schemes to do these too — we do know that C has a sufficient set of non-goto primitives — but these often involve creating flag variables and much more complex loop or guard conditions; increasing the entanglement of the control analysis with the data analysis makes the program harder to understand overall. It also makes it more difficult for the compiler to optimize and for the CPU to execute rapidly (most flow control constructs — and definitely goto — are very cheap).

Thus, while you shouldn't use goto unless needed, you should be aware that it exists and that it may be needed, and if you need it, you shouldn't feel too bad. An example of a case where it is needed is resource deallocation when a called function returns an error condition. (That is, try/finally.) It's possible to write that without goto but doing that can have downsides of its own, such as the problems of maintaining it. An example of the case:

int frobnicateTheThings() {
    char *workingBuffer = malloc(...);
    int i;

    for (i=0 ; i<numberOfThings ; i++) {
        if (giveMeThing(i, workingBuffer) != OK)
            goto error;
        if (processThing(workingBuffer) != OK)
            goto error;
        if (dispatchThing(i, workingBuffer) != OK)
            goto error;
    }

    free(workingBuffer);
    return OK;

  error:
    free(workingBuffer);
    return OOPS;
}

The code could be even shorter, but it's enough to demonstrate the point.

Answer 2

Yes.

It is used in, for example, the linux kernel. Here's an email from the end of a thread from nearly a decade ago, bolding mine:

From: Robert Love
Subject: Re: any chance of 2.6.0-test*?
Date: 12 Jan 2003 17:58:06 -0500

On Sun, 2003-01-12 at 17:22, Rob Wilkens wrote:

I say "please don't use goto" and instead have a "cleanup_lock" function and add that before all the return statements.. It should not be a burden. Yes, it's asking the developer to work a little harder, but the end result is better code.

No, it is gross and it bloats the kernel. It inlines a bunch of junk for N error paths, as opposed to having the exit code once at the end. Cache footprint is key and you just killed it.

Nor is it easier to read.

As a final argument, it does not let us cleanly do the usual stack-esque wind and unwind, i.e.

        do A
        if (error)
            goto out_a;
        do B
        if (error)
            goto out_b;
        do C
        if (error)
            goto out_c;
        goto out;
        out_c:
        undo C
        out_b:
        undo B:
        out_a:
        undo A
        out:
        return ret;

Now stop this.

Robert Love

That said, it requires a lot of discipline to keep yourself from creating spaghetti code once you get used to using goto, so unless you're writing something that requires speed and a low memory footprint (like a kernel or embedded system) you should really think about it before you write the first goto.

Answer 3

In my opinion, the code you posted is an example of a valid use of goto, because you only jump downwards and only use it like a primitive exception handler.

However, because of the old goto debate, programmers have been avoiding goto for some 40 years and therefore they aren't used to read code with goto. This is a valid reason to avoid goto: it simply isn't the standard.

I would have rewritten the code as something more easily read by C programmers:

Error some_func (void)
{
  Error error;
  type_t* resource = malloc(...);

  error = some_other_func (resource);

  free (resource);

  /* error handling can be placed here, or it can be returned to the caller */

  return error;
}


Error some_other_func (type_t* resource)  // inline if needed
{
  error = function_that_could_fail_1();
  if(error)
  {
    return error;
  }

  /* ... */

  error = function_that_could_fail_2();
  if(error)
  {
    return error;
  }

  /* ... */

  return ok;
}

Advantages of this design:

• The function doing the actual work does not need to concern itself with tasks that are irrelevant to its algorithm, such as allocating data.
• The code will look less alien to C programmers, since they are afraid of goto and labels.
• You can centralize error handling and deallocation at the same spot, outside the function doing the algorithm. It doesn't make sense for a function to handle its own results.

Answer 4

A famous paper describing case of valid use of was Structured Programming with GOTO Statement by Donald E. Knuth (Stanford University). The paper appeared in the days where using GOTO was considered a sin by some and when the movement for Structured Programming was the at its peak. You may want to take a look at GoTo Considered Harmful.

Answer 5

I think it is a decent use case, but in case "error" is nothing more than a boolean value, there is a different way to accomplish what you want:

error = function_that_could_fail_1();
error = error || function_that_could_fail_2();
error = error || function_that_could_fail_3();
if(error)
{
     // do cleanup
}

This makes use of the short-circuit evaluation of boolean operators, which guarantees that after the first failing function the others are not executed any more. If this "better", is up to your personal taste and how much you are accustomed to that idiom.

Answer 6

In Java you'd do it like this:

makeCalls:  {
    error = function_that_could_fail_1();
    if (error) {
        break makeCalls;
    }
    error = function_that_could_fail_2();
    if (error) {
        break makeCalls;
    }
    error = function_that_could_fail_3();
    if (error) {
        break makeCalls;
    }
    ...
    return 0;  // No error code.
}
// deal with error if it exists, clean up
// return error code

I use this a lot. Much as I dislike goto's, in most other C-style languages I use your code; there's no other good way to do it. (Jumping out of nested loops is a similar case; in Java I use a labeled break and everywhere else I use a goto.)

Answer 7

The linux style guide gives specific reasons to use goto's which are in line with your example:

https://www.kernel.org/doc/Documentation/process/coding-style.rst

The rationale for using gotos is:

• unconditional statements are easier to understand and follow
• nesting is reduced
• errors by not updating individual exit points when making modifications are prevented
• saves the compiler work to optimize redundant code away ;)

Disclaimer I'm not supposed to share my work. The examples here are a bit contrived so bear please bear with me.

This is good for memory management. I recently worked on code that had dynamically allocated memory (for example a char * returned by a function). A function that looks at a path and ascertains whether the path is valid by parsing the tokens of the path:

tmp_string = strdup(string);
token = strtok(tmp_string,delim);
while( token != NULL ){
    ...
    some statements, some involving dynamically allocated memory
    ...
    if ( check_this() ){
        free(var1);
        free(var2);
        ...
        free(varN);
        return 1;
    }
    ...
    some more stuff
    ...
    if(something()){
        if ( check_that() ){
            free(var1);
            free(var2);
            ...
            free(varN);
            return 1;
        } else {
            free(var1);
            free(var2);
            ...
            free(varN);
            return 0;
        }
    }
    token = strtok(NULL,delim);
}

free(var1);
free(var2);
...
free(varN);
return 1;

Now to me, the following code is much nicer and easier to maintain if you need to add a varNplus1:

int retval = 1;
tmp_string = strdup(string);
token = strtok(tmp_string,delim);
while( token != NULL ){
    ...
    some statements, some involving dynamically allocated memory
    ...
    if ( check_this() ){
        retval = 1;
        goto out_free;
    }
    ...
    some more stuff
    ...
    if(something()){
        if ( check_that() ){
            retval = 1;
            goto out_free;
        } else {
            retval = 0;
            goto out_free;
        }
    }
    token = strtok(NULL,delim);
}

out_free:
free(var1);
free(var2);
...
free(varN);
return retval;

Now the code had all sorts of other problems with it, namely that N was somewhere above 10, and the function was over 450 lines, with 10 levels of nestedness in some places.

But I offered my supervisor to refactor it, which I did and now it's a bunch of functions that are all short, and they all have the linux style

int function(const char * param)
{
    int retval = 1;
    char * var1 = fcn_that_returns_dynamically_allocated_string(param);
    if( var1 == NULL ){
        retval = 0;
        goto out;
    }

    if( isValid(var1) ){
         retval = some_function(var1);
         goto out_free;
    }

    if( isGood(var1) ){
         retval = 0;
         goto out_free;
    }

out_free:
    free(var1);
out:
    return retval;
}

If we consider the equivalent without gotos:

int function(const char * param)
{
    int retval = 1;
    char * var1 = fcn_that_returns_dynamically_allocated_string(param);
    if( var1 != NULL ){

       if( isValid(var1) ){
            retval = some_function(var1);
       } else {
          if( isGood(var1) ){
               retval = 0;
          }
       }
       free(var1);

    } else {
       retval = 0;
    }

    return retval;
}

To me, in the first case, it is obvious to me that if the first function returns NULL, we're outta here and we're returning 0. In the second case, I have to scroll down to see that the if contains the whole function. Granted the first one indicates this to me stylistically (the name "out") and the second one does it syntactically. The first one is still more obvious.

Also, I greatly prefer having free() statements at the end of a function. That's partly because, in my experience, free() statements in the middle of functions smell bad and indicate to me that I should create a subroutine. In this case, I created var1 in my function and could not free() it in a subroutine, but that's why the goto out_free, goto out style is so practical.

I think programmers need to be brought up believing that goto's are evil. Then, when they're mature enough, they should browse the Linux source code and read the linux style guide.

I should add that I use this style very consistently, every function has an int retval, an out_free label and an out label. Because of stylistic consistency, readability is improved.

Bonus: Breaks and continues

Say you have a while loop

char *var1, *var2;
char line[MAX_LINE_LENGTH];
while( sscanf(line,... ){
    var1 = functionA(line,count);
    var2 = functionB(line,count);

    if( functionC(var1, var2){
         count++
         continue;
    }

    ...
    a bunch of statements
    ...

    count++;
    free(var1);
    free(var2);
}

There are other things wrong with this code, but one thing is the continue statement. I would like to rewrite the whole thing, but I was tasked with modifying it in a small way. It would have taken me days to refactor it in a way that satisfied me, but the actual change was about a half day's work. The problem is that even if we 'continue' we still need to free var1 and var2. I had to add a var3, and it made me want to puke to have to mirror the free() statements.

I was a relatively new intern at the time, but I had been looking at the linux source code for fun a while back, so I asked my supervisor if I could use a goto statement. He said yes, and I did this:

char *var1, *var2;
char line[MAX_LINE_LENGTH];
while( sscanf(line,... ){
    var1 = functionA(line,count);
    var2 = functionB(line,count);
    var3 = newFunction(line,count);

    if( functionC(var1, var2){
         goto next;
    }

    ...
    a bunch of statements
    ...
next:
    count++;
    free(var1);
    free(var2);
}

I think continues are OK at best but to me they're like a goto with an invisible label. The same goes for breaks. I would still prefer continue or break unless, as was the case here, it forces you to mirror modifications in multiple places.

And I should also add that this use of goto next; and the next: label are unsatisfactory to me. They are merely better than mirroring the free()'s and the count++ statements.

goto's are almost always wrong, but one must know when they are good to use.

One thing that I didn't discuss is error handling which has been covered by other answers.

Performance

One can look at the implementation of strtok() http://opensource.apple.com//source/Libc/Libc-167/string.subproj/strtok.c

#include <stddef.h>
#include <string.h>

char *
strtok(s, delim)
    register char *s;
    register const char *delim;
{
    register char *spanp;
    register int c, sc;
    char *tok;
    static char *last;


    if (s == NULL && (s = last) == NULL)
        return (NULL);

    /*
     * Skip (span) leading delimiters (s += strspn(s, delim), sort of).
     */
cont:
    c = *s++;
    for (spanp = (char *)delim; (sc = *spanp++) != 0;) {
        if (c == sc)
            goto cont;
    }

    if (c == 0) {       /* no non-delimiter characters */
        last = NULL;
        return (NULL);
    }
    tok = s - 1;

    /*
     * Scan token (scan for delimiters: s += strcspn(s, delim), sort of).
     * Note that delim must have one NUL; we stop if we see that, too.
     */
    for (;;) {
        c = *s++;
        spanp = (char *)delim;
        do {
            if ((sc = *spanp++) == c) {
                if (c == 0)
                    s = NULL;
                else
                    s[-1] = 0;
                last = s;
                return (tok);
            }
        } while (sc != 0);
    }
    /* NOTREACHED */
}

Please correct me if I'm wrong, but I believe that the cont: label and the goto cont; statement are there for performance (they surely don't make the code more readable). They could be replaced with readable code by doing

while( isDelim(*s++,delim));

to skip delimiters. But to be as fast as possible and avoid unnecessary function calls, they do it this way.

I read the paper by Dijkstra and I find it quite esoteric.

google "dijkstra goto statement considered harmful" because I don't have enough reputation to post more than 2 links.

I've seen it quoted as a reason not to use goto's and reading it has not changed anything as far as my uses of goto's are concered.

Addendum:

I've come up with a neat rule while thinking about all this about continues and breaks.

• If in a while loop, you have a continue, then the body of the while loop should be a function and the continue be a return statement.
• If in a while loop, you have a break statement, then the while loop itself should be a function and the break should become a return statement.
• If you have both, then something might be wrong.

It's not always possible due to scope issues but I've found that doing this make it much easier to reason about my code. I had noticed that whenever a while loop had a break or a continue it gave me a bad feeling.

Answer 8

Personally I'd refactor it more like this:

int DoLotsOfStuffThatCouldFail (paramstruct *params)
{
    int errcode = EC_NOERROR;

    if ((errcode = FunctionThatCouldFail1 (params)) != EC_NOERROR) return errcode;
    if ((errcode = FunctionThatCouldFail2 (params)) != EC_NOERROR) return errcode;
    if ((errcode = FunctionThatCouldFail3 (params)) != EC_NOERROR) return errcode;
    if ((errcode = FunctionThatCouldFail4 (params)) != EC_NOERROR) return errcode;

    return EC_NOERROR;
}

void DoStuff (paramstruct *params)
{
    int errcode = EC_NOERROR;

    InitStuffThatMayNeedToBeCleaned (params);

    if ((errcode = DoLotsOfStuffThatCouldFail (params)) != EC_NOERROR)
    {
         CleanupAfterError (params, errcode);
    }
}

That would be more motivated by avoiding the deep nesting than avoiding the goto however (IMO a worse problem with the first code sample), and would of course be dependent on CleanupAfterError being possible out-of-scope (in this case "params" could be a struct containing some allocated memory that you need to free, a FILE * that you need to close or whatever).

One major advantage I see with this approach is that it's both easier and cleaner to slot a hypothetical future extra step between, say, FTCF2 and FTCF3 (or remove an existing current step), so it lends itself better to maintainability (and the person who inherits my code not wanting to lynch me!) - goto aside, the nested version lacks that.

Answer 9

Take a look at the MISRA (Motor Industry Software Reliability Association) C coding guidelines that allow goto under strict criteria (Which your example meets)

Where I work the same code would be written - no goto needed - Avoiding needless religious debate about them is a big plus in any software house.

error = function_that_could_fail_1();
if(!error) {
  error = function_that_could_fail_2();
}
if(!error) {
  error = function_that_could_fail_3();
} 
if(!error) {
...
if (error) {
  cleanup:
} 

or for "goto in drag" - something even more dodgy than goto, but gets around the "No goto Ever!!!" camp) "Surely it must be OK, does not use Goto" ....

do {
  if (error = function_that_could_fail_1() ){
    break 
  }
  if (error = function_that_could_fail_2() ){
    break 
  }
  ....... 
} while (0) 
cleanup();
.... 

If the functions have the same parameter type, put them into a table and use a loop -

Answer 10

I also use goto if the alternative do/while/continue/break hackery would be less readable.

gotos have the advantage that their targets have a name and they read goto something;. This may be more readable than break or continue if you're not actually stopping or continuing something.

Answer 11

There will always be camps that say one way is acceptable and another that is not. Companies I've worked for have frowned or strongly discouraged goto use. Personally, I can't think of any time I've used one, but that doesn't mean they are bad, just another way of doing things.

In C, I typically do the following:

• Test for conditions that could prevent processing (bad inputs, etc) and "return"
• Do all steps that require resource allocation (e.g. mallocs)
• Perform the processing, where multiple steps check for success
• Release any resources, if successfully allocated
• Return any results

For the processing, using your goto example, I'd do this:

error = function_that_could_fail_1(); if(!error) { error = function_that_could_fail_2(); } if(!error) { error = function_that_could_fail_3(); }

There no nesting, and inside the if clauses, you can do any error reporting, if the step generated an error. So, it doesn't have to be "worse" than a method using gotos.

I have yet to run across a case where someone has gotos that can't be done with another method and is just as readable/understandable and that is the key, IMHO.

Answer 12

for (int y=0; y<height; ++y) {
    for (int x=0; x<width; ++x) {
        if (find(x, y)) goto found;
    }
}
found: