¿Cuál es la mejor manera de liberar memoria después de regresar de un error?

Question

Supongamos que tengo una función que asigna memoria para la persona que llama:

int func(void **mem1, void **mem2) { 
    *mem1 = malloc(SIZE); 
    if (!*mem1) return 1; 

    *mem2 = malloc(SIZE); 
    if (!*mem2) { 
     /* ... */ 
     return 1; 
    } 

    return 0; 
} 

me gustaría escuchar sus comentarios sobre la mejor manera de liberar() la memoria asignada en el caso de la segunda malloc() falla. Puede imaginar una situación más elaborada con más puntos de salida de error y más memoria asignada.

Answer 1

Sé que la gente se muestran reacios a utilizarlos, pero esta es la situación perfecta para una goto en C.

int func(void** mem1, void** mem2) 
{ 
    int retval = 0; 
    *mem1 = malloc(SIZE); 
    if (!*mem1) { 
     retval = 1; 
     goto err; 
    } 

    *mem2 = malloc(SIZE); 
    if (!*mem2) { 
     retval = 1; 
     goto err; 
    } 
// ...  
    goto out; 
// ... 
err: 
    if(*mem1) free(*mem1); 
    if(*mem2) free(*mem2); 
out: 
    return retval; 
}  

Answer 2

Mi propia inclinación es crear una función de argumentos variable que libera todos los punteros no NULL. A continuación, la persona que llama puede manejar el caso de error:

void *mem1 = NULL; 
void *mem2 = NULL; 

if (func(&mem1, &mem2)) { 
    freeAll(2, mem1, mem2); 
    return 1; 
} 

Answer 3

¿La persona que llama hacer algo útil con los bloques de memoria que han sido asignados correctamente antes del fallo? Si no, el destinatario debería manejar la desasignación.

Una posibilidad para hacer la limpieza eficiente es el uso de do..while(0), lo que permite a break donde sus ejemplo return s:

int func(void **mem1, void **mem2) 
{ 
    *mem1 = NULL; 
    *mem2 = NULL; 

    do 
    { 
     *mem1 = malloc(SIZE); 
     if(!*mem1) break; 

     *mem2 = malloc(SIZE); 
     if(!*mem2) break; 

     return 0; 
    } while(0); 

    // free is NULL-safe 
    free(*mem1); 
    free(*mem2); 

    return 1; 
} 

Si usted hace un montón de las asignaciones, es posible que desee utilizar su función freeAll() hacer la limpieza aquí también.

Answer 4

Aquí es donde un Goto es apropiado, en mi opinión. Solía ​​seguir el dogma anti-goto, pero lo cambié cuando me fue indicado que hago {...} mientras (0); compila al mismo código, pero no es tan fácil de leer. Sólo tiene que seguir las reglas básicas, como no ir hacia atrás con ellos, manteniéndolos al mínimo, sólo con ellos por las condiciones de error, etc ...

int func(void **mem1, void **mem2) 
{ 
    *mem1 = NULL; 
    *mem2 = NULL; 

    *mem1 = malloc(SIZE); 
    if(!*mem1) 
     goto err; 

    *mem2 = malloc(SIZE); 
    if(!*mem2) 
     goto err; 

    return 0; 
err: 
    if(*mem1) 
     free(*mem1); 
    if(*mem2) 
     free(*mem2); 

    *mem1 = *mem2 = NULL; 

    return 1; 
} 

Answer 5

Esto es un poco controvertido, pero creo que el enfoque goto utilizado en el núcleo de Linux realmente funciona bastante bien en esta situación:

int get_item(item_t* item) 
{ 
    void *mem1, *mem2; 
    int ret=-ENOMEM; 
    /* allocate memory */ 
    mem1=malloc(...); 
    if(mem1==NULL) goto mem1_failed; 

    mem2=malloc(...); 
    if(mem2==NULL) goto mem2_failed; 

    /* take a lock */ 
    if(!mutex_lock_interruptible(...)) { /* failed */ 
    ret=-EINTR; 
    goto lock_failed; 
    } 

    /* now, do the useful work */ 
    do_stuff_to_acquire_item(item); 
    ret=0; 

    /* cleanup */ 
    mutex_unlock(...); 

lock_failed: 
    free(mem2); 

mem2_failed: 
    free(mem1); 

mem1_failed: 
    return ret; 
} 

Answer 6

estoy un poco horrorizado por todas las recomendaciones para una instrucción goto!

he encontrado que el uso de Goto conduce a código confuso que es más probable que dé lugar a programador errores. Mi preferencia ahora es evitar su uso por completo, excepto en las situaciones más extremas. Casi nunca lo usaría. No por el perfeccionismo académico, sino porque un año o más después siempre parece más difícil recordar la lógica general que la alternativa que voy a sugerir.

la misma naturaleza que ama a refactorizar cosas para minimizar mi opción de olvidar cosas (como despejar un puntero), me gustaría añadir unas pocas funciones en primer lugar. Presumiré que es probable que los reutilice un poco en el mismo programa. La función imalloc() haría la operación malloc con el puntero indirecto; ifree() desharía esto. cifree() liberaría la memoria de forma condicional.

Con eso en la mano, mi versión del código (con un tercer arg, por el bien de la demostración) sería la siguiente:

// indirect pointer malloc 
int imalloc(void **mem, size_t size) 
{ 
    return (*mem = malloc(size)); 
} 

// indirect pointer free 
void ifree(void **mem) 
{ 
    if(*mem) 
    { 
    free(*mem); 
    *mem = NULL; 
    } 
} 

// conditional indirect pointer free 
void cifree(int cond, void **mem) 
{ 
    if(!cond) 
    { 
    ifree(mem); 
    } 
} 

int func(void **mem1, void **mem2, void **mem3) 
{ 
    int result = FALSE; 
    *mem1 = NULL; 
    *mem2 = NULL; 
    *mem3 = NULL; 

    if(imalloc(mem1, SIZE)) 
    { 
    if(imalloc(mem2, SIZE)) 
    { 
     if(imalloc(mem3, SIZE)) 
     { 
     result = TRUE; 
     }    
     cifree(result, mem2); 
    } 
    cifree(result, mem1); 
    } 
    return result; 
} 

Yo prefiero tener sólo un retorno de una función, en el fin. Saltar en el medio es rápido (y en mi opinión, algo sucio).Pero lo que es más importante, le permite evitar fácilmente el código de limpieza asociado involuntariamente.

Answer 7

Si las sentencias goto anteriores se horrorizan por alguna razón, siempre se puede hacer algo como esto:

int func(void **mem1, void **mem2) 
{ 
    *mem1 = malloc(SIZE); 
    if (!*mem1) return 1; 

    *mem2 = malloc(SIZE); 
    if (!*mem2) { 
     /* Insert free statement here */ 
     free(*mem1); 
     return 1; 
    } 

    return 0; 
} 

utilizo este método con bastante regularidad, pero sólo cuando está muy claro lo que está pasando.

Answer 8

Esta es una alternativa legible:

int func(void **mem1, void **mem2) { 
    *mem1 = malloc(SIZE); 
    *mem2 = malloc(SIZE); 
    if (!*mem1 || !*mem2) { 
    free(*mem2); 
    free(*mem1); 
    return 1; 
    } 
    return 0; 
} 

Answer 9

lo personal; Tengo una biblioteca de seguimiento de recursos (básicamente un árbol binario equilibrado) y tengo envoltorios para todas las funciones de asignación.

Los recursos (como memoria, sockets, descriptores de archivos, semáforos, etc., cualquier cosa que asigne y desasigne) pueden pertenecer a un conjunto.

También tengo una biblioteca de manejo de errores, donde el primer argumento para cada función es un conjunto de errores y si algo sale mal, la función que experimenta un error envía un error al conjunto de errores.

Si un conjunto de errores contiene un error, ninguna función ejecuta. (Tengo una macro en la parte superior de cada función que hace que vuelva).

Así que múltiples mallocs se ven así;

mem[0] = malloc_wrapper(error_set, resource_set, 100); 
mem[1] = malloc_wrapper(error_set, resource_set, 50); 
mem[2] = malloc_wrapper(error_set, resource_set, 20); 

No hay necesidad de comprobar el valor de retorno, ya que si se produce un error, no hay siguientes funciones se ejecutarán, por ejemplo, los siguientes mallocs nunca ocurren.

Por lo tanto, cuando llegue el momento de desasignar recursos (por ejemplo, al final de una función, donde todos los recursos utilizados internamente por esa función se han colocado en un conjunto), desasigno el conjunto. Es solo una llamada de función.

res_delete_set(resource_set); 

No necesito para comprobar específicamente para errores - hay ninguna si() s en mi código de comprobación de valores de retorno, lo que hace que sea fácil de mantener; Encuentro que la vida útil del control de errores en línea destruye la legibilidad y, por lo tanto, el mantenimiento. Solo tengo una lista simple de llamadas a funciones.

Es arte, el hombre :-)