Discussion :

[poukill]

Bonjour à tous,

Juste une petite question sur une memory leak éventuelle:
J'ai une classe CFP qui contient un pointeur vers une autre classe Warping:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
class CFP {
 
public:
	CFP(Choc & c, std::string nom_cfp); //Constructeur avec nom du composant
	CFP(Choc &c);
	virtual ~CFP();
 
// autre fonctions membres...
 
private:
       Warping *warping; //Pointeur vers un objet de la classe Warping
 
};

Est-ce que le destructeur doit faire cet appel explicite:

Code :

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

1
2
3
4
5
CFP::~CFP()
{
	delete[] warping; //Nécessaire ici???
	std::cout << "CFP détruit :" << nom_composant << std::endl;
}

Merci !

[Matthieu Brucher]

Ben oui, sinon comment savoir que l'espace pointé par warping doit être supprimé ?

[Luc Hermitte]

Si c'est un tableau, pourquoi ne pas avoir un vecteur ?
Et attention aux copies que tu n'as pas interdites.

[poukill]

Ben oui, sinon comment savoir que l'espace pointé par warping doit être supprimé ?

Oui bien sur, mais je m'y remet doucement au C++ là, et j'avais un doute...

Si c'est un tableau, pourquoi ne pas avoir un vecteur ?
Et attention aux copies que tu n'as pas interdites.

C'est pas un tableau, c'est un pointeur vers un objet de la classe warping...
J'aurai du utilisé

Code :

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

delete warping;

à la place???

Concernant l'interdiction des copies, je ne savais pas qu'il fallait le faire, et je ne sais pas comment le faire... Je peux avoir un petit coup de pouce?

Merci

[Laurent Gomila]

J'aurai du utilisé

Code :

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

delete warping;

à la place???

Oui, puisqu'il a été alloué avec new et non new[].

Concernant l'interdiction des copies, je ne savais pas qu'il fallait le faire, et je ne sais pas comment le faire... Je peux avoir un petit coup de pouce?

Disons qu'il y a deux cas à distinguer :
- Ta classe est copiable : dans ce cas il faut définir correctement tout ce qu'il faut pour qu'elle le soit vraiment (constructeur par copie, opérateur d'affectation, destructeur)
- Ta classe n'est pas copiable : pour éviter que quelqu'un tente de la copier, ce qui entraînerait des résultats incohérents (ou des crashs selon le contexte), on peut interdire la copie, en déclarant le constructeur par copie et l'opérateur d'affectation, mais en accès privé et en ne les implémentant pas. Ainsi toute tentative de les utiliser se soldera par une erreur de compilation.

Pourquoi ici ta classe n'est pas copiable ? Car si tu construis une copie d'une instance, elle ira pointer vers la même donnée warping en mémoire, et lorsque tes deux instances seront détuites elles libéreront toutes deux ce même objet warping, ce qui fait une fois de trop.

Bien sûr si tu veux rendre ta classe copiable (ça dépend de la sémantique que tu veux lui donner) tu peux englober ton pointeur dans un boost::shared_ptr par exemple.

[olive_le_malin]

heu ... à aucun moment je n'ai vu que c'est sa classe qui allouait ce pointeur Warping*
Donc si ça n'est pas le cas, pourquoi devrait-il le désallouer dans son constructeur ?

@+

[Laurent Gomila]

heu ... à aucun moment je n'ai vu que c'est sa classe qui allouait ce pointeur Warping*

C'est suggéré par sa première question.

[olive_le_malin]

Poukill ???
Est-ce ta classe qui l'alloue ?

Remarque : je sais que c'est pas laquestion, mais ne faudrait-il pas mettre qques const dans les paramètres des différents constructeurs ??

@+

[Laurent Gomila]

La classe peut très bien être responsable de la destruction de l'objet même si elle ne l'a pas alloué. C'est ce qui se passe par exemple pour les pointeurs intelligents.

Quant aux paramètres const ou non, il faudrait commencer par savoir ce que font les constructeurs. Ce n'est pas parce qu'une référence n'est pas const qu'il y a une erreur

[olive_le_malin]

La classe peut très bien être responsable de la destruction de l'objet même si elle ne l'a pas alloué. C'est ce qui se passe par exemple pour les pointeurs intelligents.

Oui ... De toute façon, je le disais juste au cas où ...
Seul Poukill le sait.

Quant aux paramètres const ou non, il faudrait commencer par savoir ce que font les constructeurs. Ce n'est pas parce qu'une référence n'est pas const qu'il y a une erreur

Oui. C'était juste une remarque destinée à porter l'attention sur un éventuel oubli.

@+

[poukill]

Oui, puisqu'il a été alloué avec new et non new[].


Disons qu'il y a deux cas à distinguer :
- Ta classe est copiable : dans ce cas il faut définir correctement tout ce qu'il faut pour qu'elle le soit vraiment (constructeur par copie, opérateur d'affectation, destructeur)
- Ta classe n'est pas copiable : pour éviter que quelqu'un tente de la copier, ce qui entraînerait des résultats incohérents (ou des crashs selon le contexte), on peut interdire la copie, en déclarant le constructeur par copie et l'opérateur d'affectation, mais en accès privé et en ne les implémentant pas. Ainsi toute tentative de les utiliser se soldera par une erreur de compilation.

Pourquoi ici ta classe n'est pas copiable ? Car si tu construis une copie d'une instance, elle ira pointer vers la même donnée warping en mémoire, et lorsque tes deux instances seront détuites elles libéreront toutes deux ce même objet warping, ce qui fait une fois de trop.

Bien sûr si tu veux rendre ta classe copiable (ça dépend de la sémantique que tu veux lui donner) tu peux englober ton pointeur dans un boost::shared_ptr par exemple.

Là chapeau... ^^
Je suis toujours étonné de la faciliter que des gens ont pour déduire des trucs de mon programme que je ne sais pas moi-même !!!

Effectivement, ton cheminement est très logique, ma classe CFP n'est pas copiable! J'ai bien compris ce que tu viens de m'expliquer, je te remercie donc.

@Olive_le_malin:

Oui, c'est bien ma classe CFP qui va allouer de la mémoire pour la classe Warping, mais pas dans son constructeur:

Code :

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

1
2
3
4
5
void CFP::lancerWarping()
{
	warping = new Warping(getChocNum(), nom_composant, image_visible, image_IR_ref);
	image_IR_warped = warping->lancerWarping();
}

Pour ce qui est des const, je sais pas trop encore, a priori c'est ok comme c'est maintenant!
Merci tout de même pour les conseils...

Bonne continuation,

[Luc Hermitte]

Je suis toujours étonné de la faciliter que des gens ont pour déduire des trucs de mon programme que je ne sais pas moi-même !!!

A forcede maintenir d'autres codes, cela devient un réflexe pour ma part.
Si je vois un truc qui pourrait déstabiliser les copies, je blinde en les interdisant. Si cela compile toujours tant mieux, c'était encore un de ces trucs faits à la va-vite. Sinon, je sais que j'ai initié la neutralisation d'un bug.

[poukill]

^^
Merci Luc!

Pendant que j'y suis, une toute dernière petite question :

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
class Choc {

public:
	Choc (int num_choc); // Constructeur général
	Choc ();
	~Choc();

	void ajouterComposant(CFP*);
	std::vector <CFP*> getTableauCFP();
	int getNumeroChoc() const ;
	void setNumeroChoc (int);

private:
	std::vector <CFP*> tableau_cfp;
	int numero_choc;
};

Mon destructeur actuel est:

Code :

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

1
2
3
4
Choc::~Choc()
{
	std::cout << "Choc numero" << numero_choc << " detruit"<< std::endl;
}

Le vector va t-il se détruire tout seul ?
OU bien manque t-il qqch dans mon code?

[Matthieu Brucher]

Le vecteur va se détruire, mais pas ce qui est pointé par les pointeurs.

[poukill]

J'ai maintenance ce code ci, est-il correct???

Code :

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

1
2
3
4
5
6
7
8
Choc::~Choc()
{
	std::vector<CFP*>::const_iterator i;
	for (i=tableau_cfp.begin();i!=tableau_cfp.end();i++)
		delete *i;
 
	std::cout << "Choc numero" << numero_choc << " detruit"<< std::endl;
}

Merci

[olive_le_malin]

salut,
tu peux aussi voir ça :
http://c.developpez.com/faq/cpp/?pag...elete_sequence

[poukill]

Merci Olive...
Je ne vais plus assez souvent dans le FAQ... ^^
Ce code a l'avantage de pouvoir s'appliquer à n'importe quel type de données dans un type de conteneur donné...

Dans mon projet, le coté "template" n'est pas primordial... Mais je note !

[olive_le_malin]

Salut,

moi je rajoute dans le destructeur, aprés la supression des éléments un :

Code :

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

tableau_cfp.clear()

Je sais bien que le destructeur du vector est sensé le faire ...
mais bon, je trouve que c'est bien d'en être sûr.

Si c'est pas correct, dites le moi.

@+

[Laurent Gomila]

Je sais bien que le destructeur du vector est sensé le faire ...
mais bon, je trouve que c'est bien d'en être sûr.

Si c'est pas correct, dites le moi.

C'est correct, mais inutile. Le destructeur de std::vector détruit bien les éléments, ça ne fait aucun doute

[olive_le_malin]

ouais t'as raison ...
Et puis je viens de réfléchir :
je le fais pas pour les std::string, alors que si j'étais cohérent je devrais faire pour chaque donnée membre de type string :

Code :

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

m_string.erase(m_string.begin(), m_string.end()).

Je sais pas .. le fait que ce soit des vecteus ou des list ça me donnait envie de les clearer() moi même.

Je m'en vais de ce pas corriger mes inutilités.
Merci
@+