Discussion :

[phil.alex]

Bonjour,

j'ai un hash_set (membre de ma classe) de const char * que je rempli avec des données lues dans un fichier. A la destruction de la classe j'essaye de libérer la mémoire occupée par les éléments contenues dans le hash_set et là paf *** glibc detected *** ./ww: double free or corruption (fasttop): 0x098074d8 ***

Voici le code si quelqu'un à une idée je suis preneur:

WeakWords.hh

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
 
#ifndef CLASS_WEAKWORDS_HH
#define CLASS_WEAKWORDS_HH 1
 
#include <string>
#include <ext/hash_set>
 
struct eqstr
{
  bool operator()(const char* s1, const char* s2) const
  {
    return strcmp(s1, s2) == 0;
  }
};
 
class WeakWords
{
private:
	__gnu_cxx::hash_set<const char*, __gnu_cxx::hash<const char*>, eqstr> m_weakWordsSet;
	std::string m_weakWordsFileName;
	unsigned int _loadWeakWords(const char *fileName);
 
public:
	WeakWords();
	~WeakWords();
	unsigned int loadWeakWords(const std::string fileName);
};
#endif

WeakWords.cc

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
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
 
#include <string>
#include <fstream>
#include <iostream>
#include <ext/hash_set>
 
#include "WeakWords.hh"
 
using namespace std;
using namespace __gnu_cxx;
 
//
// Public
//
WeakWords::WeakWords()
{
}
 
WeakWords::~WeakWords()
{
	/* Clean m_weakWordsSet */
	__gnu_cxx::hash_set<const char*, __gnu_cxx::hash<const char*>, eqstr>::iterator weakWordsSetIt = m_weakWordsSet.begin();
	while(weakWordsSetIt != m_weakWordsSet.end())
	{
		cout << "Suppression de [" << (*weakWordsSetIt) << "]" << endl;
 
		if((*weakWordsSetIt) != NULL)
		{
// Le problème semble être ici 
			delete[] (*weakWordsSetIt);
		}
		weakWordsSetIt++;
	}
	m_weakWordsSet.clear();
}
 
unsigned int WeakWords::loadWeakWords(const string fileName)
{
	m_weakWordsFileName.assign(fileName);
	return _loadWeakWords(m_weakWordsFileName.c_str());
}
 
//
// Private
//
unsigned int WeakWords::_loadWeakWords(const char *fileName)
{
	string fileLine;
 
	ifstream weakWordsFile (m_weakWordsFileName.c_str(), ios::in);
	if(weakWordsFile.is_open() == false)
	{
		return 1;
	}
 
	while(weakWordsFile.good())
	{
		weakWordsFile >> fileLine;
		if(fileLine.length() > 0)
		{
			char *t=new char[fileLine.length()+1];
// Le memset est il necessaire ?
			//memset(t, 0, fileLine.length());
			strcpy(t, fileLine.c_str());
 
			m_weakWordsSet.insert(t);
		}
		fileLine.clear();
	}
 
	weakWordsFile.close();
	return 0;
}

[Mat007]

Salut,

Pourquoi tu n'utilises pas std::string ?

MAT.

[phil.alex]

Salut,

Pourquoi tu n'utilises pas std::string ?

MAT.

Parce qu'il semble que les hash_set, hash_map, ... ne semblent fonctionner qu'avec les types "simples" (char, int, long, ...), sauf à refaire un Template chose dont pour l'instant je ne me sens pas capable.

[Mat007]

Je pense qu'il manque le caractère de terminaison de la chaîne au moment de la lecture :

Code :

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

t[fileLine.length()] = 0;

Sinon, un peu en vrac :
. il faut passer les paramètres de type std::string par const & et non par copie
. pas la peine de tester par rapport à 0 (ou NULL) avant de faire un delete (l'implémentation du delete le fait)
. tu es sûr que tu as besoin d'un hash_set et qu'un std::set ne "suffirait" pas ?
. fileLine n'a pas besoin d'être déclaré si tôt (et du coup pas besoin de le clear si la variable est locale)
. _loadWeakWords devrait manifestement renvoyer un booléen plutôt qu'un entier
. tu peux utiliser des typedef pour alléger l'écriture/lecture du type des itérateurs
. il vaut mieux faire ++weakWordsSetIt plutôt que weakWordsSetIt++
. pouquoi faire un while plutôt qu'un for dans ~WeakWords ?

MAT.

[Mat007]

Je pense qu'il manque le caractère de terminaison de la chaîne au moment de la lecture

Quoique non, je délire là !
Trop longtemps que j'ai pas fait de C tiens...

MAT.

[phil.alex]

Merci Mat pour tes remarques (j'ai modifié le code en conséquence). J'ai une question:
-pourquoi utiliser ++weakWordsSetIt plutôt que weakWordsSetIt++

Et oui je pourrais utiliser un set à la place d'une hash_set, mais je n'arrive toujours pas à comprendre pour mon destructeur ne fonctionne pas (j'aime bien savoir le pourquoi du comment ).

[Mat007]

pourquoi utiliser ++weakWordsSetIt plutôt que weakWordsSetIt++

Un élément de réponse est donné dans la FAQ.

Tiens un autre truc aussi :

Code :

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

m_weakWordsFileName.assign(fileName);

S'écrit quand même plus simplement :

Code :

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

m_weakWordsFileName = fileName;

Sinon pour le problème de gestion de la mémoire je ne vois pas vraiment là comme ça, mais il faut dire que j'ai vraiment plus du tout l'habitude de gérer les allocations/désallocations à la main...

MAT.