C error handling coding-style [closed]

Question

In the case of the following code structure:

int function_sequence(...)
{
    f1(...);
    f2(...);
    f3(...);
    f4(...);
    f5(...);
    return SUCCESS;
}

Which way of the error handling is more acceptable?

This:

int function_sequence(...)
{
    if(f1(...)){
         // Error handling
         return ERROR;
    }

    if(f2(...)){
         // Error handling
         return ERROR;
    }

    ...

    return SUCCESS;
}

Or this:

int function_sequence(...)
{
    if(f1(...)){
        goto error;
    }

    if(f2(...)){
        goto error;
    }

    ...

    return SUCCESS;

error:
    // Error handling
    return ERROR;
}

You probably want separate questions for C and C++, since the error handling idioms will be very different for each. — Paul R
– Paul R, Commented Mar 6, 2013 at 9:10
this is a c question. c++ has better ways of error handling. the 2nd one is definitely not preferred in c. goto is synonymous with havoc. never use it. you have many ways to get around it. — Koushik Shetty
– Koushik Shetty, Commented Mar 6, 2013 at 9:11
I've seen both. The last time I've seen the goto error idiom it was corporate mandated coding style. — Some programmer dude
– Some programmer dude, Commented Mar 6, 2013 at 9:13
@JoachimPileborg is it not better to use flags and such than the goto statement even though this is within the scope? — Koushik Shetty
– Koushik Shetty, Commented Mar 6, 2013 at 9:19
@Koushik As all questions about coding styles, this is very much personal. Unless mandated by others, do what you feel most comfortable with. — Some programmer dude
– Some programmer dude, Commented Mar 6, 2013 at 9:21

hate-engine · Accepted Answer · 2013-03-06 09:16:35Z

3

For C, I prefer your second option.

Further, it's useful to do gradual cleanup (free allocated memory and so on), like in Linux kernel:

int function_sequence(...)
{
    if(f1(...)){
        goto error1;
    }

    if(f2(...)){
        goto error2;
    }

    ...

    return SUCCESS;

error2:
    cleanup2();

error1:
    cleanup1();

    // Error handling
    return ERROR;
}

edited Mar 6, 2013 at 9:16

answered Mar 6, 2013 at 9:11

hate-engine

2,37018 silver badges26 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

David Ranieri · Accepted Answer · 2013-03-06 09:14:53Z

3

int function_sequence(...)
{
    if (f1(...) && f2(...) && f3(...)  && f4(...) && f5(...)) 
        return SUCCESS;
    else
        return ERROR;
}

answered Mar 6, 2013 at 9:14

David Ranieri

41.2k8 gold badges58 silver badges103 bronze badges

2 Comments

Alex Over a year ago

This complicates a bit reading and debugging, as you are not able to set breakpoint in a separate functions.

David Ranieri Over a year ago

Thats true, but you can set breakpoint inside each function, don't you?

RichieHindle · Accepted Answer · 2013-03-06 09:13:08Z

1

In C, I'd use the third option:

int function_sequence(...)
{
    bool ok = true;
    if (ok) ok = f1(...);
    if (ok) ok = f2(...);
    if (ok) ok = f3(...);
    if (ok) ok = f4(...);
    if (ok) ok = f5(...);

    if (!ok) error_handling();

    cleanup();  // Your example didn't have this, but it's often here.

    return ok ? SUCCESS : ERROR;
}

answered Mar 6, 2013 at 9:13

RichieHindle

283k49 gold badges367 silver badges408 bronze badges

Comments

Luchian Grigore · Accepted Answer · 2013-03-06 09:13:50Z

1

If it's an exceptional thing that makes the function not work, throw an exception.

If you have reason to stick to a return code, you can use

if ( f1() && f2() && .... )
    return SUCCESS;
else
    return ERROR;

Execution will stop after the first false or 0 is encountered, so the effects are pretty much the same as your version.

Some libraries return 0 on success (heh, even C++'s main does so). So you might want

if ( !f1() &&.... )

or a simple

return f1() && f2() &&.... ;

answered Mar 6, 2013 at 9:13

Luchian Grigore

260k68 gold badges465 silver badges634 bronze badges

Comments

Barath Ravikumar · Accepted Answer · 2013-03-06 09:14:52Z

1

This is what i normally use

int function_sequence(...)
{
    if(f1(...)){
        goto error1;
    }

    if(f2(...)){
        goto error2;
    }

    return SUCCESS;
}

error1:
    // Error 1 handling
    return -eerno1;

error2:
    // Error 2 handling
    return -eerno2;

//end of main

answered Mar 6, 2013 at 9:14

Barath Ravikumar

5,8763 gold badges26 silver badges39 bronze badges

1 Comment

Armen Michaeli Over a year ago

Is this valid C code? What is the context it is pasted from? main? I just can't seem to figure out whether you then have definition of function_sequence procedure inside the body of another procedure (context) or this is more of a pseude-code with some valid C syntax.

Joze · Accepted Answer · 2013-03-06 09:19:18Z

1

The way i like to do is the following in c langage :

int function_sequence(...)
{
    int res=0;

    if(f1(...)){
        goto ENDF;
    }

    if(f2(...)){
        goto ENDF;
    }

    ...

    res = 1;

ENDF:
   if(res ==0)
   {
      //Error handling
   } 

   //Success and Error stuff to do (like free...).

    return res;
}

So there is only one return, and sometimes there is some actions to do in both : error and success (such as free...).

I guess my point of view can be discussed.

edited Mar 6, 2013 at 9:19

answered Mar 6, 2013 at 9:13

Joze

1,3054 gold badges19 silver badges33 bronze badges

3 Comments

Blagovest Buyukliev Over a year ago

This code would still invoke the error handling even if all the calls succeed.

Anirudh Ramanathan Over a year ago

add a return success above the error label. It will be correct, though not good.

Joze Over a year ago

I change it the real way i'm doing it... ( copy / paste mistake :/)

Collectives™ on Stack Overflow

C error handling coding-style [closed]

6 Answers 6

Comments

2 Comments

Comments

Comments

1 Comment

3 Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

Comments

2 Comments

Comments

Comments

1 Comment

3 Comments

Linked

Related