Is there a suggested way to deal with errors in C?

Question

There are two main ways，which is better？

1. Deal with error right now.

int func(){
  rv = process_1();
  if(!rv){
    // deal with error_1
    return -1;
  }
  rv = process_2();
  if(!rv){
    // deal with error_1
    // deal with error_2
    return -1;
  }
  return 0;
}

2. Deal with errors at go-to. I found a lot of this style of code in the Linux kernel code.

int func(){
  rv = process_1();
  if(!rv){
    goto err_1
  }
  rv = process_2();
  if(!rv){
    goto err_2;
  }
  return 0;

err_2:
    // deal with error_2
err_1:
    // deal with error_1
  return -1;
}

Answer 1

I'd recommend you to read thoroughly the examples you have found (more if they are in the kernel code of an operating system.) The situation you describe corresponds to an algorithm that should make decisions at each stage of the execution, and those stages require to undo the previous steps.

• You first allocate some resource #1, and continue.
• then you allocate another resource (say resource #2) if that fails, then you have to free resource #1, as it is not longer valid.
• ...
• finally you allocate resource #N, if that fails you must free resources #1 to #N-1. The figure you show allows you to write in one line, a set of resource allocations, between which you have to decide if you continue. In this scenario a policy like this is recommended (for novice C programmers, as it avoids the use of goto but becomes less readable (as it nests as things happen)

    if ((res_1 = some_allocation(blablah)) != ERROR_CODE) {
        if ((res_2 = some_other_allocation(blablatwo)) != ANOTHER_ERROR_CODE) {
            ...
                if ((res_N = some_N_allocation(blablaN)) != NTH_ERROR_CODE) {
                    do_what_is_needed();
                    return_resource_N(res_N); /* free resN */
                } else {
                    do_action_corresponding_to_failed_N(); /* error for failing N */
                }
                return_resource_N_minus_one(resN_1); /* free resN_1 */
            ...
        } else {
            do_action_corresponding_to_failed_2(); /* error for failing #2 */
        }
        return_resource_1(res1);  /* free #1.  (A): (see below) */
    } else {
        do_acttion_corresponding_to_failed_1(); /* error for failing #1 */
    }
    /* there's nothing to undo here, as we have returned the first resource in (A) above. */

nothing to say about this code, but that it has no gotos, but is incredible far less readable (it's a mess of nested things in which, when you fail for resource N, then you have to return up to N-1 resources.) you can messup the resources deallocated by putting them in the wrong position and it's error prone. But on the other side, it allocates and deallocates the things in just one place and is as compact as the code with gotos.

writing this code with gotos gives this:

    if ((res_1 = some_allocation(blablah)) == ERROR_CODE) {         
        do_acttion_corresponding_to_failed_1(); /* error for failing #1 */
        goto end;
    }
    if ((res_2 = some_other_allocation(blablatwo)) == ANOTHER_ERROR_CODE) {
        do_action_corresponding_to_failed_2(); /* error for failing #2 */
        goto res1;
    }
    ...
    if ((res_N = some_N_allocation(blablaN)) == NTH_ERROR_CODE) {
        do_action_corresponding_to_failed_N(); /* error for failing #N */
        goto resN1;
    }
    do_what_is_needed();
    return_resource_N(res_N); /* free resN */
resN1: return_resource_N_minus_one(resN_1); /* free resN_1 */
    ...
res1: return_resource_1(res1);  /* free #1.  (A): (see below) */
end: /* there's nothing to undo here, as we have returned the first resource in (A) above. */

There's only thing that can be said about the first code that will make it perform better in some architectures. Dealing with goto is a pain for the compiler, as normally it has to make assumptions about all the possible resulting blocks that will end jumping to the same label, and this makes things far more difficult to optimice, resulting in not so optimiced code. (this is clear when you use structured blocks, and only implies one or two places you can come from), and you will get worse performance code (not much worse, but somewhat slower code)

You will agree with me that the equivalent code you post in your code is more readable, probably exactly the same level of correctness.

Other required use of goto constructs is when you have several nested loops and you have to exit more than the closest loop to exit.

    for(...) {
        for(...) {
            ...
                for (...) {
                    goto out;
                }
            ...
        }
    }
out:

this is also C specific, as other languages allow you to label the construct you want to exit from and specify it in the break statement.

E.g. in Java:

    external_loop: for(...) {
        for(...) {
            ...
                for (...) {
                    break external_loop;
                }
            ...
        }
    }

In this case you don't need to jump, as the break knows how many loops we need to exit.

One last thing to say. With just the while() construct, all other language constructs can be simulated, by introducing state variables to allow you to do things (e.g. stepping out of each loop by checking some variable used precisely for that). And even less.... if we allow for recursive function call, even the while() loop can be simulated, and optimicers are capable of guessing a faster implementation without recursion for the simulated block. Why in the schools nobody says never use if sentences, they are evil? This is because there's a frequent fact that newbies tend to learn one struct better than others and then, they get the vice of using it everywhere. This happens frequently with goto and not with others, more difficult to understand but easier to use, once they have been understood.

The use of goto for everything (this is the legacy of languages like assembler and early fortran) and maintaining that code normally ends in what is called spaghetti programming. A programmer just selects at random a place to write his/her code in the main code of a program, opens an editor and inserts it's code there:

Let's say that we have to do several steps, named A to F:

{
    code_for_A();
    code_for_B();
    code_for_C();
    code_for_D();
    code_for_E();
    code_for_F();
}

and later, some steps, named G and H have to be added to be executed at the end. Spaghetti programming can make the code end being something like this:

{
    code_for_A();
    code_for_B();
    code_for_C();     /* programmer opened the editor in this place */
    goto A;-------.
                  |
B:<---------------+-.
    code_for_G(); | | /* the code is added in the middle of the file */
    code_for_H(); | |
    goto C;-------+-+--.
                  | |  |
A:<---------------' |  |
    code_for_D();   |  |
    code_for_E();   |  |
    code_for_F();   |  |
    goto B; --------'  |
                       |
C:<--------------------'  
}

While this code is correct (it executes steps A to H in sequence), it will take a programmer some time to guess how the code flows from A to H, by following back and forward the gotos.

Answer 2

For an alternate open that can sometimes be used to "hide" the gotos, one of our programmers got us using what he calls "do once" loops. They look like this:

failed = true;  // default to failure
do // once
{
   if( fail == func1(parm1) )
   {  // emit error
      break;
   }
   failed = false;  // we only succeed if we get all the way through
}while(0);

// do common cleanup
// additional failure handling and/or return success/fail result

Obviously, the if block inside the 'do once' would be repeated. For example, we like this structure for setting up a network connection because there are many steps that have the possibility of failure. This structure can get tricky to use if you need a switch or another loop embedded within, but it has proven to be a surprisingly handy way to deal with error detection and common cleanup for us.

If you hate it, don't use it. (smile) We like it.

Answer 3

This is really prone to become a flame war, but here my opinion :

A lot of people will say that goto is inherently evil, that you should never use it. While I can agree to a certain degree, I also can say that when it come to clean multiple variable (like by using fclose / free / etc etc), I find goto to be the cleanest (or more readable, at least) way of doing it.

To be clear, I advise to always use the simplest way for error handling, not using always goto.

For exemple,

bool MyFunction(void)
{
    char *logPathfile = NULL;
    FILE *logFile = NULL;
    char *msg = NULL;
    bool returnValue = false;
 
    logPathfile = malloc(...);
    if (!logPathfile) {
        // Error message  (use possibly perror (3) / strerror (3))
        goto END_FUNCTION;
    }
 
    sprintf(logPathfile, "%s", "/home/user/exemple.txt");
 
    logFile = fopen(logPathfile, "w");
    if (!logFile) {
        // Error message  (use possibly perror (3) / strerror (3))
        goto END_FUNCTION;
    }
 
    msg = malloc(...);
    if (!msg) {
        // Error message  (use possibly perror (3) / strerror (3))
        goto END_FUNCTION;
    }
 
 
 
    /* ... other code, with possibly other failure test that end with goto */
 
 
 
 
    // Function's end
    returnValue = true;
 
    /* GOTO */END_FUNCTION:
    free(logPathfile);
    if (logFile) {
        fclose(logFile);
    }
    free(msg);
 
    return returnValue;
}

By using goto to handle the error, you now really reduce the risk to do memory leak. And if in the futur you have to add another variable that need cleaning, you can add the memory management really simply. Or if you have to add another test (let's say for example that the filename should not begin by "/root/"), then you reduce the risk to forgetting to free the memory because the goto whill handle it.

Like you said it, you can also use this flow structure to add rollback action. Depending the situation, you maybe don't need to have multiple goto label thougth.

Let's say that in the previous code, if there is an error, we have to delete the created file.

Simply add

/* rollback action */
if (!returnValue) {
    if (logPathfile) {
        remove(logPathfile);
    }
}

rigth after the goto label, and you're done :)

=============

edit :

The complexity added by using goto are, as far as I know, the following :

1. every variable that will be cleaned or use to use clean have to be intialized. That should not be problematic since setting pointer to a valid value (NULL or other) should always be done when declaring the variable.

for example

void MyFunction(int nbFile)
{
    FILE *array = NULL;
    size_t size = 0;

    array = malloc(nbFile * sizeof(*array));
    if (!array) {
        // Error message  (use possibly perror (3) / strerror (3))
        goto END_FUNCTION;
    }

    for (int i = 0; i < nbFile; ++i) {
        array[i] = fopen("/some/path", "w");
        if (!array[i]) {
            // Error message  (use possibly perror (3) / strerror (3))
            goto END_FUNCTION;
        }
        ++size;
    }


    /* ... other code, with possibly other failure test that end with goto */



    /* GOTO */END_FUNCTION:
    /* We need size to fclose array[i], so size should be initialized */
    for (int i = 0; i < size; ++i) {
        flcose(array[i]);
    }
    free(array);
}

(yeah, I know that If I had use calloc instead of malloc, I could have tested if array[i] != NULL to know if I need to fclose, but it's for the sake of the explanation ...)

2. You probably have to add another variable for the function return value. I usually set this variable to indicate failure at the beginning (like setting false) and give it's success value just before the goto. Sometime, in some situation, this can seem weird, but it's, in my opinion, still understandable (just add a comment :) )