¿Es esta una buena manera de liberar memoria en C?

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

Con su código, lo más probable es que esto 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, hacer tmpFoo = 0; (dentro de DestroyFoo) es suficiente. memset(tmpFoo, 0, sizeof(Foo)); evitará el bloqueo si Foo tiene atributos adicionales a los que se puede acceder erróneamente después de la memoria es liberado.

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

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 es demasiado grande.

Lo que debe hacer si desea estar en un lado seguro es establecer el puntero a NULL después de liberar memoria. Siempre es una buena práctica.

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

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

Configuración de la memoria a 0 (o algo más) es solo en algunos casos una buena práctica ya que free () no borrará la memoria. Solo marcará una región de la memoria para ser libre para que pueda ser reutilizada. 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 se debe usar para liberar toda la memoria. En la mayoría de los casos, liberar sin limpiar es suficiente y no tiene que sacrificar el rendimiento para hacer algo innecesario operación.

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 una variable adicional
• no tiene sentido establecer los valores en ceros
• liberar una estructura directamente no funciona si contiene cosas que deben ser liberadas{[14]]}

Creo que lo que su 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í. pruebe 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 debe ser destruido, primero definamos cómo debe ser creado

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 destroy oppose a la función create

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

Desafortunadamente, esta idea no está funcionando.

Si la intención era atrapar double free, no está cubriendo casos como los siguientes.

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 accidente, porque ptr_2 no se restablece a NULL, y todavía la memoria ya liberado.