Discussion :

[MacInTouch]

Bonjour à tous,

Je me permets de vous contacter car j'ai une question à vous poser concernant l'allocation et la désallocation en C++

Je dispose d'une classe basique :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
class Test
{
 
	private:
 
	    char * pointeur;
 
	public:
 
	    Test( const char * chaine )
	    {
	        pointeur = new char[ 10 ];
	        strcpy( pointeur, chaine );
	    }
 
	    ~Test()
	    {
	        delete[] pointeur;
	        pointeur = NULL;
	    }
 
	    void show()
	    {
	    	std::cout << pointeur <<std::endl;
	    }
};

et ma fonction main :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
int main()
{
	Toto t1("ooo"); 
	Toto t2("aaaa");
	Toto t3 = t2;
 
	t3.show();
 
	return 1;
}

L'erreur que j'obtiens dans la console est la suivante :

a.out(6947) malloc: *** error for object 0x10de008d0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

Mon erreur provient du destructeur quand je fais delete[] pointeur;

Je ne comprends pas pourquoi alors que j'ai bien fais l'allocation à l'aide de new[] utiliser delete[] devrait donc être la solution...

Merci pour votre aide :/

MacInTouch.

[djuju]

Salut,

Bon, je suppose que les Toto de ton main, sont en fait des Test.

Tu as un bug parce que tu n'as pas d'opérateur de copie explicite pour gérer ton pointer.
À la ligne 5 de ton main tu copie t2 dans t3. Le pointer de t3 et de t2 pointent donc au même endroit: le tableau de char alloué par t2.
Disons que t2 est détruit en premier il désalloue le tableau. Lorsque t3 est à son tour détruit, il essaye lui aussi de désallouer le tableau, mais comme c'est déjà fait, tu plante

Tu dois déclarer un opérateur de copie explicite pour que chaque instance de Test ait son propre tableau. Dans cet opérateur, tu alloues un tableau de la même taille que celui de l'instance à copier, puis tu copie le contenu.

[ternel]

Lis donc la faq sur les formes canonique (celle-ci et les suivantes)

Tu y liras, entre autre, que chaque fois que tu écris un destructeur non trivial pour un type T, il faut gérer les constructeurs, la construction par copie ( T(const T&) ) et l'assignation (operator=).

[Niark13]

Bonjour,

À la ligne 5, tu crées un objet t3 comme une copie de t2. Le problème est que tu ne définis pas de constructeur de copie dans ta classe Test. Le compilateur en crée donc un pour toi, qui se contente de copier bêtement t2.pointeur dans t3.pointeur.

Tu as donc deux objets dont les membres pointeur pointent sur la même chaine de caractères.

À la fin de ta fonction, tes objets sont détruits dans l'ordre inverse de leur création. t3 est donc détruit en premier et son constructeur libère la mémoire pointée par t3.pointeur. Ensuite, c'est au tour de t2 d'être détruit. Problème : t2.pointeur pointe vers une adresse qui a déjà été libérée par le destructeur de t3.

Appeler delete[] sur quelque chose qui ne pointe plus sur une donnée allouée par new[] est une erreur qui peut provoquer des bugs sympatiques comme une corruption mémoire. C'est à éviter absolument.

[ternel]

ce genre de problématique est d'ailleurs la raison même de l'existance des pointeurs intelligents (smart pointers), dans boost et maintenant dans la norme STL (depuis C++11)

[koala01]

Salut,

C'est, effectivement, un problème classique, dans le sens où un pointeur est une variable (non signée) numérique "comme les autres", qui peut etre copiée et assignée, mais qui représente... l'adresse mémoire à laquelle on va trouver une information.
L'une des formes canoniques de Coplien nous indique que chaque classe doit disposer de quatre fonction particulières:

1. Le constructeur
2. Le constructeur par copie
3. L'opérateur d'affectation " = "
4. Le destructeur

Si l'on ne donne pas de raison au compilateur de ne pas le faire en les définissant nous même, il va implicitement définir ces fonctions sous une forme publique, inline, non virtuelle (pour le destructeur, les autres ne pouvant pas être virtuels ) avec des comportements de base :

Ainsi, il produira:

1. Un constructeur public ne prenant pas d'argument et appelant les (pseudo) constructeurs sans arguments des membre de la classe dans l'ordre de leur déclaration si tu ne fournis aucun un constructeur
2. Un constructeur public par copie (prenant une référence constante sur ton type) utilisant les (pseudo) constructeurs par copie des membres de la classe dans l'ordre de leur déclaration
3. un opérateur d'affectation public, qui renverra une copie de la référence constante passée en paramètre sous la forme d'une référence
4. un destructeur non virtuel et public qui ne fait rien (qui se contente du comportement de destruction des membres de la classe dans l'ordre inverse de leur déclaration, comme pour toute variable déclarée "sur la pile")

Le problème, avec ces comportements "de base", c'est que, si ta classe a un pointeur comme membre, le constructeur par copie va, tout simplement, copier la valeur numérique (non signée) du pointeur.

Cela signifie que tu auras deux objets "distincts" dont le membre pointe vers... une adresse mémoire identique

Du coup, voici à peut près ce qui se passe avec ton code quand il a la forme de:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
int main()
{
	Toto t1("ooo"); // pas de problème, le constructeur est appelé,t1.pointeur est alloué avec new[] 
	Toto t2("aaaa");// pas de problème, le constructeur est appelé,t2.pointeur est alloué avec new[] 
	Toto t3 = t2; // CRACK seule l'adresse de la mémoire représentée
                      // par t2.pointeur est copiée dans t3.pointeur
 
	t3.show();
 
	return 1;
} // 1- le destructeur de t3 est appelé (CRAAACK) : il invoque delete sur t3.pointeur
  // 2- le destructeur de t2 est appelé : BOOOUM : il invoque delete sur
  //    t2.pointeur qui correspond à l'adresse mémoire qui a déjà été libérée
  //    lors de la destruction de t3

Il faut donc veiller à ce que la copie fasse une "copie en profondeur" du pointeur.

Comprends par là qu'il faut que la copie:

1. Alloue un espace mémoire suffisant pour contenir l'ensemble des éléments de l'objet copié.
2. copie effectivement l'ensemble des éléments de l'objet copié dans l'objet de destination.

sous une forme (temporaire) qui pourrait etre proche de

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
class Test
{
 
	private:
 
	    char * pointeur;
 
	public:
 
	    Test( const char * chaine )
	    {
	        pointeur = new char[ 10 ];
	        strcpy( pointeur, chaine );
	    }
            Test(const Test & rhs)
            {
                pointeur = new char[10];
                strcoy(pointeur, rhs.pointer);
            }
	    ~Test()
	    {
	        delete[] pointeur;
	        pointeur = NULL;
	    }
 
	    void show()
	    {
	    	std::cout << pointeur <<std::endl;
	    }
};

Mais ce ne sera pas suffisant, car le problème viendrait avec un code proche de

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
int main()
{
	Test t1("ooo"); // pas de problème, le constructeur est appelé,t1.pointeur est alloué avec new[] 
	Test t2("aaaa");// pas de problème, le constructeur est appelé,t2.pointeur est alloué avec new[] 
	Test t3 = t2; // Plus de problème ici, t3.pointeur pointe sur une adresse mémoire différente de t2.pointeur
         t1 = t2; // Gros problème ici : t1.pointeur est alloué deux fois:
                  // 1- lors de la déclaration de t1
                  // 2- lors de la copie de t2 dans t1 qui nous fait perdre la valeur originelle de t1.pointeur ==> fuite mémoire
 
	t3.show();
 
	return 1;
}

Il faut donc veiller à ce que l'affectation évite la fuite mémoire, et, pour cela, rien ne vaut l'idiome "copy and swap".

En effet, on a la certitude, si l'on crée un objet de type Test, que la mémoire allouée à son membre "pointeur" sera correctement libérée lorsque l'objet est détruit.

On a aussi la certitude que, si l'on provoque la copie d'un objet de type Test, la copie est "correcte", vu qu'on vient de définir le constructeur par copie qui a le comportement adéquat

On sait enfin (c'est la norme qui nous l'assure ) que, si l'on crée une variable(quelle qu'elle soit) "sur la pile", son destructeur est automatiquement appelé lorsque l'on quitte la portée dans laquelle elle a été déclarée.

L'idée est donc que, si l'on crée, dans l'opérateur d'affectation, une variable de type Test (selon l'exemple), elle sera détruite automatiquement lorsque l'on sortira de l'opérateur d'affectation.

Si l'on s'arrange pour intervertir tous les membres de la copie avec tous les membres de l'élément sur lequel on travaille, l'élément sur lequel on travaille représentera, effectivement, toutes les valeurs "d'origine" de la copie et ce sont... les membres "d'origine" du membre sur lequel on travaille qui seront détruit lorsque la copie sera automatiquement détruite

Ce comportement d'interversion pourrait être "naïvement" produit sous une forme proche de

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
char * temp = copie.pointeur;
copie.pointeur = origine.pointeur;
origine.pointeur = temp;

et s'appelle, en anglais, un "swap".

Pour garder des fonctions aussi simples que possibles (dans le cas présent, ca va, il n'y a qu'un membre à "swaper", mais il y a souvent bien plus de membres ), on va donc créer une nouvelle fonction (qui peut etre privée, car "à usage interne uniquement") "swap" qui fera le travail

Mais, comme le développeur informatique est paresseux de nature, je te propose d'utiliser un comportement fournis par le standard qui fait exactement ce que l'on veut : la fonction swap, disponible (comme tout ce qui vient du standard) dans l'espace de noms std par simple inclusion du fichier d'en-tête <algorithm>.

Au final, ta classe Test ressemblerait à quelque chose comme

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
#include <algorithm> // pour disposer de std::swap
class Test
{
	private:
	    char * pointeur;
            void swap(Test & copie)
            {
                // à faire pour tous les membres de Test ;)
                std::swap(this.pointeur, copie.pointeur); // intervertit le membre
            }
	public:
 
	    Test( const char * chaine )
	    {
	        pointeur = new char[ 10 ];
	        strcpy( pointeur, chaine );
	    }
            Test(const Test & rhs)
            {
                pointeur = new char[10];
                strcoy(pointeur, rhs.pointer);
            }
            Test & operator=(Test const & rhs)
            {
                Test copie(rhs); // crée une copie non constante de rhs
                swap(rhs); //voir plus bas
                return *this; // renvoie l'objet courent sous la forme d'une référence 
            } // copie est détruit ici
	    ~Test()
	    {
	        delete[] pointeur;
	        pointeur = NULL;
	    }
	    void show()
	    {
	    	std::cout << pointeur <<std::endl;
	    }
};

Et là, tu n'auras plus aucun problème avec ta classe Test (du moins, tant que tu te limiteras à 9 caractères + le '\0'): tu pourras en créer autant que tu veux, les réassigner dans tous les sens, tu n'auras plus ni fuite mémoire ni tentative de double libération de la mémoire

Un petit détail pour terminer...

Pour indiquer qu'un programme s'est correctement terminé, la fonction main renvoie la valeur 0 et non 1

[MacInTouch]

Merci beaucoup à tous surtout à toi Koala01, votre aide a été vraiment précieuse et grâce à vous j'ai résolu mon soucis.

J'aurai au moins appris quelque chose et c'est une erreur que je ne risque plus de faire !!

Bonne journée à tous =)

MacInTouch

[gbdivers]

ce genre de problématique est d'ailleurs la raison même de l'existance des pointeurs intelligents (smart pointers), dans boost et maintenant dans la norme STL (depuis C++11)

Petite précision (pour ceux qui lisent, pas pour toi leternel ) :
"l'une des raisons". Une autre raison est d'avoir un code sur en cas d'exception (voir FAQ RAII et Gérer ses ressources de manière robuste en C++)