Problème de destructeur

Version imprimable

Bonjour développeurs.

Dans le but de créer une liste chaînée, je rencontre un problème concernant le destructeur d'une classe.
J'ai deux classes:
- Maillon qui contient un pointeur sur un autre maillon (le suivant) et une donnée
- Liste qui contient deux pointeurs sur des Maillons (le premier et le dernier)

Voici ma classe Maillon :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 class Maillon { friend class Liste; // Champs prives private: T _data ; Maillon *_suivant ; // Forme canonique de Coplien public: Maillon():_data(),_suivant(NULL){} Maillon(const T& data):_data(data),_suivant(NULL){} Maillon(const Maillon& m):_data(m._data),_suivant(m._suivant){} ~Maillon() { delete _suivant; } // C'est ici que le problème se pose friend ostream& operator<< (ostream& o, Maillon& m); };
Et voici ma classe Liste :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 class Liste { private: Maillon *_tete, *_fin ; public: Liste():_tete(NULL),_fin(NULL){} Liste(const Liste& l) ; ~Liste() // probleme : le destructeur appelle deux fois le destructeur d'un meme objet { Maillon *m = _tete; Maillon *tmp = new Maillon(); while(m != NULL) { tmp = m; //(1) m = m->_suivant; //(2) delete tmp; //(3) } _tete = NULL; _fin = NULL; }
La première fois que l'on passe sur la ligne (3) on détruit l'objet tmp mais aussi son suivant. ensuite on repasse sur la ligne (1), tmp pointe alors sur un objet qui vient d’être détruit. Enfin, au deuxième passage sur la ligne (3), deuxième delete sur une même zone mémoire, le programme plante :

Citation:

Process terminated with status -1073741571 (0 minutes, 3 seconds)

22/10/2011, 16h56
quentinh
Ton destructeur ~Liste() est très étrange…

Dans le cas de listes chaînées, le destructeur de Liste ne doit détruire que le premier objet (tete) et c'est tout…
Le destructeur de Maillon va supprimer le suivant (au passage, teste si _suivant est non NULL avant de le supprimer) qui va détruire le suivant, etc…
Code:

1 2 3 4 5 6 7 8 9 10 11 12 Maillon::~Maillon() { if (_suivant != NULL) delete _suivant; _suivant = NULL; } Liste::~Liste() { if (_tete != NULL) delete _tete; _tete = NULL; _fin = NULL; }
22/10/2011, 18h07
Steph_ng8
Bonjour.

C'est une façon de faire.
Je ne sais pas si c'est la meilleure.

Mais ce n'est pas ça le plus étrange dans ton destructeur.
Code:

1 2 #line 12 Maillon *tmp = new Maillon();
Pourquoi donc construis-tu un nouveau maillon ???
Je suppose que tu voulais utiliser un pointeur temporaire, qui pointerait sur les maillons de la liste, afin de les détruire.

Il faut bien comprendre que, dans cas, on ne réserve pas une zone mémoire particulière pour ce pointeur ; il pointe sur des zonnes déjà allouées.
Donc il ne faut pas appeler new.
Surtout que toi tu pers l'adresse de la mémoire que tu viens d'allouer dès l'entrée dans la boucle... :?

Si tu veux qu'un maillon détruise son « suivant » lorsqu'il est lui-même détruit, reporte toi à la solution de quentinh ; les maillons se détruiront en cascade.
Sinon, le constructeur de Maillon n'a rien à faire, si ce n'est (éventuellement) mettre le champ _suivant à NULL, et le destructeur de Liste est correct, modulo ma remarque ci-dessus.

Selon tes besoins, tu peux définir une fonction membre qui retire le premier (et/ou dernier) élément de la liste, en faisant en sorte que les pointeurs _debut et _fin reste cohérents.
Du coup, le destructeur se contentera d'appeler cette fonction jusqu'à ce que la liste soit vide...
22/10/2011, 18h08
Jean-Pascal

D'accord, merci beaucoup!
23/10/2011, 02h01
koala01
Salut,

Personnellement, je ne crois pas qu'il soit intéressant de faire en sorte que le maillon détruise son suivant :aie:

La raison est simple : Tu peux, à un moment donné, décider de supprimer un maillon sans pour autant... suprimer celui qui le suit (et encore moins les suivants :D ) :P

De plus, la responsabilité de la décision de la destruction d'un maillon n'a, a priori, aucune raison... d'incomber à un maillon :D

Enfin, le destructeur de ta liste boucle déjà sur tous les maillons pour les détruire... Cela signifie que, au mieux, la boucle de la liste est inutile, au pire, tu vas te retrouver à essayer de détruire deux fois certains maillons, et le programme ne le supportera pas :P

Par contre, étant donné que la responsabilité de la liste est la gestion des maillons, il semble tout à fait cohérent qu'elle s'occupe aussi... de les détruire (après tout, il faut essayer de garder un certain "parallélisme" : si la liste s'occupe de créer un maillon, pourquoi ne s'occuperait-elle pas de le détruire :question:

Par contre, il faudra aussi très certainement t'intéresser à la problématique de la copie et de l'affectation! Telle que tu l'implémentes, tu armes un pistolet et tu le diriges dés à présent vers ton pied, pret à tirer :P

Il faut en effet s'assurer :
- Qu'il n'y ait aucun maillon qui, suite à une copie (une affectation) soit référencié comme suivant par deux maillons distincts car cela t'amènera fatalement à une tentative de double libération de la mémoire
- qu'il n'y aura pas de fuite mémoire si tu effectue l'affectation d'un maillon existant à un autre maillon existant
L'idiome copy and swap devrait t'intéresser pour éviter ces problèmes ;)

Enfin, mais c'est la "tuch of class" :D, il serait pas mal de mettre en place un système d'itérateur : un objet qui permet de manipuler le pointeur sur maillon sous la forme d'une référence et qui permettra d'assurer la const-correctness du maillon ;)
23/10/2011, 10h07
3DArchi

Salut

Je suis assez d'accord avec Koala. Il n'est pas très pertinent de faire détruire le pointeur dans le maillon. Comment feras-tu lorsque tu souhaiteras uniquement retirer un élément ?? La liste est clairement là pour gérer les différents maillons et entre autre les allouer et les libérer.

Autre problème : ta classe maillon est erronée. La copie telle quelle du pointeur dans le constructeur de copie Maillon(const Maillon& m):_data(m._data),_suivant(m._suivant){} est incompatible avec le destructeur ~Maillon() { delete _suivant; }. Si tu fais une copie d'un objet Maillon, le destructeur va être appelé deux fois.

Ensuite, si tu as un constructeur par copie, il serait assez logique de définir aussi l'opérateur d'affectation ! D'ailleurs, tu l'as (presque) marqué en commentaire dans ton code // Forme canonique de Coplien !!!

Ceci dit, une fois retirer la responsabilité de gestion du pointeur de Maille vers Liste, il deviendra assez naturel de ne garder que les versions implicites par défaut des destructeurs, constructeur par copie et opérateur d'affectation.