Faut-il vraiment mettre les pointeurs à "NULL" après les avoir libérés?

le second est bien plus important: réutiliser un pointeur libéré peut être une erreur subtile. Votre code continue de fonctionner, et puis s'écrase sans raison claire parce qu'un code apparemment sans rapport écrit dans la mémoire que le pointeur réutilisé se trouve pointé.

une fois, j'ai dû travailler sur un vraiment programme buggy quelqu'un d'autre a écrit. Mes instincts m'ont dit que beaucoup des bogues étaient liés à des tentatives négligées de continuer à utiliser pointeurs après avoir libéré la mémoire; j'ai modifié le code pour définir les pointeurs à NULL après avoir libéré la mémoire, et bam , les exceptions à pointeur nul ont commencé à venir. Après avoir corrigé toutes les exceptions du pointeur nul, soudainement le code était beaucoup plus stable.

dans mon propre code, je n'appelle que ma propre fonction qui est un wraper around free(). Il prend un pointeur-à-un-pointeur, et annule le pointeur après avoir libéré la mémoire. Et avant qu'il appelle libre, il appelle Assert(p != NULL); donc il attrape toujours des tentatives de double-libre le même pointeur.

mon code fait aussi d'autres choses, comme (dans la construction de débogage seulement) remplir la mémoire avec une valeur évidente immédiatement après l'avoir affectée, faire la même chose avant d'appeler free() au cas où il y aurait une copie du pointeur, etc. détails ici.

Modifier: pour une requête, voici un exemple de code.

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

commentaires:

vous pouvez voir dans la deuxième fonction que je dois vérifier les deux pointeurs de ressources pour voir si elles ne sont pas nulles, puis appeler FreeAnything() . C'est à cause du assert() qui se plaindra d'un pointeur nul. Je n'ai que faire valoir afin de détecter une tentative de double-libre, mais je ne pense pas qu'il a réellement pris beaucoup de bugs pour moi; si vous voulez quitter l'affirme, alors vous pouvez laisser la case juste et toujours appelez FreeAnything() . A part l'assert, rien de mal ne se produit quand vous essayez de libérer un pointeur null avec FreeAnything() parce qu'il vérifie le pointeur et retourne juste s'il était déjà null.

mes noms de fonction réels sont un peu plus ternes, mais j'ai essayé de choisir des noms auto-documentant pour cet exemple. De plus, dans mon code actuel, j'ai seulement du code de débogage qui remplit les tampons avec la valeur 0xDC avant d'appeler free() de sorte que si j'ai un pointeur supplémentaire à ce même mémoire (une qui ne s'annule pas) il devient vraiment évident que les données qu'il pointe sont des données fausses. J'ai une macro, DEBUG_ONLY() , qui compile à rien sur un build non-debug; et une macro FILL() qui fait un sizeof() sur une structure. Ces deux-là fonctionnent également bien: sizeof(FOO) ou sizeof(*pfoo) . Donc voici le FILL() macro:

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

voici un exemple d'utilisation de FILL() pour mettre les valeurs 0xDC avant appelant:

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

exemple d'utilisation:

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

Je ne fais pas ça. Je ne me souviens pas particulièrement d'insectes qui auraient été plus faciles à traiter si je l'avais fait. Mais cela dépend vraiment de la façon dont vous écrivez votre code. Il y a environ trois situations où je libère n'importe quoi:

• lorsque le pointeur qui le tient est sur le point de sortir de sa portée, ou fait partie d'un objet sur le point de sortir de sa portée ou d'être libéré.
• lorsque je remplace l'objet par un nouvel objet (comme dans la réallocation, pour exemple).
• quand je libère un objet qui est éventuellement présent.

dans le troisième cas, vous définissez le pointeur à NULL. Ce n'est pas spécifiquement parce que vous le Libérez, c'est parce que le whatever-it-is est optionnel, donc bien sûr NULL est une valeur spéciale qui signifie "Je n'en ai pas".

dans les deux premiers cas, mettre le pointeur à nul me semble être un travail occupé sans but particulier:

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

il est vrai que NULL-ing le pointeur peut le rendre plus évident si vous avez un bug où vous essayez de le déréférencer après la libération. Dereferencing ne fait probablement pas de mal immédiat si vous N'annulez pas le pointeur, mais est erroné à long terme.

il est également vrai que nul-ing le pointeur obscurcit bugs où vous double-free. Le second free ne fait pas de mal immédiat si vous faites NULL le pointeur, mais est erroné à long terme (parce qu'il trahit le fait que vos cycles de vie objet sont brisés). Vous pouvez affirmer que les choses ne sont pas nulles quand vous les Libérez, mais cela se traduit par le code suivant pour libérer une structure qui contient une valeur optionnelle:

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

ce que ce code vous dit, c'est que vous êtes allé trop loin. Il devrait être:

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

je dis, NULL le pointeur si c'est supposé pour rester utilisable. Si elle n'est plus utilisable, mieux vaut ne pas faites-le apparaître faussement, en mettant dans une valeur potentiellement significative comme NULL. Si vous voulez provoquer un défaut de page, utilisez une valeur dépendante de la plate-forme qui n'est pas déréférenciable, mais que le reste de votre code ne traitera pas comme une valeur spéciale "tout va bien et dandy":

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

si vous ne pouvez pas trouver une telle constante sur votre système, vous pouvez être en mesure d'attribuer une page Non lisible et/ou non inscriptible, et d'utiliser l'adresse de celle-ci.

si vous ne définissez pas le pointeur à NULL, il y a de fortes chances que votre application continue à fonctionner dans un état non défini et se bloque plus tard à un point complètement différent. Ensuite, vous passerez beaucoup de temps avec le débogage d'une erreur inexistante avant de découvrir, que c'est une corruption de la mémoire de plus tôt.

j'avais placé le pointeur à NULL parce que les chances sont plus élevées que vous atteindrez le bon point de l'erreur plus tôt que si vous ne l'aviez pas placé to NULL. L'erreur logique de libérer la mémoire une deuxième fois est encore à penser et l'erreur que votre application ne se bloque pas sur l'accès null-pointer avec un décalage assez grand est à mon avis tout à fait académique, bien que pas impossible.

Conclusion: j'opterais pour le réglage du pointeur à NULL.

si le pointeur doit être réutilisé, il doit être ramené à 0(NULL) après utilisation, même si l'objet pointé n'est pas libéré du tas. Cela permet de vérifier la validité de NULL comme si (p){ //faire quelque chose}. Aussi juste parce que vous libérez un objet dont l'adresse pointe vers le pointeur ne signifie pas que le pointeur est mis 0 après avoir appelé le mot-clé de suppression ou la fonction libre du tout.

si le pointeur est utilisé une seule fois et fait partie d'une portée qui fait il local, alors il n'est pas nécessaire de le définir à NULL étant qu'il sera éliminé de la pile après le retour de la fonction.

si le pointeur est un membre (struct ou class), vous devez le définir à NULL après avoir libéré l'objet ou les objets sur un double pointeur à nouveau pour une vérification valide contre NULL.

faire cela vous aidera à soulager les maux de tête de pointeurs invalides comme '0xcdcd..."et ainsi de suite. Donc, si le pointeur est à 0 alors vous savez qu'il n'est pas pointant vers une adresse et assurez-vous que l'objet est libéré dans le tas.

la réponse dépend (1) de la taille du projet, (2) de la durée de vie prévue de votre code, (3) de la taille de l'équipe. Sur un petit projet avec une durée de vie courte, vous pouvez sauter le réglage des pointeurs à NULL, et juste déboguer le long.

sur un grand projet de longue durée, il y a de bonnes raisons de mettre des pointeurs à NULL: (1) la programmation défensive est toujours bonne. Votre code est peut-être correct, mais le débutant d'à côté peut encore se battre avec des pointeurs. (2) ma croyance personnelle est que toute variable devrait ne contiennent que des valeurs valides à tout moment. Après un delete / free, le pointeur n'est plus une valeur valide, il doit donc être supprimé de cette variable. Le remplacer par NULL (la seule valeur de pointeur qui est toujours valide) est une bonne étape. (3) le Code ne meurt jamais. Il est toujours réutilisé, et souvent d'une manière que vous n'avez pas imaginé au moment où vous l'avez écrit. Votre segment de code peut être compilé en C++ contexte, et probablement obtenir déplacé à un destructeur ou une méthode qui est appelée par un destructeur. Les interactions des méthodes virtuelles et des objets qui sont en train d'être détruits sont des pièges subtils, même pour des programmeurs très expérimentés. (4) Si votre code finit par être utilisé dans un contexte multi-threadé, un autre thread pourrait lire cette variable et essayer d'y accéder. De tels contextes se produisent souvent lorsque le code d'héritage est enveloppé et réutilisé dans un serveur web. Donc une façon encore meilleure de libérer la mémoire (d'un point de vue paranoïaque) est de (1) copier le pointeur sur une variable locale, (2) Mettre l'original variable à NULL, (3) Supprimer/libérer la variable locale.

tous deux sont très importants puisqu'ils traitent de comportements non définis. Vous ne devez laisser aucune façon de comportement non défini dans votre programme. Les deux peuvent conduire à des accidents, corrompre des données, des bugs subtils, d'autres mauvaises conséquences.

les deux sont assez difficiles à déboguer. Les deux ne peuvent pas être évités à coup sûr, en particulier dans le cas de structures de données complexes. De toute façon, vous êtes beaucoup mieux si vous suivez les règles suivantes:

• toujours initialiser pointeurs-les définir à NULL ou une adresse valide
• après avoir appelé free (), définissez le pointeur à NULL
• cochez tous les pointeurs qui peuvent éventuellement être nuls pour être effectivement NULS avant de les déréférencer.

en C++ pourrait attraper à la fois en implémentant votre propre pointeur intelligent (ou dérivant des implémentations existantes) et en implémentant quelque chose comme:

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

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

alternativement, en C vous pouvez au moins fournir deux macros au même effet:

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

#define SAFE_PTR(pt) assert(pt!=NULL); pt

ces problèmes ne sont le plus souvent que des symptômes pour un problème beaucoup plus profond. Cela peut se produire pour toutes les ressources qui nécessitent une acquisition et une publication ultérieure, par exemple la mémoire, les fichiers, les bases de données, les connexions réseau, etc. Le problème de base est que vous avez perdu la trace des allocations de ressources par une structure de code manquante, jetant des mallocs aléatoires et des frees partout dans la base de code.

organisez le code à sec - Ne vous répétez pas. Garder les choses liées ensemble. Faire un seule chose, et le faire bien. Le "module" qui affecte une ressource est responsable de la libérer et doit fournir une fonction pour le faire qui garde le soin pour les pointeurs, aussi. Pour n'importe quelle ressource spécifique, vous avez alors exactement un endroit où il est alloué et un endroit où il est libéré, les deux étroitement ensemble.

dit que vous voulez diviser une chaîne en sous-couches. Directement en utilisant malloc (), votre fonction doit prendre soin de tout: analyser la chaîne, attribuer la bonne quantité de mémoire, copiez les sous-couches, et et et. Rendez la fonction assez compliquée, et ce n'est pas la question si vous perdrez la trace des ressources, mais quand.

votre premier module prend soin de l'allocation de mémoire réelle:

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

il n'y a que votre seul endroit dans toute votre base de données où malloc() et free() sont appelés.

ensuite, nous devons attribuer des chaînes:

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

ils prennent soin que len+1 soit nécessaire et que le pointeur soit réglé à NULL lorsqu'il est libéré. Fournir une autre fonction pour copier un substrat:

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

il prendra soin si index et len sont à l'intérieur de la chaîne src et le modifiera si nécessaire. Il appellera StringAlloc pour dst, et il se souciera que DST soit correctement terminé.

maintenant vous pouvez écrire votre fonction split. Vous n'avez plus à vous soucier des détails de bas niveau plus, il suffit d'analyser le chaîne et obtenir les sous-chaînes. La plupart de la logique est maintenant dans le module où il appartient, au lieu d'mélangés ensemble dans une grande monstruosité.

bien sûr, cette solution a ses propres problèmes. Il fournit des couches d'abstraction, et chaque couche, tout en résolvant d'autres problèmes, vient avec son propre ensemble d'entre eux.

il n'y a pas vraiment de partie" plus importante " à laquelle des deux problèmes vous essayez d'éviter. Vous devez vraiment, vraiment éviter les deux si vous voulez écrire un logiciel fiable. Il est également très probable que l'un ou l'autre des éléments ci-dessus conduise à la corruption de données, ayant votre webserver repassé et d'autres plaisir le long de ces lignes.

il y a aussi une autre étape importante à garder à l'esprit - placer le pointeur à NULL après free'ing ce n'est que la moitié du travail. Idéalement, si vous êtes en utilisant cet idiome, vous devriez également envelopper l'accès de pointeur dans quelque chose comme ceci:

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

juste le réglage du pointeur lui-même à NULL aura seulement le programme crash inopportun places, ce qui est probablement mieux que corrompre silencieusement des données, mais ce n'est toujours pas ce que vous voulez.

il n'y a aucune garantie que le programme crash lors de l'accès au pointeur NULL.

peut-être pas par la norme, mais vous auriez du mal à trouver une implémentation qui ne la définisse pas comme une opération illégale qui provoque un crash ou une exception (comme approprié à l'environnement runtime).