Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How should "this can never happen" style of errors in C be handled? [closed]

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);
}

In my opinion, this second error handling strategy is worse because it does not leave behind a core file that can be used to debug this situation. Furthermore, it results in code that has more lines of code.

So, the question is, how should "this can never happen" style of errors be handled in C? Should I do an abort or a controlled exit with a descriptive error message?

like image 960
juhist Avatar asked Mar 17 '15 08:03

juhist


People also ask

Why is throwing an exception better than returning an error value?

When you code using return codes, you're preparing yourself for failure, and hope your fortress of tests is secure enough. When you code using exception, you know that your code can fail, and usually put counterfire catch at chosen strategic position in your code.


2 Answers

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.

like image 195
Tyler Durden Avatar answered Oct 16 '22 11:10

Tyler Durden


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.

like image 45
Leonard Michlmayr Avatar answered Oct 16 '22 11:10

Leonard Michlmayr