Ist dies eine gute Möglichkeit, Speicher in C freizugeben?

Question

Die Funktion eine Instanz von struct Foo unten gegeben wird für die Freigabe:

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

Ein Kollege von mir vorgeschlagen, die folgenden statt:

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

Ich sehe, dass der Zeiger auf NULL Einstellung nach Freigabe ist besser, aber ich bin mir nicht sicher über die folgenden:

1. Müssen wir wirklich den Zeiger auf a zuweisen temporäre? Hilft es in Bezug auf Nebenläufigkeit und geteilten Speicher?
2. Ist es wirklich eine gute Idee, den ganzen Block auf 0 zu setzen, um das Programm zum Absturz zu bringen oder zumindest Ergebnisse mit signifikanter Diskrepanz auszugeben?

Vielen Dank im Voraus!

Answer 1

Müssen wir den Zeiger wirklich einem temporären zuweisen? Ist es Hilfe in Bezug auf Nebenläufigkeit und gemeinsamen Speicher?

Es hat nichts zu tun Nebenläufigkeit oder Shared Memory. Es ist zwecklos.

Ist es wirklich eine gute Idee, den ganzen Block auf 0 zu zwingen, das Programm zum Absturz zu bringen oder zumindest Ausgabeergebnisse mit erheblicher Diskrepanz zu setzen?

Nein. Überhaupt nicht.

Die von Ihrem Kollegen vorgeschlagene Lösung ist schrecklich. Hier ist warum:

• Einstellung ganzen Block auf 0 erreicht auch nichts. Da jemand versehentlich einen freien() Ed-Block verwendet, würde er das aufgrund der Werte im Block nicht wissen. Das ist die Art von Block calloc() kehrt zurück. So ist es unmöglich zu wissen, ob es frisch zugeordneter Speicher ist (calloc() oder malloc()+memset()) oder der, der durch Ihren Code früher frei war(). Wenn es etwas ist, ist es zusätzliche Arbeit für Ihr Programm, jeden Speicherblock, der frei() ist, auf Null zu setzen.

• free(NULL); ist wohldefiniert und ist ein No-Op, so dass die if Zustand in if(ptr) {free(ptr);} nichts erreicht.

• Da free(NULL); no-op, würde versteckt den Zeiger auf NULL Einstellung tatsächlich diesen Fehler, denn wenn eine Funktion free() auf einem bereits frei() 'ed Zeiger Aufruf tatsächlich, dann würden sie das nicht wissen.

• meisten Benutzerfunktionen würde eine NULL-Check am Anfang haben und nicht berücksichtigen kann NULL, um es als Fehlerzustand vorbei:

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

    /*Do something with ptr here */ 
} 

So die alle diese zusätzlichen Kontrollen und zero'ing Es gibt ein falsches Gefühl von "Robustheit", während es nichts wirklich verbessert. Es ersetzt nur ein Problem durch ein anderes, die zusätzlichen Kosten für Leistung und Code-Blähung.

ist also free(ptr); ohne Wrapper-Funktion nur anrufen einfach und robust (die meisten malloc() Implementierungen abstürzen würde sofort auf Doppel frei, die eine gute Sache ist).

Es gibt keinen einfachen Weg um "versehentlich" free() zweimal oder mehr zu rufen. Es liegt in der Verantwortung des Programmierers, den gesamten zugewiesenen Speicher und free() entsprechend zu verfolgen. Wenn jemand das schwer zu handhaben findet, dann ist C wahrscheinlich nicht die richtige Sprache für sie.

Answer 2

Was Ihr Kollege vorschlägt, wird den Code "sicherer" machen, falls die Funktion zweimal aufgerufen wird (siehe sleske Kommentar ... da "sicherer" vielleicht nicht für alle gleich ist ... ;-).

mit Ihrem Code, dies wird höchstwahrscheinlich Absturz:

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

Mit Ihrem collegues-Version des Codes, dies wird nicht abstürze:

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

nun für dieses spezielle Szenario tun tmpFoo = 0; (innerhalb DestroyFoo) ist genug. memset(tmpFoo, 0, sizeof(Foo)); verhindert einen Absturz, wenn Foo über zusätzliche Attribute verfügt, auf die nach der Freigabe des Speichers fälschlicherweise zugegriffen werden kann.

So würde ich sagen, ja, kann es eine gute Praxis, dies zu tun .... aber es ist nur eine Art Sicherheit gegen Entwickler, die schlechten Praktiken haben (weil es definitiv kein Grund DestroyFoo zweimal anrufen, ohne es zu Neuzuweisung) ... am Ende machen Sie DestroyFoo "sicherer" aber langsamer (es macht mehr Sachen, um eine schlechte Nutzung davon zu verhindern).

Answer 3

Die zweite Lösung scheint überentwickelt zu sein. Natürlich könnte es in manchen Situationen sicherer sein, aber der Overhead und die Komplexität sind einfach zu groß.

Was Sie tun sollten, wenn Sie auf der sicheren Seite sein möchten, ist das Setzen des Zeigers auf NULL nach dem Freigeben von Speicher. Dies ist immer eine gute Übung.

Was ist mehr, ich weiß nicht, warum Menschen überprüfen, ob der Zeiger vor dem Aufruf von free() NULL ist. Dies wird nicht benötigt, da free() die Arbeit für Sie erledigt.

Einstellung von Speicher auf 0 (oder etwas anderes) ist nur in einigen Fällen eine gute Übung, da free() den Speicher nicht löscht. Es markiert nur eine Speicherregion, um frei zu sein, damit sie wiederverwendet werden kann. Wenn Sie den Speicher löschen möchten, damit niemand ihn lesen kann, müssen Sie ihn manuell löschen. Aber das ist eine ziemlich schwere Operation und deshalb sollte dies nicht verwendet werden, um den gesamten Speicher freizugeben. In den meisten Fällen reicht es nur aus, ohne zu löschen, und Sie müssen nicht auf Leistung verzichten, um unnötige Operationen auszuführen.

Answer 4

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

Ihr Kollege Code ist schlecht, weil

• es stürzt ab, wenn fooNULL ist
• es keinen Sinn, zusätzliche Variable bei der Schaffung von
• es bei der Festlegung der Werte auf Null keinen Sinn
• das Freigeben einer Struktur direkt funktioniert nicht, wenn sie Dinge enthält, die freizugeben sind

Ich denke, was Ihr Kollege könnte im Sinne hat, ist dies Use-Case

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

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

In diesem Fall sollte es so sein. try here

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

---

Zuerst müssen wir einen Blick auf Foo nehmen, nehmen wir an, dass es wie folgt nun, wie es zerstört werden sollte, definieren

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

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

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

sieht, lassen Sie uns definieren zunächst, wie es sollte erstellt werden

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 t schreiben er Funktion erstellen

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 try here

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

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

    printf("success"); 
    return 0; 
}