Error handling is not an excuse for goto

Source

In discussions about the infamous goto statement, people frequently point out that it's okay to use goto for error handling. The argument usually goes that without goto, you'd have to repeat your error handling code for each and every error condition that can occur, which would suck more than using goto. Both is wrong.

The problem usually arises when constructing a complex object that requires a bunch of steps to complete successfully. Each one of those steps may allocate extra resources or initialize other things that will need to be cleaned up in case of errors. I'll use the constructor of the Decoder class in my ffmpeg Python module to illustrate the concept. This class is part of an audio player I wrote for myself. The constructor is called from Python as such:

decoder = Decoder(fpath.encode("utf-8"))

It then needs to do a bunch of things:

  1. Allocate memory for the Python object itself.

  2. Parse arguments passed to it to get the path of the input file.

  3. Open the input file using libavformat.

  4. Search for streams in the file.

  5. Find the best stream for playing audio (which is usually the only one in the file).

  6. Open the codec used to play the file, using libavcodec.

  7. Return self.

Needless to say, any one of those operations may or may not fail. Fortunately, there are only two operations that need to be undone when something fails: Allocation of memory for self and opening the input file. So how do we do that?

Naïve approach

Well, we can of course put each statement into an if test and if it fails, run cleanup code. This leads to:

if( Allocate memory for the Python object itself failed ){
    return NULL;
}

if( Parse arguments failed ){
    free memory;
    return NULL;
}

if( Open the input file using libavformat failed ){
    free memory;
    return NULL;
}

if( Search for streams in the file failed ){
    close infile;
    free memory;
    return NULL;
}

if( Find the best stream for playing audio failed ){
    close infile;
    free memory;
    return NULL;
}

if( Open the codec used to play the file failed ){
    close infile;
    free memory;
    return NULL;
}

return self;

This obviously sucks, because we have to repeat the cleanup code over and over again. It sure would be nice if we could get rid of all that duplication. Fortunately David Tribble outlines a very nice solution, so before you say "right, so, goto", check this out:

Awesome approach

How about we introduce an err variable which we use to track the state of whether or not we're currently in an error situation, and at which stage the error occurred? Then we can do this:

int err = 0;

if( Allocate memory for the Python object itself failed ){
    return NULL;
}

if( Parse arguments failed ){
    err = 1;
}

if( !err && Open the input file using libavformat failed ){
    err = 2;
}

if( !err && Search for streams in the file failed ){
    err = 3;
}

if( !err && Find the best stream for playing audio failed ){
    err = 4;
}

if( !err && Open the codec used to play the file failed ){
    err = 5
}

// Handle errors

if( err > 2 ){
    close infile;
}

if( err > 0 ){
    free memory;
    self = NULL;
}

return self;

This gives us all the benefits of goto, without actually using goto:

  • Error handling code is nicely deduplicated

  • it is collected at the end of the function

  • since the if( err > 2 ) check contains the undo code for where err = 2 happened, it's still easy to find out what belongs together

  • it could hardly be any simpler.