sentencia - ¿Uso válido de goto para la gestión de errores en C?
return en c (14)
Como regla general, evitar goto es una buena idea, pero los abusos que prevalecían cuando Dijkstra escribió por primera vez ''GOTO Considered Harmful'' ni siquiera cruzan la mente de la mayoría de la gente como una opción en estos días.
Lo que delineas es una solución generalizable al problema del manejo de errores; me sirve 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 se ve 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 que deberían ser); por ejemplo, con frecuencia hay más estado para pasar a las rutinas de inicialización (configuración) que las que se muestran, y por lo tanto más estado para pasar a las rutinas de limpieza también.
Esta pregunta es en realidad el resultado de una discusión interesante 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 goto aquí parece ser el mejor camino a seguir, lo que da como resultado 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 Code Complete :
El 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. El goto reduce la probabilidad de que te olvides de desasignar los recursos en cada lugar donde detectas un error.
Otro soporte para este enfoque proviene del libro Controladores de dispositivo de Linux , en esta sección .
¿Qué piensas? ¿Es este caso un uso válido para goto en C? ¿Preferiría otros métodos, que producen un código más complicado o menos eficiente, pero evite goto ?
Creo que la pregunta aquí es falaz con respecto al código dado.
Considerar:
- do_something (), init_stuff () y prepare_stuff () parecen saber si han fallado, ya que devuelven falso o nulo en ese caso.
- La responsabilidad de configurar el estado parece ser la 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 hacer su propia limpieza . Tener una función cleanup_1 () separada que se 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 creó su propio estado que necesitaba ser derribado, entonces sería apropiado.
El problema con la palabra clave goto es en su mayoría incomprendido. No es simple, malvado. Solo necesita conocer 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 tutoriales de developer.apple.com, ellos toman el enfoque goto para el manejo de errores.
No usamos gotos. Se le da mayor importancia a los valores de retorno. El manejo de excepciones se realiza a través de setjmp/longjmp , lo poco que pueda.
En general, consideraría el hecho de que un fragmento de código podría escribirse más claramente usando goto como un síntoma de que el flujo del programa es probablemente más complicado de lo que generalmente es deseable. La combinación de otras estructuras de programas de formas extrañas para evitar el uso de goto trataría de tratar el síntoma, en lugar de la enfermedad. Su ejemplo particular podría no ser demasiado difícil de implementar sin goto :
do { .. set up thing1 that will need cleanup only in case of early exit if (error) break; do { .. set up thing2 that will need cleanup in case of early exit if (error) break; // ***** SEE TEXT REGARDING THIS LINE } while(0); .. cleanup thing2; } while(0); .. cleanup thing1;
pero si la limpieza solo se suponía que ocurriría cuando la función fallara, el caso de goto podría manejarse poniendo un return justo antes de la primera etiqueta de destino. El código anterior requeriría agregar un return en la línea marcada con ***** .
En el escenario de "limpieza incluso en casos normales", consideraría el uso de goto como más claro que los constructos do / while(0) , entre otras cosas porque las propias etiquetas de destino prácticamente gritan "MIRENME" mucho más que las construcciones break y do / while(0) . Para el caso de "limpieza solo si hay error", la declaración de return 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 principio de una función o en lo que "parece" " el fin); tener un return justo antes de que una etiqueta de destino cumpla 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 goto para el manejo de errores está dentro de una declaración de switch , cuando el código para múltiples casos comparte el mismo código de error. Aunque mi compilador suele ser lo suficientemente inteligente como para reconocer que múltiples casos terminan con el mismo código, creo que es más claro decir:
REPARSE_PACKET: switch(packet[0]) { case PKT_THIS_OPERATION: if (problem condition) goto PACKET_ERROR; ... handle THIS_OPERATION break; case PKT_THAT_OPERATION: if (problem condition) goto PACKET_ERROR; ... handle THAT_OPERATION break; ... case PKT_PROCESS_CONDITIONALLY if (packet_length < 9) goto PACKET_ERROR; if (packet_condition involving packet[4]) { packet_length -= 5; memmove(packet, packet+5, packet_length); goto REPARSE_PACKET; } else { packet[0] = PKT_CONDITION_SKIPPED; packet[4] = packet_length; packet_length = 5; packet_status = READY_TO_SEND; } break; ... default: { PACKET_ERROR: packet_error_count++; packet_length = 4; packet[0] = PKT_ERROR; packet_status = READY_TO_SEND; break; } }
Aunque uno podría reemplazar las declaraciones goto con {handle_error(); break;} {handle_error(); break;} , y aunque uno podría usar un ciclo do / while(0) junto con continue procesando 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_ERROR todas partes, se usa goto PACKET_ERROR , 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 goto hace más fácil de detectar los lugares que configuran el paquete un poco diferente (por ejemplo, si la instrucción "ejecutar condicionalmente" decide no ejecutar).
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;
}
Estoy de acuerdo con que la limpieza goto en el orden inverso que se da en la pregunta es la manera 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 solucione de todos modos. En estos casos, utilizo la siguiente variante if if (0) {label:} idioma 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 ;
}
FWIF, considero que el lenguaje de manejo de errores que proporcionó 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 el momento. Si bien goto es una mala idea en general, puede ser útil para el manejo de errores cuando se realiza de una manera simple y uniforme. En esta situación, a pesar de que es un goto , se está utilizando de una manera bien definida y más o menos estructurada.
GOTO es útil. Es algo que su procesador puede hacer y es por eso que debería tener acceso a él.
A veces quieres agregar algo a tu función y solo ir, hazlo fácilmente. Puede ahorrar tiempo ...
Me parece que cleanup_3 debería hacer su limpieza, luego llamar a cleanup_2 . Del mismo modo, cleanup_2 debería hacer su limpieza, luego invocar cleanup_1. Parece que en cualquier momento que haga cleanup_[n] , esa cleanup_[n-1] es obligatoria, por lo tanto, debería ser responsabilidad del método (para que, por ejemplo, cleanup_3 no pueda ser llamado sin llamar a cleanup_2 y posiblemente cause una fuga) .)
Dado ese enfoque, en lugar de gotos, simplemente llama a la rutina de limpieza y luego regresa.
El enfoque goto no es malo o malo , sin embargo, 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, entonces supongo que la solución goto es la mejor. Sin embargo, solo espero que sea relevante en unas pocas aplicaciones de rendimiento crítico (por ejemplo, controladores de dispositivos, dispositivos integrados, etc.). De lo contrario, es una micro-optimización que tiene menor prioridad que la claridad del código.
Me sorprende que nadie haya sugerido esta alternativa, por lo tanto, aunque la pregunta lleva tiempo, la agregaré: una buena forma de abordar este problema es usar variables para hacer un seguimiento del estado actual. Esta es una técnica que se puede usar ya sea que se use o no goto para llegar al código de limpieza. Al igual que cualquier técnica de codificación, tiene sus pros y sus contras, y no será adecuada para cada situación, pero si elige un estilo, vale la pena considerarlo, especialmente si quiere evitar goto sin terminar anidado profundamente if s.
La idea básica es que, por cada acción de limpieza que pueda necesitarse, hay una variable de cuyo valor podemos decir si la limpieza necesita o no.
Primero mostraré la versión goto , porque está más cerca del código en 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, utilizando el método de switch descrito en otra respuesta, si el orden de inicialización cambia, entonces el switch debe ser editado cuidadosamente para evitar tratar de limpiar algo que en realidad no se inicializó 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 es cierto, pero en la práctica a menudo una variable existente ya rastrea, o se puede hacer para rastrear, el estado requerido. Por ejemplo, si el prepare_stuff() es en realidad una llamada a malloc() o a open() , 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 además rastreamos el estado de error con una variable, podemos evitar goto completamente, y aún así limpiar correctamente, sin tener una sangría que se profundice más y más la inicialización que 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;
}
Una vez más, hay críticas potenciales de esto:
- ¿No todos esos "si" perjudican el rendimiento? No, porque en el caso de éxito, debe hacer todos los controles de todos modos (de lo contrario, no está verificando todos los casos de error); y en el caso de falla, la mayoría de los compiladores optimizarán la secuencia de fallas
if (oksofar)comprueba 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? En este caso, sí, pero a menudo la variable
return_valuese puede usar para jugar el rol que está jugandooksofaraquí. Si estructura sus funciones para devolver errores de manera consistente, puede incluso evitar el segundoifen 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 de esta manera es que la coherencia significa que cualquier lugar donde el programador original olvidó 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. Usado correctamente permite un código muy limpio y consistente, y como cualquier técnica, en las manos equivocadas puede terminar produciendo código que es largo y confuso :-)
No hay nada moralmente incorrecto sobre la declaración de goto más de lo que hay algo moralmente incorrecto con punteros (vacíos) *.
Todo depende de cómo uses la herramienta. En el caso (trivial) que presentó, una declaración de caso puede lograr la misma lógica, aunque con más sobrecarga. La verdadera pregunta es, "¿cuál es mi requerimiento de velocidad?"
Goto es simplemente rápido, especialmente si tienes cuidado de asegurarte de que compila a un salto corto. Perfecto para aplicaciones donde la velocidad es alta. Para otras aplicaciones, probablemente tenga sentido tomar el golpe aéreo con if / else + case para mantenimiento.
Recuerde: goto no mata aplicaciones, los desarrolladores matan aplicaciones.
ACTUALIZACIÓN: Aquí está el ejemplo del 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 ;
}
}
Prefiero usar la técnica que se describe 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 biblioteca Daynix CSteps como otra solución para el " problema goto " en las funciones init.
Mira here y aquí .
Personalmente soy seguidor de "El poder de diez: 10 reglas para escribir un código de seguridad crítica" .
Incluiré un pequeño fragmento de ese texto que ilustre lo que creo que es una buena idea sobre goto.
Regla: Restringir todo el código a construcciones de flujo de control muy simples: no use declaraciones goto, construcciones setjmp o longjmp y recursión 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, da como resultado una mejor claridad del código. El destierro de la recursividad es quizás la mayor sorpresa aquí. Sin recursion, sin embargo, tenemos la garantía de tener un gráfico de llamadas 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 de hecho están limitadas. (Tenga en cuenta que esta regla no requiere que todas las funciones tengan un solo 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 permitir la verificación del código donde, literalmente, su vida puede depender de su corrección: código que se utiliza para controlar el avión que usted vuela, la planta de energía nuclear a unos pocos kilómetros 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 de su automóvil: al principio son tal vez un poco incómodas, pero después de un tiempo su uso se convierte en una segunda naturaleza y el no usarlas se vuelve inimaginable.