8

I'm developing a large project in C with other team members and we have a disagreement on how "this can never happen" style of errors should be handled. By this I mean error cases that the code currently can never reach, but somebody can modify the code and after the modification it is possible the error case will be reached.

For example, let's say I have a function that returns either 0 on success or -EINVAL if it's given a NULL argument. What I naturally do is:

int err;
struct myctx ctx;
err = callFunction(&ctx);
if (err != 0)
{
  abort();
}

or even:

int err;
struct myctx ctx;
err = callFunction(&ctx);
if (err == -EINVAL)
{
  abort();
}
else if (err != 0)
{
  abort();
}

...to distinguish between the two cases of -EINVAL and unknown error code.

The benefits of this error handling approach are that it is simple to program (few lines of code) and leaves behind a core file that can be used to quickly debug the situation.

However, some team members disagree with this error handling approach because I'm doing an unclean exit and while the code currently can never reach the abort() line it is possible that in the future somebody modifies the code and it reaches the abort() line then. According to them, I should try to do a controlled exit and print the reason to the user. So, I guess instead of this I should do:

int err;
struct myctx ctx;
err = callFunction(&ctx);
if (err == -EINVAL)
{
  fprintf(stderr, "Argument was NULL in file %s line %d\n", __FILE__, __LINE__);
  exit(1);
}
else if (err != 0)
{
  fprintf(stderr, "Error was %d in file %s line %d\n", err, __FILE__, __LINE__);
  exit(1);
}

This second error handling strategy has a downside that it does not leave behind a core file that can be used to debug this situation. Also it results in code that has more lines of code.

So, the question is, which strategies exist in C to handle "this can never happen" style of errors? What are their pros and cons? How to decide which to use? Should I do an abort or a controlled exit with a descriptive error message?

5
  • 2
    While I like the question, it seems to be quite opinion based.
    – martin
    Mar 17, 2015 at 8:41
  • 1
    The latter is better - you don't get a core dump but you do get trace saying exactly where the problem occurred, and the user gets a (slightly) better experience (you can improve the error message, but a core dump never looks good. You could write the core dump too by using abort instead of exit, which would be better all round, or you could use google's coredumper lib to write the dump and then exit (though, why not just abort in this case - dead is dead whether you exit or abort)
    – gbjbaanb
    Mar 17, 2015 at 8:46
  • Thanks! I didn't know that a coredumper library that allows generating a core dump from a running application exists. I was aware of gcore, though, but it's a separate command and not a library that the application could use to generate a coredump of itself.
    – juhist
    Mar 17, 2015 at 9:05
  • 2
    Agree with @martin, I think this question is more suited for programmers.stackexchange.com.
    – harald
    Mar 17, 2015 at 9:07
  • I wasn't aware of programmers.stackexchange.com but in the future, if the question is more suited for that site, I'll ask it there then. Can this question be moved to programmers.stackexchange.com by admins?
    – juhist
    Mar 17, 2015 at 9:11

2 Answers 2

4

On principle, you should always handle errors and generate the most explanatory description possible to the user. That is the proper thing to do, what they would call honte in Japan.

However, in practice, your unexplained abort() in the first example is what most everyone does because it is faster to write. The consequence of this is that most applications, when they fail, exit miserably. For example, when Microsoft Word crashes, the message to the user is "Word has encountered a problem and will now exit." or something to that effect. Many applications give no message at all but simply terminate.

Proper programming is to fully code what is called the negative path. Not only should every possible failure generate a meaningful text message, but as the failure returns and percolates backwards through the call chain, each calling function should add its context and any relevant parameters to the front of the message. So the end message to user should be something like this:

While attempting to initialize: while attempting to load configuration file at [c:\user\data\configuration.ini]: failed to read line 453: invalid parameter value 'NumRedirects' (out of bounds): -43

In this error message what we see is each function as it is receiving the error inserts its context at the beginning of a string followed by a colon and then returns failure to the function that called it. The net result shows all the important information (the file that was being read, the line that was being parsed, the parameter that was being loaded, the invalid value that was supplied for the parameter). This fully explains to the user (or developer) what the problem was.

3

I think this is what assertions are for. The return value is checked unless the code is compiled with NDEBUG defined.

#include <assert.h>
...
int err;
struct myctx ctx;
err = callFunction(&ctx);
assert(err != -EINVAL);
assert(err == 0);

The idea is that bugs usually lead to the unexpected. Also, assert statements increase the readability of the code. When the assertion fails the automatic error message helps the programmer to identify the failed assertion. The release version can be built with -DNDEBUG to reduce the number of branches.

Not the answer you're looking for? Browse other questions tagged or ask your own question.