c - Moet men echt pointers op `NULL` zetten nadat ze zijn vrijgemaakt?

Er lijken twee argumenten te zijn waarom je een aanwijzer op NULLmoet zetten nadat je ze hebt vrijgemaakt.

Voorkom crashen wanneer dubbel-bevrijdende wijzers.

Kort: free()een tweede keer aanroepen, per ongeluk, crasht niet wanneer het is ingesteld op NULL.

Bijna altijd maskeert dit een logische bug omdat er geen reden is om free()een tweede keer aan te roepen. Het is veiliger om de applicatie te laten crashen en deze te kunnen repareren.
Het is niet gegarandeerd dat het crasht, omdat er soms nieuw geheugen op hetzelfde adres wordt toegewezen.
Dubbel vrij komt meestal voor wanneer er twee aanwijzers naar hetzelfde adres wijzen.

Logische fouten kunnen ook leiden tot gegevenscorruptie.

Vermijd hergebruik van vrijgekomen pointers

Kort: toegang tot vrijgemaakte aanwijzers kan gegevensbeschadiging veroorzaken als malloc()geheugen op dezelfde plek toewijst, tenzij de vrijgemaakte aanwijzer is ingesteld op NULL

Er is geen garantie dat het programma crasht bij het openen van de NULL-aanwijzer, als de offset groot genoeg is (someStruct->lastMember, theArray[someBigNumber]). In plaats van te crashen zal er datacorruptie zijn.
De aanwijzer instellen op NULLkan het probleem van het hebben van een andere aanwijzer met dezelfde aanwijzerwaarde niet oplossen.

De vragen

Hier is een bericht tegen het blindelings instellen van een aanwijzer op NULLna het vrijgeven.

Welke is moeilijker te debuggen?
Is er een mogelijkheid om beide te vangen?
Hoe waarschijnlijk is het dat dergelijke bugs leiden tot datacorruptie in plaats van crashen?

Voel deze vraag gerust uit.

Antwoord 1, autoriteit 100%

De tweede is veel belangrijker: het hergebruiken van een vrijgekomen aanwijzer kan een subtiele fout zijn. Je code blijft gewoon werken en crasht vervolgens zonder duidelijke reden omdat een schijnbaar niet-gerelateerde code in het geheugen heeft geschreven waar de hergebruikte aanwijzer toevallig naar verwijst.

Ik heb ooit moeten werken aan een echtprogramma met fouten dat iemand anders schreef. Mijn instinct vertelde me dat veel van de bugs te maken hadden met slordige pogingen om pointers te blijven gebruiken nadat het geheugen was vrijgemaakt; Ik heb de code aangepast om de aanwijzers op NULL te zetten nadat ik het geheugen had vrijgemaakt, en bam, de uitzonderingen op de null-aanwijzer begonnen te komen. Nadat ik alle null pointer-uitzonderingen had opgelost, was de code plotseling veelstabieler.

In mijn eigen code roep ik alleen mijn eigen functie aan die een wrapper is rond free(). Er is een pointer-naar-a-pointer nodig en de aanwijzer wordt nulls gegeven nadat het geheugen is vrijgemaakt. En voordat het gratis aanroept, roept het Assert(p != NULL);aan, dus het vangt nog steeds pogingen om dezelfde aanwijzer dubbel te bevrijden.

Mijn code doet ook andere dingen, zoals (alleen in de DEBUG-build) het geheugen vullen met een voor de hand liggende waarde onmiddellijk na toewijzing, hetzelfde doen voordat free()wordt aangeroepen voor het geval er een kopie van de aanwijzer, enz. Details hier.

EDIT: per verzoek is hier een voorbeeldcode.

void
FreeAnything(void **pp)
{
    void *p;
    AssertWithMessage(pp != NULL, "need pointer-to-pointer, got null value");
    if (!pp)
        return;
    p = *pp;
    AssertWithMessage(p != NULL, "attempt to free a null pointer");
    if (!p)
        return;
    free(p);
    *pp = NULL;
}
// FOO is a typedef for a struct type
void
FreeInstanceOfFoo(FOO **pp)
{
    FOO *p;
    AssertWithMessage(pp != NULL, "need pointer-to-pointer, got null value");
    if (!pp)
        return;
    p = *pp;
    AssertWithMessage(p != NULL, "attempt to free a null FOO pointer");
    if (!p)
        return;
    AssertWithMessage(p->signature == FOO_SIG, "bad signature... is this really a FOO instance?");
    // free resources held by FOO instance
    if (p->storage_buffer)
        FreeAnything(&p->storage_buffer);
    if (p->other_resource)
        FreeAnything(&p->other_resource);
    // free FOO instance itself
    free(p);
    *pp = NULL;
}

Opmerkingen:

Je kunt in de tweede functie zien dat ik de twee resource-pointers moet controleren om te zien of ze niet null zijn, en dan FreeAnything()aanroepen. Dit komt door de assert()die zal klagen over een null-pointer. Ik heb die bewering om een poging om dubbel te bevrijden te detecteren, maar ik denk niet dat het echt veel bugs voor mij heeft opgevangen; als je de beweringen wilt weglaten, dan kun je de check weglaten en gewoon altijd FreeAnything()aanroepen. Afgezien van de bewering, gebeurt er niets ergs wanneer je probeert een null-pointer vrij te maken met FreeAnything()omdat het de pointer controleert en alleen teruggeeft als deze al null was.

Mijn eigenlijke functienamen zijn wat beknopter, maar ik heb geprobeerd zelfdocumenterende namen te kiezen voor dit voorbeeld. Ook heb ik in mijn eigenlijke code debug-only code die buffers vult met de waarde 0xDCvoordat ik free()aanroep, zodat als ik een extra verwijzing naar diezelfde geheugen (een die niet wordt uitgeschakeld) wordt het echt duidelijk dat de gegevens waarnaar het verwijst nepgegevens zijn. Ik heb een macro, DEBUG_ONLY(), die compileert naar niets op een niet-debug build; en een macro FILL()die een sizeof()doet op een struct. Deze twee werken even goed: sizeof(FOO)of sizeof(*pfoo). Dus hier is de macro FILL():

#define FILL(p, b) \
    (memset((p), b, sizeof(*(p)))

Hier is een voorbeeld van het gebruik van FILL()om de 0xDC-waarden in te voeren voordat wordt aangeroepen:

if (p->storage_buffer)
{
    DEBUG_ONLY(FILL(pfoo->storage_buffer, 0xDC);)
    FreeAnything(&p->storage_buffer);
}

Een voorbeeld van het gebruik hiervan:

PFOO pfoo = ConstructNewInstanceOfFoo(arg0, arg1, arg2);
DoSomethingWithFooInstance(pfoo);
FreeInstanceOfFoo(&pfoo);
assert(pfoo == NULL); // FreeInstanceOfFoo() nulled the pointer so this never fires

Antwoord 2, autoriteit 33%

Ik doe dit niet. Ik herinner me niet echt bugs die gemakkelijker te verhelpen zouden zijn als ik dat wel had gedaan. Maar het hangt er echt van af hoe je je code schrijft. Er zijn ongeveer drie situaties waarin ik iets vrijgeef:

Als de aanwijzer die hem vasthoudt op het punt staat buiten het bereik te gaan, of deel uitmaakt van een object dat op het punt staat buiten het bereik te gaan of te worden bevrijd.
Als ik het object vervang door een nieuw (zoals bij hertoewijzing bijvoorbeeld).
Als ik een object vrijgeef dat optioneel aanwezig is.

In het derde geval stelt u de aanwijzer in op NULL. Dat is niet specifiek omdat je het vrijmaakt, het is omdat wat-het-is optioneel is, dus natuurlijk is NULL een speciale waarde die betekent “Ik heb er geen”.

In de eerste twee gevallen lijkt het mij een druk werk zonder specifiek doel om de aanwijzer op NULL te zetten:

int doSomework() {
    char *working_space = malloc(400*1000);
    // lots of work
    free(working_space);
    working_space = NULL; // wtf? In case someone has a reference to my stack?
    return result;
}
int doSomework2() {
    char * const working_space = malloc(400*1000);
    // lots of work
    free(working_space);
    working_space = NULL; // doesn't even compile, bad luck
    return result;
}
void freeTree(node_type *node) {
    for (int i = 0; i < node->numchildren; ++i) {
        freeTree(node->children[i]);
        node->children[i] = NULL; // stop wasting my time with this rubbish
    }
    free(node->children);
    node->children = NULL; // who even still has a pointer to node?
    // Should we do node->numchildren = 0 too, to keep
    // our non-existent struct in a consistent state?
    // After all, numchildren could be big enough
    // to make NULL[numchildren-1] dereferencable,
    // in which case we won't get our vital crash.
    // But if we do set numchildren = 0, then we won't
    // catch people iterating over our children after we're freed,
    // because they won't ever dereference children.
    // Apparently we're doomed. Maybe we should just not use
    // objects after they're freed? Seems extreme!
    free(node);
}
int replace(type **thing, size_t size) {
    type *newthing = copyAndExpand(*thing, size);
    if (newthing == NULL) return -1;
    free(*thing);
    *thing = NULL; // seriously? Always NULL after freeing?
    *thing = newthing;
    return 0;
}

Het is waar dat het NULL-gebruiken van de aanwijzer het duidelijker kan maken als je een bug hebt waarbij je de verwijzing probeert te verwijderen na het vrijgeven. Dereferentie is waarschijnlijk niet direct schadelijk als u de aanwijzer niet NULL-tekent, maar is op de lange termijn verkeerd.

Het is ook waar dat NULL-ing van de aanwijzer verduistertbugs waar je dubbel-vrij maakt. De tweede gratis doet geen onmiddellijke schade als je de aanwijzer NULL doet, maar is op de lange termijn verkeerd (omdat het verraadt dat de levenscyclus van je object is verbroken). Je kunt beweren dat dingen niet-null zijn als je ze vrijgeeft, maar dat resulteert in de volgende code om een struct vrij te maken die een optionele waarde heeft:

if (thing->cached != NULL) {
    assert(thing->cached != NULL);
    free(thing->cached);
    thing->cached = NULL;
}
free(thing);

Wat die code je vertelt, is dat je te ver bent gekomen. Het zou moeten zijn:

free(thing->cached);
free(thing);

Ik zeg, NULL de aanwijzer als het verondersteldbruikbaar blijft. Als het niet meer bruikbaar is, kun je het het beste niet onterecht laten lijken door een potentieel zinvolle waarde in te voeren, zoals NULL. Als je een paginafout wilt veroorzaken, gebruik dan een platformafhankelijke waarde die niet kan worden verwijderd, maar die door de rest van je code niet wordt behandeld als een speciale “alles is goed en dandy” -waarde:

free(thing->cached);
thing->cached = (void*)(0xFEFEFEFE);

Als u zo’n constante niet op uw systeem kunt vinden, kunt u wellicht een niet-leesbare en/of niet-schrijfbare pagina toewijzen en het adres daarvan gebruiken.

Antwoord 3, autoriteit 15%

Als u de aanwijzer niet op NULL zet, is de kans niet zo klein dat uw toepassing in een niet-gedefinieerde staat blijft draaien en later crasht op een volledig niet-gerelateerd punt. Dan ben je veel tijd kwijt met het debuggen van een niet-bestaande fout voordat je erachter komt dat het een geheugencorruptie is van eerder.

Ik zou de aanwijzer op NULL zetten omdat de kans groter is dat je de juiste plek van de fout eerder bereikt dan wanneer je deze niet op NULL zou hebben gezet. De logische fout van het voor de tweede keer vrijmaken van geheugen moet nog worden overwogen en de fout dat uw toepassing NIET crasht bij null-pointertoegang met een voldoende grote offset is naar mijn mening volledig academisch, hoewel niet onmogelijk.

Conclusie: ik zou de aanwijzer op NULL zetten.

Antwoord 4, autoriteit 11%

Het antwoord hangt af van (1) projectgrootte, (2) verwachte levensduur van uw code, (3) teamgrootte.
Bij een klein project met een korte levensduur kun je de instellingsaanwijzers naar NULL overslaan en gewoon debuggen.

Bij een groot, langlevend project zijn er goede redenen om pointers op NULL in te stellen:
(1) Defensief programmeren is altijd goed. Je code is misschien in orde, maar de beginner naast de deur kan nog steeds moeite hebben met aanwijzingen
(2) Mijn persoonlijke overtuiging is dat alle variabelen te allen tijde alleen geldige waarden moeten bevatten. Na een delete / free is de aanwijzer geen geldige waarde meer, dus moet deze van die variabele worden verwijderd. Het vervangen door NULL (de enige pointerwaarde die altijd geldig is) is een goede stap.
(3) Code sterft nooit. Het wordt altijd hergebruikt, en vaak op manieren die je je niet had kunnen voorstellen toen je het schreef. Uw codesegment kan uiteindelijk worden gecompileerd in een C++-context en waarschijnlijk worden verplaatst naar een destructor of een methode die wordt aangeroepen door een destructor. De interacties van virtuele methoden en objecten die worden vernietigd, zijn subtiele valstrikken, zelfs voor zeer ervaren programmeurs.
(4) Als uw code uiteindelijk wordt gebruikt in een context met meerdere threads, kan een andere thread die variabele lezen en proberen deze te openen. Dergelijke contexten ontstaan vaak wanneer oude code wordt verpakt en hergebruikt in een webserver. Dus een nog betere manier om geheugen vrij te maken (vanuit paranoïde oogpunt) is door (1) de aanwijzer naar een lokale variabele te kopiëren, (2) de oorspronkelijke variabele in te stellen op NULL, (3) de lokale variabele te verwijderen/vrij te maken.

Antwoord 5, autoriteit 7%

Als de aanwijzer opnieuw wordt gebruikt, moet deze na gebruik worden teruggezet naar 0 (NULL), zelfs als het object waarnaar hij wees niet van de heap is bevrijd. Dit zorgt voor een geldige controle op NULL zoals if (p){ //do something}. Ook alleen omdat u een object vrijmaakt waarvan het adres waarnaar de aanwijzer verwijst, betekent niet dat de aanwijzer op 0 staat na het aanroepen van het delete-sleutelwoord of de gratis functie.

Als de aanwijzer eenmaal wordt gebruikt en deze deel uitmaakt van een bereik waardoor deze lokaal is, hoeft deze niet op NULL te worden ingesteld, omdat deze van de stapel wordt verwijderd nadat de functie is geretourneerd.

Als de aanwijzer een lid is (struct of klasse), moet u deze op NULL zetten nadat u het object of de objecten opnieuw op een dubbele aanwijzer hebt vrijgemaakt voor geldige controle tegen NULL.

Als u dit doet, kunt u de hoofdpijn van ongeldige verwijzingen zoals ‘0xcdcd…’ enzovoort verlichten. Dus als de aanwijzer 0 is, weet je dat hij niet naar een adres wijst en kun je ervoor zorgen dat het object van de heap wordt gehaald.

Antwoord 6, autoriteit 4%

Beide zijn erg belangrijk omdat ze te maken hebben met ongedefinieerd gedrag. U moet geen manieren overlaten aan ongedefinieerd gedrag in uw programma. Beide kunnen leiden tot crashes, corrupte gegevens, subtiele bugs en andere slechte gevolgen.

Beide zijn vrij moeilijk te debuggen. Beide zijn zeker niet te vermijden, zeker niet in het geval van complexe datastructuren. Hoe dan ook, je bent veel beter af als je de volgende regels volgt:

aanwijzers altijd initialiseren – stel ze in op NULL of een geldig adres
nadat je free() hebt aangeroepen, zet je de aanwijzer op NULL
controleer alle verwijzingen die mogelijk NULL kunnen zijn als NULL voordat ze worden verwijderd.

Antwoord 7, autoriteit 4%

In C++ kan zowel worden opgevangen door uw eigen slimme aanwijzer te implementeren (of af te leiden uit bestaande implementaties) als door iets te implementeren als:

void release() {
    assert(m_pt!=NULL);
    T* pt = m_pt;
    m_pt = NULL;
    free(pt);
}
T* operator->() {
    assert(m_pt!=NULL);
    return m_pt;
}

Als alternatief kunt u in C ten minste twee macro’s met hetzelfde effect opgeven:

#define SAFE_FREE(pt) \
    assert(pt!=NULL); \
    free(pt); \
    pt = NULL;
#define SAFE_PTR(pt) assert(pt!=NULL); pt

Antwoord 8, autoriteit 4%

Deze problemen zijn meestal slechts symptomen voor een veel dieper probleem. Dit kan gebeuren voor alle resources die moeten worden aangekocht en later worden vrijgegeven, b.v. geheugen, bestanden, databases, netwerkverbindingen, enz. Het kernprobleem is dat u de brontoewijzingen uit het oog bent verloren door een ontbrekende codestructuur, willekeurige mallocs gooit en de hele codebasis vrijmaakt.

Organiseer de code rond DRY – Don’t Repeat Yourself. Houd gerelateerde dingen bij elkaar. Doe maar één ding en doe het goed. De “module” die een resource toewijst, is verantwoordelijk voor het vrijgeven ervan en moet hiervoor een functie bieden die ook voor de pointers zorgt. Voor elk specifiek middel heb je dan precies één plaats waar het wordt toegewezen en één plaats waar het wordt vrijgegeven, beide dicht bij elkaar.

Stel dat je een string in substrings wilt splitsen. Rechtstreeks met malloc(), moet je functie voor alles zorgen: de string analyseren, de juiste hoeveelheid geheugen toewijzen, de substrings daarheen kopiëren, en en en. Maak de functie ingewikkeld genoeg, en het is niet de vraag of je de middelen uit het oog verliest, maar wanneer.

Uw eerste module zorgt voor de daadwerkelijke geheugentoewijzing:


    void *MemoryAlloc (size_t size)
    void  MemoryFree (void *ptr)

Er is je enige plek in je hele codebase waar malloc() en free() worden genoemd.

Vervolgens moeten we strings toewijzen:


    StringAlloc (char **str, size_t len)
    StringFree (char **str)

Ze zorgen ervoor dat len+1 nodig is en dat de aanwijzer op NULL wordt gezet wanneer deze wordt vrijgegeven. Geef een andere functie op om een substring te kopiëren:


    StringCopyPart (char **dst, const char *src, size_t index, size_t len)

Het zal ervoor zorgen dat index en len zich in de src-tekenreeks bevinden en deze indien nodig aanpassen. Het zal StringAlloc voor dst aanroepen, en het zal ervoor zorgen dat dst correct wordt beëindigd.

Nu kunt u uw splitsingsfunctie schrijven. Je hoeft niet meer om de details op laag niveau te geven, analyseer gewoon de string en haal de substrings eruit. De meeste logica zit nu in de module waar het thuishoort, in plaats van vermengd in één groot gedrocht.

Natuurlijk heeft deze oplossing zijn eigen problemen. Het biedt abstractielagen, en elke laag lost andere problemen op, maar heeft zijn eigen set ervan.

Antwoord 9

Er is niet echt een “belangrijker” deel voor welk van de twee problemen je probeert te vermijden. Je moet beide echt vermijden als je betrouwbare software wilt schrijven. Het is ook zeer waarschijnlijk dat een van de bovenstaande zaken zal leiden tot datacorruptie, het pwn van je webserver en andere leuke dingen in die zin.

Er is ook nog een belangrijke stap om in gedachten te houden: de aanwijzer op NULL zetten na het vrijgeven is maar het halve werk. Idealiter, als je dit idioom gebruikt, zou je de aanwijzertoegang ook in iets als dit moeten stoppen:

if (ptr)
  memcpy(ptr->stuff, foo, 3);

Alleen al door de aanwijzer zelf op NULL te zetten, zal het programma alleen op ongelegen plaatsen crashen, wat waarschijnlijk beter is dan het stilletjes beschadigen van gegevens, maar het is nog steeds niet wat je wilt.

Antwoord 10

Er is geen garantie dat het programma crasht bij het openen van de NULL-aanwijzer.

Misschien niet volgens de standaard, maar het zou moeilijk zijn om een implementatie te vinden die het niet definieert als een illegale bewerking die een crash of uitzondering veroorzaakt (afhankelijk van de runtime-omgeving).

Moet men echt pointers op `NULL` zetten nadat ze zijn vrijgemaakt?