2010-09-14 23 views
10

gcc 4.4.3 c89gestión de sentencias if

Tengo algunas funciones que inicializan algunos hardware y devuelven verdadero o falso. Si es falso, entonces tengo que desinicializar en el orden inverso.

Sin embargo, mi código se ve muy desordenado con todas las declaraciones if.

Por ejemplo, cada función puede ser verdadera o falsa. Esta es una muestra Como puede ver, el código se ve muy desordenado. Solo estoy buscando algún consejo sobre cómo puedo limpiarlo para hacerlo más manejable y, si es posible, escabroso.

Muchas gracias por cualquier consejo,

if(init_A() == TRUE) { 
if(init_B() == TRUE) { 
    if(init_C() == TRUE) { 
    if(init_D() == TRUE) { 
    if(init_E() == TRUE) { 
    /* ALL STARTED OK */  
    } 
    else { 
    uninit_A(); 
    uninit_B(); 
    uninit_C(); 
    uninit_D();  
    } 
    } 
    else { 
    uninit_A(); 
    uninit_B(); 
    uninit_C(); 
    } 
    } 
    else { 
    uninit_A(); 
    uninit_B(); 
    } 
} 
else { 
    /* Failed to initialize B */ 
    uninit_B(); 
} 
} 
else { 
/* Failed to start */ 
} 
+1

Entonces, si llega a 'TODO COMIENZO OK', ¿nunca va a desconectar las cosas? – GManNickG

+0

Una vez que el programa está marcado para el apagado, se inicializará en el orden inverso. Sin embargo, no lo especifiqué en la muestra. Como quería que fuera simple. Gracias. – ant2009

+1

Puedo estar equivocado, pero ¿no debería el segundo, el último, tener uninit_A(), no uninit_B()? –

Respuesta

8

Este es un problema bastante común, donde los pasos "init" corresponden a cosas como malloc() o lock(), y los pasos "uninit" corresponden a cosas como free() y unlock(). Es particularmente un problema cuando los recursos tienen que ser desasignados en el orden estrictamente inverso en el que fueron asignados.

Este es un caso en que se justifica el uso de goto:

int somefunc() 
{ 
    int retval = ERROR; 

    if (init_A() != TRUE) 
     goto out_a; 

    if (init_B() != TRUE) 
     goto out_b; 

    if (init_C() != TRUE) 
     goto out_c; 

    if (init_D() != TRUE) 
     goto out_d; 

    if (init_E() != TRUE) 
     goto out_e; 

    /* ALL STARTED OK */ 
    /* ... normal processing here ... */ 
    retval = OK; 

    uninit_E(); 
    out_e: 
    uninit_D(); 
    out_d: 
    uninit_C(); 
    out_c: 
    uninit_B(); 
    out_b: 
    uninit_A(); 
    out_a: 
    return retval; 
} 
+0

Así es exactamente como lo hago, un ejemplo: http://code.google.com/p/cpfs/source/browse/cpfs.c#1379 –

7

Me bucle a través de una matriz de punteros de función, llamar a las funciones en el bucle, a continuación, si es que la función devuelve falso, realizan la función uninit_* correspondiente.

He aquí un ejemplo:

void (*inits[5]) (void); 
void (*uninits[4]) (void); 

int main(void) { 
    inits[0] = init_A; 
    inits[1] = init_B; 
    inits[2] = init_C; 
    inits[3] = init_D; 
    inits[4] = init_E; 

    uninits[0] = uninit_A; 
    uninits[1] = uninit_B; 
    uninits[2] = uninit_C; 
    uninits[3] = uninit_D; 

    for(int i = 0; i < 5; i++) { 
     if((*inits[i])() != TRUE) { 
     int j = (i < 4) ? i : 4; 
     while(j--) { 
      (*uninits[j])(); 
     } 
     break; 
     } 
    } 
    return 1; 
} 
+0

En realidad es más complicado de lo que parece. Necesita llamar a uninit_ * para cada función anterior en la matriz si alguna función actual devuelve falso. –

+0

@Daniel, daré un ejemplo pronto. Sugerencia: Utilizaré ** dos ** matrices de indicadores de función, con los índices correspondientes. –

+0

@Jacob. La forma en que funciona el hardware es inicializar todo primero. y cuando desee apagar la unidad, revísela en el orden inverso. Sin embargo, si una función no se inicializa, entonces tengo que desinicializar a partir de la función que falló. Gracias. – ant2009

25
if(init_A() != TRUE) { 
    goto EndA; 
} 
if(init_B() != TRUE) { 
    goto EndB; 
} 
if(init_C() != TRUE) { 
    goto EndC; 
} 
if(init_D() != TRUE) { 
    goto EndD; 
} 
if(init_E() != TRUE) { 
    goto EndE; 
} 
... 
return; 
EndE: uninitD(); 
EndD: uninitC(); 
EndC: uninitB(); 
EndB: uninitA(); 
EndA: return; 
+0

Pensé en usar las declaraciones de goto. Sin embargo, siempre he tratado de evitarlas. Gracias. – ant2009

+4

@robUK, @Jacob: digo, use la herramienta correcta para la tarea, sin prejuicios. – adamk

+4

+1: conserva la lógica, reduce la duplicación (de llamadas no iniciadas), mejora la legibilidad y tiene el valor de usar 'goto' frente a la inevitable policía goto. –

6
 
BOOL a = FALSE, b = FALSE, c = FALSE, d = FALSE, e = FALSE; 

if ((a = init_A()) && (b = init_B()) && (c = init_C()) && (d = init_D()) && (e = init_E())) 
{ 
} 
else 
{ 
    if (e) uninit_E(); 
    if (d) uninit_D(); 
    if (c) uninit_C(); 
    if (b) uninit_B(); 
    if (a) uninit_A(); 
} 

funciones UNINIT también llamados en orden directa, como en el código. Si se requiere orden inverso, simplemente cambie esto.

+0

¿Todas las implementaciones verifican la expresión ANDed de izquierda a derecha y se detienen en cero?Pensé que el sistema podría evaluar TODAS las expresiones ANDed antes de continuar. –

+2

@crypto: lógico AND (y lógico OR) debe evaluarse de izquierda a derecha, ya que tienen un comportamiento de cortocircuito: si el lado izquierdo determina completamente el resultado final, el lado derecho debe omitirse por completo. –

+0

@Bart van Ingen Schenau, gracias por la información! –

4

Si sus uninit_* funciones pueden detectar si tienen o no tienen que hacer nada que pueda simplemente:

if (!init_A() || !init_B() || !init_C() || !init_D()) 
{ 
    uninit_C(); 
    uninit_B(); 
    uninit_A(); 
    return FALSE; 
} 
+0

Esta es la solución más limpia aquí. imo – Erik

2

comprensión limitada de C en el trabajo aquí, si usted decide downvote, por favor dígame por qué.

#include <stdio.h> 

int init_a() { return 1; }; // succeed 
int init_b() { return 1; }; // succeed 
int init_c() { return 0; }; // fail 

void uninit_a() { printf("uninit_a()\n"); } 
void uninit_b() { printf("uninit_b()\n"); } 
void uninit_c() { printf("uninit_c()\n"); } 

typedef struct _fp { 
     int (*init)(); 
     void (*uninit)(); 
} fp; 

int init() { 
     fp fps[] = { 
       (fp){&init_a, &uninit_a}, 
       (fp){&init_b, &uninit_b}, 
       (fp){&init_c, &uninit_c} 
     }; 

     unsigned int i = 0, j; 
     for(; i < sizeof(fps)/sizeof(fp); ++i) { 
       if(!(*fps[i].init)()) { 
         for(j = 0; j < i; ++j) { 
           (*fps[j].uninit)(); 
         } 
         return -1; 
       } 
     } 
     return 0; 
} 

int main() { 
     init(); 
     return 0; 
} 

Salida:

uninit_a() 
uninit_b() 

Este es el mismo orden que el código en el post original sería ejecutado, pero es posible que desee revertirla (bucle interno).

+1

Esto es demasiado complicado IMO. El código puede funcionar, pero es difícil de leer sin pensarlo mucho. Otras soluciones presentadas aquí son mucho más fáciles de entender. –

+0

Creo que puedes llamar '(* fps [i] .init)()' como 'fps [i] .init()', lo mismo para 'uninit', y no necesitas el molde para' (fp) 'en la inicialización, pero soy demasiado cobarde cuando se trata de C para simplemente editarlo :) –

1

No tengo un compilador para probar esto. Pero algo como esto podría funcionar?

int (*init[])() = {init_A, init_B, init_C, init_D, init_E}; 
int (*uninit[])() = {uninit_A, uninit_B, uninit_C, uninit_D, uninit_E}; 

int main() 
{ 
    initfunction(init, 0) 
    return 0; 
} 

void initfunction((*init[])(), pos) 
{ 
    if(init[pos]() == TRUE) 
    initfunction(init, pos++) 
    else 
    return; 

    uninit[pos](); 
} 
+0

Una vez que esto llega al final de tu matriz de inicio; Tendrás que decidir qué hacer. –

4

¿Es eso "orden inverso"?Para mí el orden inverso es así:

void uninit(int from) { 
    switch (from) { 
     /* ... */ 
     case 3: uninit_C(); /* fall_through */ 
     case 2: uninit_B(); /* fall_through */ 
     case 1: uninit_A(); /* fall_through */ 
     case 0: break; 
    } 
} 

Y el proceso init iría así

int count = 0; 
    if (init_A()) { 
     count++; 
     if (init_B()) { 
      count++; 
      if(init_C()) { 
       count++; 
       if(init_D()) { 
        count++; 
        if(init_E()) { 
         count++; 
        } 
       } 
      } 
     } 
    } 
    if (count == 5) /* ALL OK */; 
    uninit(count); 
+0

Olvidé un 'predeterminado' en la declaración de cambio. – pmg

2

Lo que tal vez está buscando es "gestión de recursos con destino alcance". C++ tradicionalmente hace eso con constructores/destructores. Pero hay una manera de hacerlo de manera diferente (tanto en C99 como en C++) al abusar un poco de la for-declaración. Escribí algo sobre esta línea aquí: scope bound resource management with for scopes.

1
int X = 0; 
if(init_A() == TRUE) { 
    X++; 
    if(init_B() == TRUE) { 
    X++; 
    if(init_C() == TRUE) { 
     X++; 
     if(init_D() == TRUE) { 
     X++; 
     if(init_E() == TRUE) { 
      X++; 
      /* ALL STARTED OK */  
     } 
     } 
    } 
    } 
} 

/* You said reverse order which I took to mean this, 
* though your did not do it this way. */ 
switch (X) { 
    case 5: 
     return SUCCESS; 
    case 4: 
    uninit_D(); 
    case 3: 
    uninit_C(); 
    case 2: 
    uninit_B(); 
    case 1: 
    uninit_A(); 
    return FAILURE; 
} 

Algo me encuentro haciendo para evitar que a mí mismo de cometer errores en el código de este tipo es:

static int do_A(void); 
static int do_B(void); 
static int do_C(void); 
static int do_D(void); 

static int do_A(void) { 
    if (init_A() == FALSE) { 
     return FALSE; 
    } 
    if (do_B() == FALSE) { 
     uninit_A(); 
     return FALSE; 
    } 
    return TRUE; 
}  

... 

static int do_D(void) { 
    return init_D(); 
} 

Todas las otras funciones do_ debería ser similar a do_A.

Cuestiones relacionadas