Discussion :

[christouf]

bonjour,

j'ai parcouru le forum et la faq..en vain.. J'ai toujours un problème de fuite mémoire, qui je pense vient de ma list.

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
class PacketListEntry {
public:
	Packet *p;
	double rxStartTime;
	double rxEndTime;
	double Pr;
	int right_ths;
 
	PacketListEntry(Packet *packet,double start,double end ,double pr,int ths):
		p(packet),rxStartTime(start),rxEndTime(end),Pr(pr),right_ths(ths){
 
	}
	PacketListEntry(){
 
	}
	~PacketListEntry(){
		delete p;
	}
};
typedef std::list<PacketListEntry> PacketList_t;

exemple d'ajout et de suppression :

Code :

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

1
2
3
4
5
6
7
8
9
 
DetectList.push_back(*(new PacketListEntry(p,start,end,Pr,0)));//DetectList de type PacketList_t
 
for (PacketList_t::iterator it = DetectList.begin(); it!= DetectList.end();) {
	        delete it->p;
		it->p=NULL;
		it = DetectList.erase(it);
	}
DetectList.clear();

pour vous c'est bon?

merci

[Spout]

1. A aucun moment tu ne vérifie la validité de ton Packet,
2. Tu ne supprime jamais l'espace mémoire alloué par le

Code :

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

new PacketListEntry(p,start,end,Pr,0)

lors de l'ajout dans ta liste.

[christouf]

l'utilisation d'un objet statique résoud donc le problème?

Code :

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

DetectList.push_back(PacketListEntry(p,start,end,Pr,0));

[Spout]

Je pense que oui, à la condition que tu modifies la déclaration de ta liste.
Elle doit devenir :

Code :

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

std::list<PacketListEntry>

et non plus:

Code :

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

std::list<PacketListEntry*>

EDIT: au temps pour moi, j'avais pas vu la déclaration de la liste

[Sylvain Togni]

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
class PacketListEntry {
public:
	Packet *p;
	double rxStartTime;
	double rxEndTime;
	double Pr;
	int right_ths;
 
	PacketListEntry(Packet *packet,double start,double end ,double pr,int ths):
		p(packet),rxStartTime(start),rxEndTime(end),Pr(pr),right_ths(ths){
 
	}
	PacketListEntry(){
 
	}
	~PacketListEntry(){
		delete p;
	}
};
typedef std::list<PacketListEntry> PacketList_t;

Il manque le constructeur de copie et l'opérateur d'affectation à cette classe.

Code :

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

DetectList.push_back(*(new PacketListEntry(p,start,end,Pr,0)));//DetectList de type PacketList_t

à remplacer par

Code :

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

DetectList.push_back(PacketListEntry(p,start,end,Pr,0));

effectivement.

Code :

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

1
2
3
4
5
for (PacketList_t::iterator it = DetectList.begin(); it!= DetectList.end();) {
	        delete it->p;
		it->p=NULL;
		it = DetectList.erase(it);
	}

Cette boucle est à supprimer, la libération s'effectue dans le destructeur des objets.

[Matthieu Brucher]

Le packet va être détruit dès l'insertion dans la liste puis effacer plusieurs fois dès que tu vas travailler sur la liste (car la STL utilise des copies à tout bout de champs et non des références constantes). Donc tu ferais peut-être mieux de passer par des pointeurs intelligents.

[christouf]

cela va donc fonctionner sans pointeur intelligent mais ce n'est pas optimal?

j'ai redéfini ma classe de cette façon :

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
class PacketListEntry {
public:
	Packet *p;
	double rxStartTime;
	double rxEndTime;
	double Pr;
	int right_ths;
	PacketListEntry(){
	}
	PacketListEntry(Packet *packet,double start,double end ,double pr,int ths):
		p(packet),rxStartTime(start),rxEndTime(end),Pr(pr),right_ths(ths){
	}
	PacketListEntry(const PacketListEntry& PacketList)
	:p(PacketList.p->Copy()),rxStartTime(PacketList.rxStartTime),Pr(PacketList.Pr),right_ths(PacketList.right_ths){
	}
	PacketListEntry& operator= (const PacketListEntry& p); 
	~PacketListEntry(){
		delete p;
	}
};

les éléments de la liste sont ensuite effacés en recherchant l'uid du packet p grâce au code suivant :

Code :

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

1
2
3
4
5
6
struct PacketListEntryEgal :
	public binary_function<PacketListEntry, Count_t, bool> {
	bool operator()(const PacketListEntry & m, Count_t nb) const {
		return (m.p)->uid == nb;
	}
};

Code :

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

1
2
3
4
5
6
7
8
9
10
11
bool L1Proto_UWBmicrotrame::RemoveDetectListEntry(Packet *p) {
	PacketList_t::iterator it;
	it = find_if(DetectList.begin(), DetectList.end(), bind2nd(
			PacketListEntryEgal(), p->uid));
	if (it!=DetectList.end()) {
		DetectList.erase(it);
		return true;
	} else {
		return false;
	}
}

est-ce qu'une map ne serait pas plus efficace pour les nombreux suppressions d'éléments?

[Matthieu Brucher]

OK, dans ton constructeur de recopie, tu crées explcitement une copie, donc ça devrait passer.