while(1) .. break instead of goto

Question

I found the following code in a C program:

while (1)
{
    do_something();
    if (was_an_error()) break;

     do_something_else();
     if (was_an_error()) break;

     [...]

     break;
}
[cleanup code]

Here while(1) is used as local emulation of "finally". You can also write this using gotos:

do_something()
if (was_an_error()) goto out;

do_something_else()
if (was_an_error()) goto out;

[...]
out:
[cleanup code]

I thought the goto solution is a usual idiom. I have seen several occurrences of this idiom in the kernel sources and it is also mentioned in Diomidis Spinellis' "Code Reading" book.

My question is: What solution is better? Is there any specific reason to use the while(1) solution?

Question 943826 doesn't answer my question.

Answer 1

I know my style isn't the coolest possible, but I prefer it because it doesn't need any special constructs and is concise and not too hard to understand:

error = (!error) && do_something1();
error = (!error) && do_something2();
error = (!error) && do_something3();

// Cleanup code

Answer 2

Normally, GOTOs are considered bad but at some places where there are only Forward Jumps through GOTOs, they are not AS bad. People avoid GOTO like plague but a well-thought-out use of GOTO is sometimes a better solution IMHO.

Answer 3

Though the use of goto is discouraged usually, some rare situations like yours is a place where best-practices are not the best.

So, if goto makes the clearest code I would use it. using a while(true) loop to emulate goto is something unnatural. What you really need is a goto!

Answer 4

"do while" and "goto out" are different on these area:

1.local variable initialization

void foo(bool t = false)
{
    if (t)
    {
        goto DONE;
    }

    int a = 10; // error : Goto bypass local variable's initialization 

    cout << "a=" << a << "\n";
DONE:
}

It is fine to initialize in-place local variables in do ... while(0) block.

void bar(bool t = false)
{
    do{
        if (t)
        {
            break; 
        }

        int a = 10;  // fine

        cout << "a=" << a << "\n";
    } while (0);

}

2 difference for Macros. "do while" is a slight better. "goto DONE" in a Macro is so not the case. If the exit code is more complicated, let see like this:

err = some_func(...);
if (err)
{
    register_err(err, __LINE__, __FUNC__);
#if defined (_DEBUG)
    do_some_debug(err)
#endif
    break;
}

and you write this code again and again, you will probably put them into a Macro.

#define QUIT_IF(err)                     \
if (err)                                       \
{                                              \
    register_err(err, __LINE__, __FUNC__);     \
    DO_SOME_DEBUG(err)                         \
    break; // awful to put break in macro, but even worse to put "goto DONE" in macro.  \
}

And the code become:

do
{
    initial();

    do 
    {
        err = do_step1();
        QUIT_IF(err);

        err = do_step2();
        QUIT_IF(err);

        err = do_step3();
        QUIT_IF(err);

        ....
    } while (0);
    if (err) {     // harder for "goto DONE" to get here while still using macro.
        err = do_something_else();
    }
    QUIT_IF(err);
    .....
} while (0);

3.do... while(0) handles different levels of exiting with same macro. Code is shown above. goto ... is not the case for Macro cause you need different labels for different levels.

By saying that, I do not like both of them. I'd prefer to use the exception method. If exception is not allowed, then I use "do ... while(0)", since the whole block is indented, it is actually easier to read than "goto DONE" style.

Answer 5

Putting the code into a separate function, and using return to exit early is another way to do it, with the benefit of easy integration of a return code indicating the nature of the failure.

Answer 6

Why not use a series of if statements? I usually write it this way, as I find it much clearer than a loop:

bool ok = true;

do_something();
if (was_an_error()) ok = false;

if (ok)
{
    do_something_else();
    if (was_an_error()) ok = false;
}

if (ok)
{
    do_something_else_again();
    if (was_an_error()) ok = false;
}

[...]

[Cleanup code]

Also if you are working to strict coding standards, yes goto is likely to be forbidden, but often so are break and continue so the loop is not necessarily a workaround for that.