punteros - memoria dinamica en c ejemplos

¿Es esta una buena forma de liberar memoria?

Question

La función para liberar una instancia de struct Foo se da a continuación:

void DestroyFoo(Foo* foo)
{
    if (foo) free(foo);
}

En cambio, un colega mío sugirió lo siguiente:

void DestroyFoo(Foo** foo)
{
    if (!(*foo)) return;
    Foo *tmpFoo = *foo;
    *foo = NULL; // prevents future concurrency problems
    memset(tmpFoo, 0, sizeof(Foo));  // problems show up immediately if referred to free memory
    free(tmpFoo);
}

Veo que configurar el puntero en NULL después de liberarlo es mejor, pero no estoy seguro de lo siguiente:

1. ¿Realmente necesitamos asignar el puntero a uno temporal? ¿Ayuda en términos de concurrencia y memoria compartida?

2. ¿Es realmente una buena idea establecer todo el bloque en 0 para forzar el bloqueo del programa o al menos generar resultados con una discrepancia significativa?

Answer 1

¿Realmente necesitamos asignar el puntero a uno temporal? ¿Ayuda en términos de concurrencia y memoria compartida?

No tiene nada que ver con la concurrencia o la memoria compartida. Carece de sentido.

¿Es realmente una buena idea establecer todo el bloque en 0 para forzar el bloqueo del programa o al menos generar resultados con una discrepancia significativa?

No, en absoluto.

La solución sugerida por su colega es terrible. Este es el por qué:

• Establecer todo el bloque en 0 tampoco logra nada. Debido a que alguien está usando un bloque free () ''accidentalmente, no lo sabrían en función de los valores del bloque. Ese es el tipo de bloque que devuelve calloc() . Por lo tanto, es imposible saber si se trata de memoria recién asignada ( calloc() o malloc()+memset() ) o la que ha sido liberada () por su código anteriormente. En todo caso, es un trabajo extra para su programa poner a cero cada bloque de memoria que está siendo liberado () ''ed.

• free(NULL); está bien definido y no funciona, por lo que la condición if(ptr) {free(ptr);} en if(ptr) {free(ptr);} no logra nada.

• Desde free(NULL); es no-op, establecer el puntero en NULL realmente ocultaría ese error, porque si alguna función llama a free() en un puntero ed ya libre (), entonces no lo sabrían.

• la mayoría de las funciones de usuario tendrían una comprobación NULL al inicio y podrían no considerar pasar NULL como condición de error:

void do_some_work(void *ptr) {
    if (!ptr) {
        return; 
    }

   /*Do something with ptr here */
}

Por lo tanto, todas esas comprobaciones adicionales y la reducción a cero dan una falsa sensación de "robustez" mientras que en realidad no mejoró nada. Simplemente reemplazó un problema con otro, el costo adicional del rendimiento y la acumulación de código.

Tan solo llamando free(ptr); sin ninguna función de envoltura es a la vez simple y robusta (la mayoría de las implementaciones de malloc() se bloquearían inmediatamente en doble libre, lo cual es algo bueno )

No hay una forma fácil de evitar "accidentalmente" llamadas free() dos veces o más. Es responsabilidad del programador hacer un seguimiento de toda la memoria asignada y free() adecuadamente. Si alguien encuentra esto difícil de manejar, entonces C probablemente no sea el lenguaje adecuado para ellos.

Answer 2

Desafortunadamente, esta idea simplemente no está funcionando.

Si la intención era atrapar el doble libre, no está cubriendo casos como el siguiente.

Asume este código:

Foo *ptr_1 = (FOO*) malloc(sizeof(Foo));
Foo *ptr_2 = ptr_1;
free (ptr_1);
free (ptr_2); /* This is a bug */

La propuesta es escribir en su lugar:

Foo *ptr_1 = (FOO*) malloc(sizeof(Foo));
Foo *ptr_2 = ptr_1;
DestroyFoo (&ptr_1);
DestroyFoo (&ptr_2); /* This is still a bug */

El problema es que la segunda llamada a DestroyFoo() todavía se bloqueará, porque ptr_2 no se restablece a NULL, y todavía apunta a la memoria ya liberada.

Answer 3

La segunda solución parece estar sobre diseñada. Por supuesto, en alguna situación podría ser más seguro, pero la sobrecarga y la complejidad son demasiado grandes.

Lo que debe hacer si desea estar seguro es configurar el puntero en NULL después de liberar memoria. Esta es siempre una buena práctica.

Foo* foo = malloc( sizeof(Foo) );
DestroyFoo(foo);
foo = NULL;

Además, no sé por qué la gente está verificando si el puntero es NULL antes de llamar a free (). Esto no es necesario ya que free () hará el trabajo por usted.

Establecer la memoria en 0 (o algo más) es solo en algunos casos una buena práctica ya que free () no borrará la memoria. Simplemente marcará una región de memoria para que esté libre y pueda reutilizarse. Si desea borrar la memoria, para que nadie pueda leerla, debe limpiarla manualmente. Pero esta es una operación bastante pesada y es por eso que no debería usarse para liberar toda la memoria. En la mayoría de los casos, liberar sin borrar es suficiente y no tiene que sacrificar el rendimiento para realizar operaciones innecesarias.

Answer 4

Lo que sugiere su colega hará que el código sea "más seguro" en caso de que la función se llame dos veces (vea el comentario de sleske ... ya que "más seguro" puede no significar lo mismo para todos ... ;-).

Con su código, lo más probable es que se bloquee:

Foo* foo = malloc( sizeof(Foo) );
DestroyFoo(foo);
DestroyFoo(foo); // will call free on memory already freed

Con la versión del código de sus colegas, esto no se bloqueará:

Foo* foo = malloc( sizeof(Foo) );
DestroyFoo(&foo);
DestroyFoo(&foo); // will have no effect

Ahora, para este escenario específico, haciendo tmpFoo = 0; (dentro de DestroyFoo ) es suficiente. memset(tmpFoo, 0, sizeof(Foo)); evitará el bloqueo si Foo tiene atributos adicionales a los que se podría acceder incorrectamente después de liberar la memoria.

Entonces diría que sí, puede ser una buena práctica hacerlo ... pero es solo una especie de seguridad contra los desarrolladores que tienen malas prácticas (porque definitivamente no hay razón para llamar a DestroyFoo dos veces sin reasignarlo) ... en Al final, hace que DestroyFoo "más seguro" pero más lento (hace más cosas para evitar el mal uso de él).

Answer 5

void destroyFoo(Foo** foo)
{
    if (!(*foo)) return;
    Foo *tmpFoo = *foo;
    *foo = NULL;
    memset(tmpFoo, 0, sizeof(Foo));
    free(tmpFoo);
}

Su código de colega es malo porque

• se bloqueará si foo es NULL
• no tiene sentido crear variables adicionales
• no tiene sentido establecer los valores en ceros
• liberar una estructura directamente no funciona si contiene cosas que deben ser liberadas

Creo que lo que tu colega podría tener en mente es este caso de uso

Foo* a = NULL;
Foo* b = createFoo();

destroyFoo(NULL);
destroyFoo(&a);
destroyFoo(&b);

En ese caso, debería ser así. Intenta aquí

void destroyFoo(Foo** foo)
{
    if (!foo || !(*foo)) return;
    free(*foo);
    *foo = NULL;
}

Primero tenemos que echar un vistazo a Foo , supongamos que se ve así

struct Foo
{
    // variables
    int number;
    char character;

    // array of float
    int arrSize;
    float* arr;

    // pointer to another instance
    Foo* myTwin;
};

Ahora, para definir cómo se debe destruir, primero definamos cómo se debe crear

Foo* createFoo (int arrSize, Foo* twin)
{
    Foo* t = (Foo*) malloc(sizeof(Foo));

    // initialize with default values
    t->number = 1;
    t->character = ''1'';

    // initialize the array
    t->arrSize = (arrSize>0?arrSize:10);
    t->arr = (float*) malloc(sizeof(float) * t->arrSize);

    // a Foo is a twin with only one other Foo
    t->myTwin = twin;
    if(twin) twin->myTwin = t;

    return t;
}

Ahora podemos escribir una función de destrucción opuesta a la función de creación

Foo* destroyFoo (Foo* foo)
{
    if (foo)
    {
        // we allocated the array, so we have to free it
        free(t->arr);

        // to avoid broken pointer, we need to nullify the twin pointer
        if(t->myTwin) t->myTwin->myTwin = NULL;
    }

    free(foo);

    return NULL;
}

Prueba prueba aquí

int main ()
{
    Foo* a = createFoo (2, NULL);
    Foo* b = createFoo (4, a);

    a = destroyFoo(a);
    b = destroyFoo(b);

    printf("success");
    return 0;
}