¿El uso válido de goto para la gestión de errores en C?

Esta pregunta es en realidad el resultado de una interesante discusión en programming.reddit.com hace un tiempo. Básicamente se reduce al siguiente código:

int foo(int bar)
{
    int return_value = 0;
    if (!do_something( bar )) {
        goto error_1;
    }
    if (!init_stuff( bar )) {
        goto error_2;
    }
    if (!prepare_stuff( bar )) {
        goto error_3;
    }
    return_value = do_the_thing( bar );
error_3:
    cleanup_3();
error_2:
    cleanup_2();
error_1:
    cleanup_1();
    return return_value;
}

El uso de gotoaquí parece ser la mejor manera de hacerlo, lo que resulta en el código más limpio y eficiente de todas las posibilidades, o al menos eso me parece a mí. Citando a Steve McConnell en Código completo :

Goto es útil en una rutina que asigna recursos, realiza operaciones en esos recursos y luego desasigna los recursos. Con un goto, puede limpiar en una sección del código. Ir a reduce la probabilidad de que se olvide de desasignar los recursos en cada lugar en el que detecte un error.

Otro soporte para este enfoque proviene del libro Controladores de dispositivos de Linux , en esta sección .

¿Qué piensas? ¿Es este caso un uso válido para gotoen C? ¿Preferiría otros métodos que produzcan un código más complicado y / o menos eficiente, pero evite goto?

FWIF, encuentro que el lenguaje de manejo de errores que dio en el ejemplo de la pregunta es más legible y más fácil de entender que cualquiera de las alternativas dadas en las respuestas hasta ahora. Si bien gotoes una mala idea en general, puede ser útil para el manejo de errores cuando se hace de una manera simple y uniforme. En esta situación, aunque sea un goto, se está utilizando de forma bien definida y más o menos estructurada.

Como regla general, evitar el goto es una buena idea, pero los abusos que prevalecían cuando Dijkstra escribió por primera vez 'GOTO Considered Dañino' ni siquiera cruzan la mente de la mayoría de las personas como una opción en estos días.

Lo que usted describe es una solución generalizable para el problema de manejo de errores; para mí está bien siempre que se use con cuidado.

Su ejemplo particular se puede simplificar de la siguiente manera (paso 1):

int foo(int bar)
{
    int return_value = 0;
    if (!do_something(bar)) {
        goto error_1;
    }
    if (!init_stuff(bar)) {
        goto error_2;
    }
    if (prepare_stuff(bar))
    {
        return_value = do_the_thing(bar);
        cleanup_3();
    }
error_2:
    cleanup_2();
error_1:
    cleanup_1();
    return return_value;
}

Continuando el proceso:

int foo(int bar)
{
    int return_value = 0;
    if (do_something(bar))
    {   
        if (init_stuff(bar))
        {
            if (prepare_stuff(bar))
            {
                return_value = do_the_thing(bar);
                cleanup_3();
            }
            cleanup_2();
        }
        cleanup_1();
    }
    return return_value;
}

Esto es, creo, equivalente al código original. Esto parece particularmente limpio ya que el código original en sí mismo estaba muy limpio y bien organizado. A menudo, los fragmentos de código no son tan ordenados como eso (aunque aceptaría un argumento de que deberían serlo); por ejemplo, con frecuencia hay más estados para pasar a las rutinas de inicialización (configuración) que los que se muestran y, por lo tanto, también hay más estados para pasar a las rutinas de limpieza.

Me sorprende que nadie haya sugerido esta alternativa, así que aunque la pregunta ha existido por un tiempo, la agregaré: una buena manera de abordar este problema es usar variables para realizar un seguimiento del estado actual. Esta es una técnica que se puede utilizar tanto si se utiliza como si no gotopara llegar al código de limpieza. Como cualquier técnica de codificación, tiene pros y contras, y no será adecuada para cada situación, pero si elige un estilo, vale la pena considerarlo, especialmente si desea evitarlo gotosin terminar con ifs profundamente anidados .

La idea básica es que, por cada acción de limpieza que deba realizarse, existe una variable a partir de cuyo valor podemos saber si la limpieza debe realizarse o no.

gotoPrimero mostraré la versión, porque está más cerca del código de la pregunta original.

int foo(int bar)
{
    int return_value = 0;
    int something_done = 0;
    int stuff_inited = 0;
    int stuff_prepared = 0;


    /*
     * Prepare
     */
    if (do_something(bar)) {
        something_done = 1;
    } else {
        goto cleanup;
    }

    if (init_stuff(bar)) {
        stuff_inited = 1;
    } else {
        goto cleanup;
    }

    if (prepare_stuff(bar)) {
        stufF_prepared = 1;
    } else {
        goto cleanup;
    }

    /*
     * Do the thing
     */
    return_value = do_the_thing(bar);

    /*
     * Clean up
     */
cleanup:
    if (stuff_prepared) {
        unprepare_stuff();
    }

    if (stuff_inited) {
        uninit_stuff();
    }

    if (something_done) {
        undo_something();
    }

    return return_value;
}

Una ventaja de esto sobre algunas de las otras técnicas es que, si se cambia el orden de las funciones de inicialización, la limpieza correcta aún ocurrirá, por ejemplo, usando el switchmétodo descrito en otra respuesta, si el orden de inicialización cambia, entonces el switchtiene que editarse con mucho cuidado para evitar intentar limpiar algo que no se inicializó realmente en primer lugar.

Ahora, algunos podrían argumentar que este método agrega una gran cantidad de variables adicionales, y de hecho en este caso eso es cierto, pero en la práctica, a menudo, una variable existente ya rastrea, o se puede hacer que rastree, el estado requerido. Por ejemplo, si en prepare_stuff()realidad es una llamada a malloc(), o a open(), entonces se puede usar la variable que contiene el puntero devuelto o el descriptor de archivo, por ejemplo:

int fd = -1;

....

fd = open(...);
if (fd == -1) {
    goto cleanup;
}

...

cleanup:

if (fd != -1) {
    close(fd);
}

Ahora, si adicionalmente hacemos un seguimiento del estado de error con una variable, podemos evitarlo por gotocompleto y aún así limpiar correctamente, sin tener una sangría que se vuelve más y más profunda cuanto más inicialización necesitamos:

int foo(int bar)
{
    int return_value = 0;
    int something_done = 0;
    int stuff_inited = 0;
    int stuff_prepared = 0;
    int oksofar = 1;


    /*
     * Prepare
     */
    if (oksofar) {  /* NB This "if" statement is optional (it always executes) but included for consistency */
        if (do_something(bar)) {
            something_done = 1;
        } else {
            oksofar = 0;
        }
    }

    if (oksofar) {
        if (init_stuff(bar)) {
            stuff_inited = 1;
        } else {
            oksofar = 0;
        }
    }

    if (oksofar) {
        if (prepare_stuff(bar)) {
            stuff_prepared = 1;
        } else {
            oksofar = 0;
        }
    }

    /*
     * Do the thing
     */
    if (oksofar) {
        return_value = do_the_thing(bar);
    }

    /*
     * Clean up
     */
    if (stuff_prepared) {
        unprepare_stuff();
    }

    if (stuff_inited) {
        uninit_stuff();
    }

    if (something_done) {
        undo_something();
    }

    return return_value;
}

Nuevamente, hay posibles críticas a esto:

• ¿No todos esos "si" perjudican el rendimiento? No, porque en el caso de éxito, debe realizar todas las comprobaciones de todos modos (de lo contrario, no estará comprobando todos los casos de error); y en el caso de falla, la mayoría de los compiladores optimizarán la secuencia de if (oksofar)comprobaciones fallidas para un solo salto al código de limpieza (GCC ciertamente lo hace) y, en cualquier caso, el caso de error suele ser menos crítico para el rendimiento.

• ¿No es esto agregar otra variable más? En este caso sí, pero a menudo la return_valuevariable se puede utilizar para desempeñar el papel que oksofarestá jugando aquí. Si estructura sus funciones para devolver errores de manera consistente, incluso puede evitar el segundo ifen cada caso:

  int return_value = 0;
  
  if (!return_value) {
      return_value = do_something(bar);
  }
  
  if (!return_value) {
      return_value = init_stuff(bar);
  }
  
  if (!return_value) {
      return_value = prepare_stuff(bar);
  }

  Una de las ventajas de codificar así es que la consistencia significa que cualquier lugar donde el programador original se haya olvidado de verificar el valor de retorno sobresale como un pulgar dolorido, lo que hace que sea mucho más fácil encontrar (esa clase de) errores.

Entonces, este es (todavía) un estilo más que se puede usar para resolver este problema. Si se usa correctamente, permite un código muy limpio y consistente, y como cualquier técnica, en las manos equivocadas puede terminar produciendo un código largo y confuso :-)

El problema con la gotopalabra clave se malinterpreta en su mayoría. No es pura maldad. Solo necesita ser consciente de las rutas de control adicionales que crea con cada goto. Se vuelve difícil razonar sobre su código y, por lo tanto, su validez.

FWIW, si busca los tutoriales de developer.apple.com, adoptan el enfoque goto para el manejo de errores.

No usamos gotos. Se concede mayor importancia a los valores de retorno. El manejo de excepciones se realiza a través de setjmp/longjmplo poco que pueda.

No hay nada moralmente incorrecto en la declaración goto más de lo que hay algo moralmente incorrecto con (nulo) * punteros.

Todo depende de cómo utilice la herramienta. En el caso (trivial) que presentó, una declaración de caso puede lograr la misma lógica, aunque con más gastos generales. La verdadera pregunta es, "¿cuál es mi requisito de velocidad?"

goto es simplemente rápido, especialmente si tiene cuidado de asegurarse de que se compile en un salto corto. Perfecto para aplicaciones donde la velocidad es una prima. Para otras aplicaciones, probablemente tenga sentido tomar el golpe de sobrecarga con if / else + case para el mantenimiento.

Recuerde: goto no mata aplicaciones, los desarrolladores matan aplicaciones.

ACTUALIZACIÓN: Aquí está el ejemplo de caso

int foo(int bar) { 
     int return_value = 0 ; 
     int failure_value = 0 ;

     if (!do_something(bar)) { 
          failure_value = 1; 
      } else if (!init_stuff(bar)) { 
          failure_value = 2; 
      } else if (prepare_stuff(bar)) { 
          return_value = do_the_thing(bar); 
          cleanup_3(); 
      } 

      switch (failure_value) { 
          case 2: cleanup_2(); 
          case 1: cleanup_1(); 
          default: break ; 
      } 
} 

GOTO es útil. Es algo que su procesador puede hacer y es por eso que debería tener acceso a él.

A veces, desea agregar algo a su función y solo ir a le permite hacerlo fácilmente. Puede ahorrar tiempo ...

En general, consideraría el hecho de que un fragmento de código podría escribirse con mayor claridad utilizando gotocomo síntoma que el flujo del programa es probablemente más complicado de lo que es generalmente deseable. La combinación de otras estructuras de programas de formas extrañas para evitar el uso gotointentaría tratar el síntoma, en lugar de la enfermedad. Es posible que su ejemplo particular no sea demasiado difícil de implementar singoto :

  hacer {
    .. configurar thing1 que solo necesitará limpieza en caso de salida anticipada
    si (error) se rompe;
    hacer
    {
      .. configurar thing2 que necesitará limpieza en caso de salida anticipada
      si (error) se rompe;
      // ***** VER TEXTO SOBRE ESTA LÍNEA
    } while (0);
    .. limpieza thing2;
  } while (0);
  .. limpieza thing1;

pero si se suponía que la limpieza solo iba a ocurrir cuando la función fallaba, el gotocaso podría manejarse poniendo un returnjusto antes de la primera etiqueta de destino. El código anterior requeriría agregar un returnen la línea marcada con *****.

En el escenario de "limpieza incluso en caso normal", consideraría el uso de gotocomo más claro que las construcciones do/ while(0), entre otras cosas porque las etiquetas de destino en sí mismas prácticamente gritan "MÍRAME" mucho más que las construcciones breaky do/ while(0). Para el caso de "limpieza solo si hay error", la returndeclaración termina teniendo que estar en el peor lugar posible desde el punto de vista de la legibilidad (las declaraciones de retorno generalmente deben estar al comienzo de una función o en lo que "parece" el fin); tener una returnetiqueta justo antes de un objetivo cumple con esa calificación mucho más fácilmente que tener una justo antes del final de un "ciclo".

Por cierto, un escenario donde a veces uso gotopara el manejo de errores es dentro de una switchdeclaración, cuando el código para varios casos comparte el mismo código de error. Aunque mi compilador a menudo sería lo suficientemente inteligente como para reconocer que varios casos terminan con el mismo código, creo que es más claro decir:

 REPARSE_PACKET:
  conmutador (paquete [0])
  {
    caso PKT_THIS_OPERATION:
      si (condición problemática)
        ir a PACKET_ERROR;
      ... manejar THIS_OPERATION
      descanso;
    caso PKT_THAT_OPERATION:
      si (condición problemática)
        ir a PACKET_ERROR;
      ... manejar THAT_OPERATION
      descanso;
    ...
    caso PKT_PROCESS_CONDITIONALLY
      si (longitud_paquete <9)
        ir a PACKET_ERROR;
      if (condición_paquete que involucra el paquete [4])
      {
        longitud_paquete - = 5;
        memmove (paquete, paquete + 5, longitud_paquete);
        ir a REPARSE_PACKET;
      }
      más
      {
        paquete [0] = PKT_CONDITION_SKIPPED;
        paquete [4] = longitud_paquete;
        longitud_paquete = 5;
        packet_status = READY_TO_SEND;
      }
      descanso;
    ...
    defecto:
    {
     PACKET_ERROR:
      packet_error_count ++;
      longitud_paquete = 4;
      paquete [0] = PKT_ERROR;
      packet_status = READY_TO_SEND;
      descanso;
    }
  }   

Aunque se podrían reemplazar las gotodeclaraciones con {handle_error(); break;}, y aunque se podría usar un do/ while(0)loop junto con continuepara procesar el paquete de ejecución condicional envuelto, realmente no creo que sea más claro que usar un goto. Además, si bien es posible copiar el código de PACKET_ERRORtodos los lugares donde goto PACKET_ERRORse usa, y mientras que un compilador puede escribir el código duplicado una vez y reemplazar la mayoría de las ocurrencias con un salto a esa copia compartida, el uso de gotohace que sea más fácil notar los lugares que configura el paquete de forma un poco diferente (por ejemplo, si la instrucción "ejecutar condicionalmente" decide no ejecutar).

Personalmente, soy un seguidor de "El poder de diez - Diez reglas para escribir un código crítico de seguridad" .

Incluiré un pequeño fragmento de ese texto que ilustra lo que creo que es una buena idea sobre goto.

Regla: Restrinja todo el código a construcciones de flujo de control muy simples; no utilice sentencias goto, construcciones setjmp o longjmp y recursividad directa o indirecta.

Justificación: Un flujo de control más simple se traduce en capacidades más sólidas para la verificación y, a menudo, resulta en una mayor claridad del código. El destierro de la recursividad es quizás la mayor sorpresa aquí. Sin embargo, sin recursividad, tenemos la garantía de tener un gráfico de llamada de función acíclica, que puede ser explotado por analizadores de código y puede ayudar directamente a demostrar que todas las ejecuciones que deberían estar limitadas son de hecho limitadas. (Tenga en cuenta que esta regla no requiere que todas las funciones tengan un único punto de retorno, aunque esto a menudo también simplifica el flujo de control. Sin embargo, hay suficientes casos en los que un retorno de error temprano es la solución más simple).

Desterrar el uso de goto parece malo pero:

Si las reglas parecen draconianas al principio, tenga en cuenta que están destinadas a hacer posible verificar el código donde, literalmente, su vida puede depender de su corrección: código que se usa para controlar el avión en el que vuela, la planta de energía nuclear a pocas millas de donde vives, o la nave espacial que lleva a los astronautas a la órbita. Las reglas actúan como el cinturón de seguridad en su automóvil: inicialmente son quizás un poco incómodas, pero después de un tiempo su uso se vuelve una segunda naturaleza y no usarlas se vuelve inimaginable.

Estoy de acuerdo en que la limpieza goto en orden inverso dado en la pregunta es la forma más limpia de limpiar las cosas en la mayoría de las funciones. Pero también quería señalar que, a veces, desea que su función se limpie de todos modos. En estos casos utilizo la siguiente variante if if (0) {label:} modismo para ir al punto correcto del proceso de limpieza:

int decode ( char * path_in , char * path_out )
{
  FILE * in , * out ;
  code c ;
  int len ;
  int res = 0  ;
  if ( path_in == NULL )
    in = stdin ;
  else
    {
      if ( ( in = fopen ( path_in , "r" ) ) == NULL )
        goto error_open_file_in ;
    }
  if ( path_out == NULL )
    out = stdout ;
  else
    {
      if ( ( out = fopen ( path_out , "w" ) ) == NULL )
        goto error_open_file_out ;
    }

  if( read_code ( in , & c , & longueur ) )
    goto error_code_construction ;

  if ( decode_h ( in , c , out , longueur ) )
  goto error_decode ;

  if ( 0 ) { error_decode: res = 1 ;}
  free_code ( c ) ;
  if ( 0 ) { error_code_construction: res = 1 ; }
  if ( out != stdout ) fclose ( stdout ) ;
  if ( 0 ) { error_open_file_out: res = 1 ; }
  if ( in != stdin ) fclose ( in ) ;
  if ( 0 ) { error_open_file_in: res = 1 ; }
  return res ;
 }

Me parece que cleanup_3debería hacer su limpieza, luego llamar cleanup_2. Del mismo modo, cleanup_2debería hacer su limpieza, luego llamar a cleanup_1. Parece que siempre que lo haga cleanup_[n], cleanup_[n-1]es necesario, por lo que debería ser responsabilidad del método (de modo que, por ejemplo, cleanup_3nunca se pueda llamar sin llamar cleanup_2y posiblemente causar una fuga).

Dado ese enfoque, en lugar de gotos, simplemente llamaría a la rutina de limpieza y luego regresaría.

Sin embargo, el gotoenfoque no es incorrecto ni está mal , solo vale la pena señalar que no es necesariamente el enfoque "más limpio" (en mi humilde opinión).

Si está buscando el rendimiento óptimo, supongo que la gotosolución es la mejor. Sin embargo, solo espero que sea relevante en unas pocas aplicaciones seleccionadas, críticas para el rendimiento (por ejemplo, controladores de dispositivos, dispositivos integrados, etc.). De lo contrario, es una microoptimización que tiene menor prioridad que la claridad del código.

Creo que la pregunta aquí es falaz con respecto al código dado.

Considerar:

1. do_something (), init_stuff () y prepare_stuff () parecen saber si han fallado, ya que devuelven false o nil en ese caso.
2. La responsabilidad de establecer el estado parece ser responsabilidad de esas funciones, ya que no se está configurando ningún estado directamente en foo ().

Por lo tanto: do_something (), init_stuff () y prepare_stuff () deberían estar haciendo su propia limpieza . Tener una función cleanup_1 () separada que limpia después de do_something () rompe la filosofía de encapsulación. Es un mal diseño.

Si hicieron su propia limpieza, entonces foo () se vuelve bastante simple.

Por otra parte. Si foo () realmente creara su propio estado que necesitaba ser derribado, entonces goto sería apropiado.

Esto es lo que he preferido:

bool do_something(void **ptr1, void **ptr2)
{
    if (!ptr1 || !ptr2) {
        err("Missing arguments");
        return false;
    }
    bool ret = false;

    //Pointers must be initialized as NULL
    void *some_pointer = NULL, *another_pointer = NULL;

    if (allocate_some_stuff(&some_pointer) != STUFF_OK) {
        err("allocate_some_stuff step1 failed, abort");
        goto out;
    }
    if (allocate_some_stuff(&another_pointer) != STUFF_OK) {
        err("allocate_some_stuff step 2 failed, abort");
        goto out;
    }

    void *some_temporary_malloc = malloc(1000);

    //Do something with the data here
    info("do_something OK");

    ret = true;

    // Assign outputs only on success so we don't end up with
    // dangling pointers
    *ptr1 = some_pointer;
    *ptr2 = another_pointer;
out:
    if (!ret) {
        //We are returning an error, clean up everything
        //deallocate_some_stuff is a NO-OP if pointer is NULL
        deallocate_some_stuff(some_pointer);
        deallocate_some_stuff(another_pointer);
    }
    //this needs to be freed every time
    free(some_temporary_malloc);
    return ret;
}

Sin embargo, una vieja discusión ... ¿qué pasa con el uso de "anti patrón de flecha" y encapsular más tarde cada nivel anidado en una función estática en línea? El código se ve limpio, es óptimo (cuando las optimizaciones están habilitadas) y no se usa goto. En resumen, divide y vencerás. A continuación un ejemplo:

static inline int foo_2(int bar)
{
    int return_value = 0;
    if ( prepare_stuff( bar ) ) {
        return_value = do_the_thing( bar );
    }
    cleanup_3();
    return return_value;
}

static inline int foo_1(int bar)
{
    int return_value = 0;
    if ( init_stuff( bar ) ) {
        return_value = foo_2(bar);
    }
    cleanup_2();
    return return_value;
}

int foo(int bar)
{
    int return_value = 0;
    if (do_something(bar)) {
        return_value = foo_1(bar);
    }
    cleanup_1();
    return return_value;
}

En términos de espacio, estamos creando tres veces la variable en la pila, lo cual no es bueno, pero esto desaparece al compilar con -O2 eliminando la variable de la pila y usando un registro en este simple ejemplo. Lo que obtuve del bloque anterior gcc -S -O2 test.cfue el siguiente:

    .section    __TEXT,__text,regular,pure_instructions
    .macosx_version_min 10, 13
    .globl  _foo                    ## -- Begin function foo
    .p2align    4, 0x90
_foo:                                   ## @foo
    .cfi_startproc
## %bb.0:
    pushq   %rbp
    .cfi_def_cfa_offset 16
    .cfi_offset %rbp, -16
    movq    %rsp, %rbp
    .cfi_def_cfa_register %rbp
    pushq   %r14
    pushq   %rbx
    .cfi_offset %rbx, -32
    .cfi_offset %r14, -24
    movl    %edi, %ebx
    xorl    %r14d, %r14d
    xorl    %eax, %eax
    callq   _do_something
    testl   %eax, %eax
    je  LBB0_6
## %bb.1:
    xorl    %r14d, %r14d
    xorl    %eax, %eax
    movl    %ebx, %edi
    callq   _init_stuff
    testl   %eax, %eax
    je  LBB0_5
## %bb.2:
    xorl    %r14d, %r14d
    xorl    %eax, %eax
    movl    %ebx, %edi
    callq   _prepare_stuff
    testl   %eax, %eax
    je  LBB0_4
## %bb.3:
    xorl    %eax, %eax
    movl    %ebx, %edi
    callq   _do_the_thing
    movl    %eax, %r14d
LBB0_4:
    xorl    %eax, %eax
    callq   _cleanup_3
LBB0_5:
    xorl    %eax, %eax
    callq   _cleanup_2
LBB0_6:
    xorl    %eax, %eax
    callq   _cleanup_1
    movl    %r14d, %eax
    popq    %rbx
    popq    %r14
    popq    %rbp
    retq
    .cfi_endproc
                                        ## -- End function

.subsections_via_symbols

Prefiero usar la técnica descrita en el siguiente ejemplo ...

struct lnode *insert(char *data, int len, struct lnode *list) {
    struct lnode *p, *q;
    uint8_t good;
    struct {
            uint8_t alloc_node : 1;
            uint8_t alloc_str : 1;
    } cleanup = { 0, 0 };

    // allocate node.
    p = (struct lnode *)malloc(sizeof(struct lnode));
    good = cleanup.alloc_node = (p != NULL);

    // good? then allocate str
    if (good) {
            p->str = (char *)malloc(sizeof(char)*len);
            good = cleanup.alloc_str = (p->str != NULL);
    }

    // good? copy data
    if(good) {
            memcpy ( p->str, data, len );
    }

    // still good? insert in list
    if(good) {
            if(NULL == list) {
                    p->next = NULL;
                    list = p;
            } else {
                    q = list;
                    while(q->next != NULL && good) {
                            // duplicate found--not good
                            good = (strcmp(q->str,p->str) != 0);
                            q = q->next;
                    }
                    if (good) {
                            p->next = q->next;
                            q->next = p;
                    }
            }
    }

    // not-good? cleanup.
    if(!good) {
            if(cleanup.alloc_str)   free(p->str);
            if(cleanup.alloc_node)  free(p);
    }

    // good? return list or else return NULL
    return (good? list: NULL);

}

fuente: http://blog.staila.com/?p=114

Usamos la Daynix CStepsbiblioteca como otra solución para el " problema goto " en las funciones de inicio.
Vea aquí y aquí .