Ist dies ein guter Weg, um Speicher freizugeben?

Was Ihr Kollege vorschlägt, macht den Code "sicherer", falls die Funktion zweimal aufgerufen wird (siehe Kommentar dazu ... als "sicherer" bedeutet dies möglicherweise nicht für alle gleich ... ;-).

Mit Ihrem Code wird dies höchstwahrscheinlich abstürzen:

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

Mit der Version des Codes Ihrer Kollegen stürzt dies nicht ab:

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

Für dieses spezielle Szenario reicht es aus, tmpFoo = 0; (innerhalb von DestroyFoo) auszuführen. memset(tmpFoo, 0, sizeof(Foo)); verhindert den Absturz, wenn Foo zusätzliche Attribute hat, auf die möglicherweise nach der Freigabe des Speichers falsch zugegriffen wird.

Also würde ich ja sagen, es ist eine gute Praxis, dies zu tun ... aber es ist nur eine Art Sicherheit gegen Entwickler, die schlechte Praktiken haben (da es definitiv keinen Grund gibt, DestroyFoo zweimal aufzurufen, ohne sie neu zuzuordnen... am Ende machen Sie DestroyFoo "sicherer", aber langsamer (es wird mehr getan, um eine schlechte Verwendung zu verhindern). 

Die zweite Lösung scheint übertrieben zu sein. In manchen Situationen ist es natürlich sicherer, aber der Aufwand und die Komplexität sind einfach zu groß.

Wenn Sie auf der sicheren Seite sein möchten, sollten Sie den Zeiger auf NULL setzen, nachdem Sie Speicher freigegeben haben. Dies ist immer eine gute Praxis. 

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

Außerdem weiß ich nicht, warum die Leute vor dem Aufruf von free () überprüfen, ob der Zeiger NULL ist. Dies ist nicht erforderlich, da free () die Arbeit für Sie erledigen wird.

Das Festlegen des Speichers auf 0 (oder etwas anderes) ist nur in einigen Fällen eine bewährte Methode, da free () den Speicher nicht löscht. Es wird nur einen Speicherbereich als frei markiert, damit er wiederverwendet werden kann. Wenn Sie den Speicher löschen möchten, damit niemand ihn lesen kann, müssen Sie ihn manuell bereinigen. Dies ist jedoch eine recht schwere Operation, und deshalb sollte nicht der gesamte Speicher freigegeben werden. In den meisten Fällen genügt das Freigeben ohne Löschen nur, und Sie müssen nicht auf die Leistung verzichten, um unnötige Operationen auszuführen.

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

Ihr Kollegencode ist schlecht, weil

• es wird abstürzen, wenn fooNULL ist
• es ist sinnlos, zusätzliche Variablen zu erstellen
• es macht keinen Sinn, die Werte auf Nullen zu setzen
• das direkte Freigeben einer Struktur funktioniert nicht, wenn sie Elemente enthält, die freigegeben werden müssen

Ich denke, was Ihr Kollege im Sinn haben könnte, ist dieser Anwendungsfall

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

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

In diesem Fall sollte es so sein. Versuch es hier

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

Zuerst müssen wir uns Foo ansehen, nehmen wir an, dass es so aussieht

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

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

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

Um zu definieren, wie es zerstört werden soll, definieren wir zunächst, wie es erstellt werden soll

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

Jetzt können wir eine Zerstörungsfunktion gegen die Erstellungsfunktion schreiben

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

Test hier versuchen

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

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

    printf("success");
    return 0;
}

Leider funktioniert diese Idee einfach nicht.

Wenn die Absicht bestand, doppelt gefangen zu werden, deckt dies nicht die folgenden Fälle ab.

Nehmen Sie diesen Code an:

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

Der Vorschlag lautet stattdessen zu schreiben:

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

Das Problem ist, dass der zweite Aufruf von DestroyFoo() immer noch abstürzt, da ptr_2 nicht auf NULL zurückgesetzt wird und immer noch auf den bereits freigegebenen Speicher verweist.