tags:

views:

266

answers:

6

Sometimes I have to write code that alternates between doing things and checking for error conditions (e.g., call a library function, check its return value, keep going). This often leads to long runs where the actual work is happening in the conditions of if statements, like

if(! (data = (big_struct *) malloc(sizeof(*data)))){
    //report allocation error
} else if(init_big_struct(data)){
    //handle initialization error
} else ...

How do you guys write this kind of code? I've checked a few style guides, but they seem more concerned with variable naming and whitespace.

Links to style guides welcome.

Edit: in case it's not clear, I'm dissatisfied with the legibility of this style and looking for something better.

+12  A: 

I usually write that code in this way:

data = (big_struct *) malloc(sizeof(*data));
if(!data){
    //report allocation error
    return ...;
}

err = init_big_struct(data);
if(err){
    //handle initialization error
    return ...;
}

...

In this way I avoid calling functions inside if and the debug is easier because you can check the return values.

Maurizio Reginelli
Took the words right out of my answer. :)
mskfisher
+1 - its more typing but readable and debuggable
pm100
+9  A: 

Though it pains me to say it, this might be a case for the never-popular goto. Here's one link I found on on the subject: http://eli.thegreenplace.net/2009/04/27/using-goto-for-error-handling-in-c/

Fred Larson
+1 for the best answer.
rmk
If you are adverse to seeing `goto` in your code, you can always disguise it. http://stackoverflow.com/questions/2314066/do-whilefalse
jschmier
+1 Using goto is still the best way in this horrible language.
Tronic
yes - this is good practice for C code (in c++ code use exceptions and RAII). set resources ptrs to NULL; set then when got and have a common exit point that releases if !NULL. Only downside is it really makes you put all your resource variables at top scope of function (I prefer to declare as locally as possible)
pm100
+2  A: 

Dont use assert in production code.
In debug mode, assert should never be used for something that can actually happen (like malloc returning NULL), rather it should be used in impossible cases (like array index is out of bounds in C)

Read this post for more.

N 1.1
+1  A: 

One method which I used to great effect is the one used by W. Richard Stevens in Unix Network Programming (code is downloadable here. For common functions which he expects to succeed all the time, and has no recourse for a failure, he wraps them, using a capital letter (code compressed vertically):

void * Malloc(size_t size) {
    void    *ptr;
    if ( (ptr = malloc(size)) == NULL)
        err_sys("malloc error");
    return(ptr);
}

err_sys here displays the error and then performs an exit(1). This way you can just call Malloc and know that it will error out if there is a problem.

UNP continues to be the only book I've where I think the author has code which checks the return values of all the functions which it's possible to fail. Every other book says "you should check the return values, but we'll leave that for you to do later".

jamuraa
It should be noted that, although such solution is often good for a standalone programs, it is very bad idea to handle errors this way in libraries – they should never abort unexpectedly.
Jacek Konieczny
A: 

I tend to

  • Delegate error checking to wrapper functions (like Stevens)
  • On error, simulate exceptions using longjmp. (I actually use Dave Hanson's C Interfaces and Implementations to simulate exceptions.)

Another option is to use Don Knuth's literate programming to manage the error-handling code, or some other kind of preprocessor. This option is available only if you get to set the rules for your shop :-)

Norman Ramsey
A: 

The only grouping property of code like this is that there simply is an externally imposed sequence that it has to follow. This is why you put these allocations into one function, but this is a very weak commonality. Why some people recommend to abandon the scope advantages of nested if's is beyond my understanding. You are effectively trying to put lipstick on a pig (no insult intended) - the code's nature will never yield anything clean, the best you can do is to use the compilers help to catch (maintenance) errors. Stick with the if's IMHO.

PS: if I haven't convinced you yet: what will the goto-solution look like if you have to take ternary decisions? The if's will get uglier for sure, but the goto's???

slartibartfast