Discussion :

[babar63]

Bonjour à tous,

J'ai développez il y a quelques temps une classe de smart pointer de type :

Code :

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

1
2
3
4
5
6
template <class T>
class CSmartPointer
{
        T   *m_Pointer;
        int *m_Counter;
};

J'ai déjà eu l'occasion de faire quelques tests, il "semblerait" qu'ils soient fonctionnels. J'ai pu lire que dans mon cas de figure seul les références circulaires empêchées les smarts pointers de fonctionner. Or j'ai découvert un bug plutôt "étrange" (tout du moins incompris de moi). Voilà le détail de ce qui se passe :

Code :

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

1
2
3
4
5
6
class CSceneManager{
     //...
     bool ReadScene(std::string const &Filename);
     bool ReadScene(CNodePtr Root);
     CNodePtr m_Root;
}

Il faut savoir que CNodePtr est donc de type CSmartPointer<CNode> (CNode n'étant pas important ici). m_Root est initialisé a NULL par défaut et le compteur à 1. Maitenant dans la fonction ReadScene :

Code :

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

1
2
3
4
5
6
7
8
9
bool CSceneManager::ReadScene(std::string const &Filename)
{
     //...
 
     // Load the scene :
     CNodePtr NodeTmp = Loader->LoadScene(Filename);  // compteur à 1!!
     bool b = ReadScene(NodeTmp);                     // compteur à 2
     return b;                                        // compteur à 0
}

Une petite explication s'impose, Loader est une instance de classe permettant de charger des scènes, la fonction LoadScene renvoie un CNodePtr correspondant à la scène chargée. Et enfin :

Code :

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

1
2
3
4
5
6
bool CSceneManager::ReadScene(CNodePtr Root)
{
     //...
     m_Root = Root;   //compteur à 3
     return true;
}

Résultat l'objet est détruit... et je bloc dessus depuis un bon moment déjà alors que ça à l'air idiot (mais bon c'est toujours comme ça)
Si vous avez besoin de plus de code (CSmartPointer, fonctions...) n'hésitez pas à demander...

Merci d'avance et bonne fin de journée

[Mat007]

Salut,

Plutôt que de perdre du temps à (mal) refaire la roue pourquoi tu n'utilises pas boost ?

MAT.

[babar63]

Plutôt que de perdre du temps à (mal) refaire la roue

Cette argument est souvent cité, personnellement je fais partie de ceux qui aiment "refaire la roue", la question n'est pas de la refaire (bien ou mal), je continuerai d'ailleurs d'utiliser boost, mais le but est d'apprendre... ce n'est donc pas une perte de temps à mon avis (mais bon après c'est personnel).

J'utilise ma classe dans ce projet afin de la tester tout simplement...

[Mat007]

Ok, mettons...

Il faudrait sans doute en effet le code de CSmartPointer pour savoir exactement ce qui se passe.

MAT.

[babar63]

Voila un code plus complet sur ma classe mais je ne vois toujours pas d'erreur

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
template <class T>
class CSmartPointer
{
public:
 
	// Constructor :
	CSmartPointer(T *Pointer = NULL);
	CSmartPointer(CSmartPointer<T> const &SmartPointer);
 
	// Destructor :
	~CSmartPointer();
 
	// Operator :
	CSmartPointer<T> & operator = (CSmartPointer<T> const &SmartPointer);
	//...
 
	//....
 
private:
 
	T		*m_Pointer;			// Pointer on the object
	int		*m_Counter;			// Counter on the object pointed
};

Et le .inl mais toujours rien de compliqué :

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
//Constructor (1param) :
template <class T>
inline CSmartPointer<T>::CSmartPointer(T *Pointer)
{
	// Initialisation :
	m_Pointer = Pointer;
	m_Counter = new int(1);
}
 
//Copy constructor :
template <class T>
inline CSmartPointer<T>::CSmartPointer(CSmartPointer<T> const &SmartPointer)
{
	// Copy :
	m_Pointer = SmartPointer.m_Pointer;
	m_Counter = SmartPointer.m_Counter;
 
	// Incrementation :
	++(*m_Counter);
}
 
//Destructor :
template <class T>
inline CSmartPointer<T>::~CSmartPointer()
{
	// Decrementation :
	if( --(*m_Counter) == 0 )
	{
		delete m_Pointer;
		delete m_Counter;
	}
}
 
//Operator = :
template <class T>
inline CSmartPointer<T> & CSmartPointer<T>::operator = (CSmartPointer<T> const &SmartPointer)
{
	Assert( (this != &SmartPointer) );
 
	// Decrementation :
	if( --(*m_Counter) == 0 )
	{
		delete m_Pointer;
		delete m_Counter;
	}
 
	// Copy :
	m_Pointer = SmartPointer.m_Pointer;
	m_Counter = SmartPointer.m_Counter;
 
	// Incrementation :
	++(*m_Counter);
 
	return *this;
}

Je passe ici les détails sur les opérateurs de casts, prédicat etc...

Mais ce qui me surprend le plus c'est à ce niveau la :

Code :

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

CNodePtr NodeTmp = Loader->LoadScene(Filename);  // compteur à 1!!

Le compteur devrait être à deux (je suppose) mais je ne trouve pas l'erreur

[vdumont]

Tu peux montrer un main et ton Loader pour qu'on voit un cas typique d'utilisation?


Note:

Code :

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

1
2
3
	// Copy :
	m_Pointer = SmartPointer.m_Pointer;
	m_Counter = SmartPointer.m_Counter;

Ici j'espère que t'es conscient que tu ne fais pas une copy véritable mais tu fais seulement pointer tes pointeurs sur les mêmes addresses que SmartPointer ?

(Aussi, ourquoi utiliser un pointeur sur un entier au lieu de tout simplement utiliser un entier? Je vois pas l'intérêt dans ton cas.)

[corrector]

Code :

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

1
2
3
4
5
6
7
template <class T>
class CSmartPointer
{
public:
 
	// Constructor :
	CSmartPointer(T *Pointer = NULL);

C'est volontairement que tu déclare une conversion implicite de T* vers CSmartPointer?

Si ce n'est pas le cas, ajoute le mot-clef "explicite".

Code :

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

1
2
3
4
5
6
7
template <class T>
inline CSmartPointer<T>::CSmartPointer(T *Pointer)
{
	// Initialisation :
	m_Pointer = Pointer;
	m_Counter = new int(1);
}

Il est idiomatique d'utiliser la liste d'initialisation, même quand ça ne change pas la signification du code, comme ici (les membres ont tous un type de base).

Code :

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

1
2
3
4
5
6
7
//Copy constructor :
template <class T>
inline CSmartPointer<T>::CSmartPointer(CSmartPointer<T> const &SmartPointer)
{
	// Copy :
	m_Pointer = SmartPointer.m_Pointer;
	m_Counter = SmartPointer.m_Counter;

Idem.

Code :

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

1
2
3
4
5
6
7
8
9
	// Incrementation :
	++(*m_Counter);
}
 
//Operator = :
template <class T>
inline CSmartPointer<T> & CSmartPointer<T>::operator = (CSmartPointer<T> const &SmartPointer)
{
	Assert( (this != &SmartPointer) );

Pourquoi assert? Tu veux interdire l'auto-affectation?

Mais ce qui me surprend le plus c'est à ce niveau la :

Code :

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

CNodePtr NodeTmp = Loader->LoadScene(Filename);  // compteur à 1!!

Le compteur devrait être à deux (je suppose) mais je ne trouve pas l'erreur

Des détails sur LoadScene?