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

liberar espacio memoria interna android

Question

Supongamos que tengo una función que asigna memoria a 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 forma de liberar () la memoria asignada en caso de que falle el segundo malloc (). Puede imaginar una situación más elaborada con más puntos de salida de error y más memoria asignada.

Answer 1

¿La persona que llama hace algo útil con los bloques de memoria que se han asignado correctamente antes de la falla? Si no, el destinatario debería manejar la desasignación.

Una posibilidad para hacer la limpieza de manera eficiente es usar do..while(0) , que permite break donde su 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 realiza muchas asignaciones, es posible que desee utilizar su función freeAll() para realizar la limpieza aquí también.

Answer 2

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. Solo sigue algunas reglas básicas, como no retroceder con ellas, manteniéndolas al mínimo, solo usándolas para 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 3

Esto es un poco controvertido, pero creo que el enfoque goto usado en el kernel 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 4

Mi propia inclinación es crear una función de argumento variable que libere todos los punteros que no sean NULL. Entonces 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 5

¡Estoy un poco horrorizado por todas las recomendaciones para una declaración goto!

He descubierto que el uso de goto conduce a código confuso que es más probable que dé lugar a errores del programador. 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.

Ser uno que adora refaccionar las cosas para minimizar mi opción de olvidar cosas (como borrar un puntero), añadiría algunas funciones primero. 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 mano, mi versión del código (con un tercer arg, por el bien de la demostración) sería así:

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

Prefiero tener solo un retorno de una función, al final. 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 6

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 7

Personalmente; 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, no se ejecutarán las funciones . (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 es necesario verificar el valor de retorno, ya que si ocurre un error, no se ejecutarán las siguientes funciones , por ejemplo, nunca se producirán los siguientes mallocs.

Entonces, cuando llega el momento de desasignar los 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 verificar específicamente los errores: no hay if () s en mi código que comprueba los valores de retorno, lo que hace que pueda mantenerse; 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 , hombre :-)

Answer 8

Si las anteriores declaraciones goto te horrorizan por alguna razón, siempre puedes 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 bastante regularmente, pero solo cuando está muy claro lo que está sucediendo.

Answer 9

Sé que las personas detestan usarlas, pero esta es la situación perfecta para un 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;
}