CODEWITHSUNDEEP
c++error-handlinggoto
Bregalad
Bregalad

Reputation: 634

How avoid goto in error processing

I know how horrible and hard to read programs with gotos are, and I'd like to avoid them at all costs, as everyone else.

However, I encountered a situation where I had a hard time avoiding them while having an elegant program. It is with error processing. I do a lot of steps, and if there is an error in any of the step, the program should display an error message, free some memory and return false. If everything works, it should return true.

The original program is like that :

bool init_routine()
{
     int errcode;   // Error code
     errcode = do_someting();
     if(errcode != RETURN_SUCCESS)
     {
         free_some_memory();
         std::cerr << "Something went wrong in initialization.";
         return false;
     }
     errcode = do_something_else();
     if(errcode != RETURN_SUCCESS)
     {
         free_some_more_memory();   // Was allocated in the do_something function
         free_some_memory();
         std::cerr << "Something went wrong in initialization.";
         return false;
     }
     /* The same pattern repeats quite a few times... */
     return true;   // Everything was ok
}

This is ugly, because some code is copy-pasted over and over, which is bad practice. I could use a function, but then the function cannot return false from the callee, because C++ lacks what I'd call a double return instruction.

Another way to do it, is to use nested if :

bool init_routine()
{
     int errcode;   // Error code
     errcode = do_someting();
     if(errcode == RETURN_SUCCESS)
     {
         errcode = do_something_else();
         if(errcode == RETURN_SUCCESS)
         {
             /* The same pattern repeats quite a few times */
             if()
             {
                if()
                {
                      if()
                      {
                         return true;
                      }
                }
             }
         }
         free_some_more_memory();
     }
     free_some_memory();
     std::cerr << "Something went wrong in initialization.";
     return false;
}

This avoids any repetition and is compact. However, as the number of nested if grows quickly, the indentation gets in the way. Moreover, all those if hides the simple control flow of the program which just sequentially executes instructions and exits on errors, and makes the reader believe there is a lot of complicated conditions to follow.

Another way, which is in my opinion more natural, is :

bool init_routine()
{
     int errcode;   // Error code
     errcode = do_someting();
     if(errcode != RETURN_SUCCESS)
     {
         goto err1;
     }
     errcode = do_something_else();
     if(errcode != RETURN_SUCCESS)
     {
         goto err2;
     }
     /* The same pattern repeats quite a few times... */
     return true;   // Everything was ok

err2:
     free_some_more_memory();
err1:
     free_some_memory();
     std::cerr << "Something went wrong in initialization";
     return false;
}

There is no repetition, the only problem is that the evil goto is used. My question is :

Is there a way to not use goto, while not using repetition, not using exceptions, and not having insane indentation ?

EDIT : Just to make it clear, I can't use exceptions because the routines I call are from an external library written in C, and I call them from my C++ code. Sorry for not mentioning that earlier.

Upvotes: 1

Views: 755

Answers (6)

Zotta
Zotta

Reputation: 2603

The standard way is using unique_ptr or similar. If that is too inconvinient for you and you are using c++0x or later, you can use finally lambdas:

T* resource = some_legacy_c_function();
if(!resource) return false; //or throw, or whatever
FINALLY([&](){
    cleanup(resource);
});
do_stuff_with(resource);
return;//cleanup is called automatically here :)

With the finally macro defined as follows (put this somewhere in your header):

template<typename F>
class _Finally
{
private:
    F _f;

public:
    inline _Finally(const F& f) :
        _f(f)
    {
    }

    inline ~_Finally(){
        _f();
    };
};

template<typename F>
inline _Finally<F> Finally(const F& f)
{
    return _Finally<F>(f);
}

#define FINALLY auto __finally_##__LINE__ = Finally

Upvotes: 0

tamas.kenez
tamas.kenez

Reputation: 7799

  1. While I don't use goto there are respected programmers who do use forward-gotos in cases like this. Anyway, there is an alternative solution: create a one-shot loop.
  2. I also agree with the other guys, use RAII if possible. However in certain situations (e.g. legacy do_something()) you can do some 'manual-RAII' with boolean flags.

So your code can be refactored like this:

bool init_routine()
{
    int errcode;   // Error code
    bool need_free_some_memory = false;
    bool need_free_some_more_memory = false;

    do { //not a loop, just scope for break

        need_free_some_memory = true;
        errcode = do_something();

        if(errcode != RETURN_SUCCESS)
            break;

        need_free_some_more_memory = true;
        errcode = do_something_else();
        if(errcode != RETURN_SUCCESS)
            break;

    } while(0);

    if(need_free_some_more_memory)
        free_some_more_memory();   // Was allocated in the do_something function
    if(need_free_some_memory)
        free_some_memory();

    if(errcode != RETURN_SUCCESS)
        std::cerr << "Something went wrong in initialization.";

    return errcode == RETURN_SUCCESS;   // Everything was ok
}

Upvotes: 1

doron
doron

Reputation: 28882

goto is generally banned because this often results in a rats-nest of if-then-goto statements that have no structure. Loops and function calls are much better ways of doing this.

Having said that when it comes to breaking out of deeply nested loops or for error handling gotos are an appropriate way of doing things. The goto cleanup pattern is used extensively in the linux kernel for error handling. So for C style programming what you doing is perfectly acceptable.

However in C++ there are often better options. RAII and smart pointers are a good first prize. And exceptions should cover the rest of the need for goto although embedded systems often have exceptions disabled.

Upvotes: 1

Otomo
Otomo

Reputation: 880

Without asking why you try to avoid exceptions:

There are a few ways to do error handling. First there are the C-Style ways. Return an integer or bool which indicates failure (instead of void) or return an error code over an integer (in C often the global variable errno is used for this). And the ugly goto.

The more C++-Style ways:

If your function returns objects or values you could define invalid objects (e.g. an image with width = height = 0).

The lack of the «double return» in c++ you mentioned is not present in c++11. You could do something like this: return std::make_pair(obj,true).

If you need to handle ressources in error cases I give you the following advice: Don't do it. Use RAII instead. It saves you a lot of efford and is save.

Upvotes: 1

Ulrich Eckhardt
Ulrich Eckhardt

Reputation: 17415

If you manage allocated resources using objects representing ownership (using e.g. smart pointers or similar things), these problems just don't occur:

resource r;
if (int e = alloc_resource(&r))
    return e;

The destructor of r will take care of releasing the resource on scope exit.

Using errorcodes instead of exceptions is completely stupid though, because they take up the returnvalue spot, carry only a limited amound of information and you can forget to check them. Use exceptions.

Upvotes: 1

Cheers and hth. - Alf
Cheers and hth. - Alf

Reputation: 145259

Use standard collections, or smart pointers such as std::unique_ptr if you absolutely have to explicitly allocate. They do the deallocation automatically. That's it.

More generally, use objects with destructors that clean up (that's what standard collections and smart pointers do). That's known as RAII, Resource Acquisition Is Initialization. Google that to learn more, and look it up in the index of your text book.

By the way, it's not a good idea to mix output into a general function. What if you want to use that code in a GUI program?

Upvotes: 2

Related Questions