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

CodePudding user response：

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 :) )

CodePudding user response：

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.