views:

2195

answers:

12

Hi!

What do you consider "best practice" when it comes to error handling errors in a consistent way in a C library.

There are two ways I've been thinking of:

Always return error code. A typical function would look like this:

MYAPI_ERROR getObjectSize(MYAPIHandle h, int* returnedSize);

The always provide an error pointer approach:

int getObjectSize(MYAPIHandle h, MYAPI_ERROR* returnedError);

When using the first approach it's possible to write code like this where the error handling check is directly placed on the function call:

int size;
if(getObjectSize(h, &size) != MYAPI_SUCCESS) {
  // Error handling
}

Which looks better than the error handling code here.

MYAPIError e;
int size;
size = getObjectSize(h, &error);
if(error != MYAPI_SUCCESS) {
  // Error handling
}

However, I think using the return value for returning data makes the code more readable, It's obvious that something was written to the size variable in the second example.

Do you have any ideas on why I should prefer any of those approaches or perhaps mix them or use something else? I'm not a fan of global error states since it tends to make multi threaded use of the library way more painful.

EDIT: C++ specific ideas on this would also be interesting to hear about as long as they are not involving exceptions since it's not an option for me at the moment.

+4  A: 

I personally prefer the former approach (returning an error indicator).

Where necessary the return result should just indicate that an error occurred, with another function being used to find out the exact error.

In your getSize() example I'd consider that sizes must always be zero or positive, so returning a negative result can indicate an error, much like UNIX system calls do.

I can't think of any library that I've used that goes for the latter approach with an error object passed in as a pointer. stdio, etc all go with a return value.

Alnitak
For the record, one library I've seen use the latter approach is the Maya programming API. It's a c++ library rather than C though. It's quite inconsistent in how it handles its errors and sometimes the error is passed as return value and other times it passes the result as a reference.
Laserallan
don't forget strtod, ok, the last argument is not only for indicating errors, but it does it, too.
quinmars
+4  A: 

I have done a lot of C programming in the past. And I really apreciated the error code return value. But is has several possible pitfalls:

  • Duplicate error numbers, this can be solved with a global errors.h file.
  • Forgetting to check the error code, this should be solved with a cluebat and long debugging hours. But in the end you will learn (or you will know that someone else will do the debugging).
Gamecat
second problem can be solved by proper compiler warning level, proper code review mechanism and by static code analyzer tools.
Ilya
You can also work on the principle: if the API function is called and the return value is not checked, there is a bug.
Jonathan Leffler
+4  A: 

I like the error as return-value way. If you're designing the api and you want to make use of your library as painless as possible think about these additions:

  • store all possible error-states in one typedef'ed enum and use it in your lib. Don't just return ints or even worse, mix ints or different enumerations with return-codes.

  • provide a function that converts errors into something human readable. Can be simple. Just error-enum in, const char* out.

  • I know this idea makes multithreaded use a bit difficult, but it would be nice if application programmer can set an global error-callback. That way they will be able to put a breakpoint into the callback during bug-hunt sessions.

Hope it helps.

Nils Pipenbrinck
+1  A: 

I definitely prefer the first solution :

int size;
if(getObjectSize(h, &size) != MYAPI_SUCCESS) {
  // Error handling
}

i would slightly modify it, to:

int size;
MYAPIError rc;

rc = getObjectSize(h, &size)
if ( rc != MYAPI_SUCCESS) {
  // Error handling
}

In additional i will never mix legitimate return value with error even if currently the scope of function allowing you to do so, you never know which way function implementation will go in the future.

And if we already talking about error handling i would suggest goto Error; as error handling code, unless some undo function can be called to handle error handling correctly.

Ilya
+2  A: 

First approach is better IMHO:

  • It's easier to write function that way. When you notice an error in the middle of the function you just return an error value. In second approach you need to assign error value to one of the parameters and then return something.... but what would you return - you don't have correct value and you don't return error value.
  • it's more popular so it will be easier to understand, maintain
Klesk
+2  A: 

The UNIX approach is most similar to your second suggestion. Return either the result or a single "it went wrong" value. For instance, open will return the file descriptor on success or -1 on failure. On failure it also sets errno, an external global integer to indicate which failure occurred.

For what it's worth, Cocoa has also been adopting a similar approach. A number of methods return BOOL, and take an NSError ** parameter, so that on failure they set the error and return NO. Then the error handling looks like:

NSError *error = nil;
if ([myThing doThingError: &error] == NO)
{
  // error handling
}

which is somewhere between your two options :-).

Graham Lee
A: 

In addition to what has been said, prior to returning your error code, fire off an assert or similar diagnostic when an error is returned, as it will make tracing a lot easier. The way I do this is to have a customised assert that still gets compiled in at release but only gets fired when the software is in diagnostics mode, with an option to silently report to a log file or pause on screen.

I personally return error codes as negative integers with no_error as zero , but it does leave you with the possible following bug

if (MyFunc())
 DoSomething();

An alternative is have a failure always returned as zero, and use a LastError() function to provide details of the actual error.

Shane MacLaughlin
A: 

EDIT:If you need access only to the last error, and you don't work in multithreaded environment.

You can return only true/false (or some kind of #define if you work in C and don't support bool variables), and have a global Error buffer that will hold the last error:

int getObjectSize(MYAPIHandle h, int* returnedSize);
MYAPI_ERROR LastError;
MYAPI_ERROR* getLastError() {return LastError;};
#define FUNC_SUCCESS 1
#define FUNC_FAIL 0

if(getObjectSize(h, &size) != FUNC_SUCCESS ) {
    MYAPI_ERROR* error = getLastError();
    // error handling
}
Igor Oks
this approach does not work in multi threaded environment
Ilya
+1  A: 

Second approach lets the compiler produce more optimized code, because when address of a variable is passed to a function, the compiler cannot keep its value in register(s) during subsequent calls to other functions. The completion code usually is used only once, just after the call, whereas "real" data returned from the call may be used more often

dmityugov
+4  A: 

I've used both approaches, and they both worked fine for me. Whichever one I use, I always try to apply this principle:

If the only possible errors are programmer errors, don't return an error code, use asserts inside the function.

An assertion that validates the inputs clearly communicates what the function expects. While too much error checking can obscure the program logic. And deciding what to do for all the various error cases can really complicate the design. Why figure out how functionX should handle a null pointer if you can instead insist that the programmer never pass one?

AShelly
+2  A: 

I use the first approach whenever I create a library. There are several advantages of using a typedef'ed enum as a return code.

  • If the function returns a more complicated output such as an array and it's length you do not need to create arbitrary structures to return.

    rc = func(..., int **return_array, size_t *array_length);
    
  • It allows for simple, standardized error handling.

    if ((rc = func(...)) != API_SUCCESS) {
       /* Error Handling */
    }
    
  • It allows for simple error handling in the library function.

    /* Check for valid arguments */
    if (NULL == return_array || NULL == array_length)
        return API_INVALID_ARGS;
    
  • Using a typedef'ed enum also allows for the enum name to be visible in the debugger. This allows for easier debugging without the need to constantly consult a header file. Having a function to translate this enum into a string is helpful as well.

The most important issue regardless of approach used is to be consistent. This applies to function and argument naming, argument ordering and error handling.

Jeffrey Cohen
+1  A: 

I was pondering this issue recently as well, and wrote up some macros for C that simulate try-catch-finally semantics using purely local return values. Hope you find it useful.

naasking
Thanks for letting me know. It was interesting to see.
Laserallan