[template] un smart pointer parmi d'autres

**camboui** · 19/11/2008, 16h27

Voici un code de "smart pointer" que je viens de trouver. Il me parait simple et assez universel.
Détail amusant, un operator "virgule" est défini. C'est bien la première fois que je vois ça.

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
template<class T>
class auto_counter
{
	long f_cnt;
	T *f_ptr;
	long Keep() { return ++f_cnt; }
	long Release() { return --f_cnt; }
	auto_counter()
		:f_cnt(1),f_ptr(0) {}
	explicit auto_counter(T *ptr)
		:f_cnt(1),f_ptr(ptr) {}
	auto_counter(const auto_counter<T> & r)
		:f_cnt(r.f_cnt),f_ptr(r.f_ptr) {}
	auto_counter<T> & operator=(const auto_counter<T> & r)
		{ f_cnt=r.f_cnt;f_ptr=r.f_ptr;return *this; }
public:
	T *Get()
		{ return f_ptr; }
	static auto_counter<T> *Init(T *ptr)
		{ return new auto_counter<T>(ptr); }
	static void Done(auto_counter<T> *p)
	{
		if (p && p->Release()==0)
		{
			delete p->f_ptr;
			delete p;
		}
	}
	static void Copy(auto_counter<T> **dpp,auto_counter<T> *sp)
	{
		if (!sp)
		{
			if (*dpp) Done(*dpp),*dpp=0;
		}
		else if (!*dpp)
			(*dpp=sp)->Keep();
		else if ((*dpp)->f_ptr!=sp->f_ptr)
		{
			Done(*dpp);
			(*dpp=sp)->Keep();
		}
	}
};
 
 
template<class T>
class smart_ptr
{
	auto_counter<T> *f_ac;
public:
	smart_ptr()
		:f_ac(NULL) {}
	explicit smart_ptr(T *ptr)
		:f_ac(NULL) { f_ac=auto_counter<T>::Init(ptr); }
	smart_ptr(const smart_ptr<T> & r)
		:f_ac(NULL) { auto_counter<T>::Copy(&f_ac,r.f_ac); }
	smart_ptr<T> & operator=(const smart_ptr<T> & r)
		{ auto_counter<T>::Copy(&f_ac,r.f_ac); return *this; }
	~smart_ptr()
		{ auto_counter<T>::Done(f_ac); }
	bool operator,(bool b) const
		{ return f_ac!=0 && f_ac->Get()!=0 && bool(*f_ac->Get())==b; }
	operator bool() const
		{ return f_ac!=0 && f_ac->Get()!=0; }
	T & operator*() const
		{ return *f_ac->Get(); }
	T *operator->() const
		{ return f_ac->Get(); }
	T *get() const
		{ return f_ac->Get(); }
};

Qu'en pensez-vous ?
Merci.

**Médinoc** · 19/11/2008, 16h46

Salut,

Déjà, quand je vois un opérateur virgule, ça me parait tout de suite suspect.
Ensuite, la fonction copy() bénéficierait d'un passage par référence plutôt que par pointeur
Et surtout, elle libère le pointeur existant avant d'augmenter le compteur de références du nouveau: Pas bien! (implémenter l'opérateur= par copy-and-swap serait une meilleure idée).
Ce n'est pas thread-safe, mais bon, on ne peut pas faire du thread-safe en C++ standard.
Ce pointeur intelligent fait juste la taille d'un pointeur, mais il fait une double indirection. J'ai plutôt tendance, lors d'un comptage de références non-intrusif, à stocker les deux au même niveau (mais je suis passé par des classes intermédiaires, donc ce code plus simple est peut-être plus exception-safe de ce côté-là)
Ce code ne prend pas en charge le cas d'une erreur mémoire lors du new: ptr ne sera pas libéré...

**camboui** · 19/11/2008, 20h55

Merci pour ta réaction. Elle est intéressante bien que les à-priori 1, 2 et 3 te décribilisent un peu.

Je n'avais pas réalisé le 6. En effet il serait judicieux de lever l'insécurité de ce memory leak (quoiqu'improbable).

Le 5 me perturbe aussi un peu. Mais le code est simple et clair, et en pratique je serais surpris si les performances étaient vraiment affectées par l'indirection supplémentaire. Les cas critiques mériteront une implémention plus spécifique.

D'autres avis ?
Merci.

**nikko34** · 19/11/2008, 21h11

Envoyé par camboui

Merci pour ta réaction. Elle est intéressante bien que les à-priori 1, 2 et 3 te décribilisent un peu.

D'autres avis ?

"Détail amusant, un operator "virgule" est défini. C'est bien la première fois que je vois ça."

ça te décrédibilise aussi un peu non?

plus sérieusement, tu reproches quelque chose à tr1::shared_ptr ?

**Médinoc** · 19/11/2008, 21h34

Le 3 n'est pas un à-priori, mais une véritable faille en cas d'auto-affectation (ou d'un truc plus bizarre, comme dépiler une pile chaînée qui utiliserait ce genre de pointeur.

**Alp** · 20/11/2008, 08h26

Envoyé par camboui

Détail amusant, un operator "virgule" est défini. C'est bien la première fois que

Tu n'as donc jamais vu boost.assign

Sinon, effectivement je te déconseille d'utiliser ce pointeur là, je te conseille plutôt de lui préférer tr1::shared_ptr ou encore boost.smart_pointer (il s'agit +- de la même chose) ou bien celui de Loki (http://loki-lib.sf.net/).

**Médinoc** · 20/11/2008, 09h32

Mais en fait, l'opérateur virgule, je ne vois même pas à quoi il est supposé servir, ni ce que cette redéfinition fait...

En clair, la plupart des redéfinitions de l'opérateur virgule sont contre-intuitives pour moi, et celle-ci en particulier l'est...

**camboui** · 20/11/2008, 11h13

Vouloir n'est pas pouvoir. Boost ne fait pas partie des choix proposés malheureusement. J'aurais dû le préciser dès le départ...

A propos du 3, l'auto-affectation est bien sûr un soucis, mais elle me semble ici traitée par Copy().

A propos de l'operator "virgule", il y en a un qui trouve ça suspect, et un autre qui sous entend que boost en use et abuse abondamment.
Les avis sont manifestement contrastés

Si je comprends bien son usage dans ici, il permet d'appeler l'operator bool de la classe T (qui doit donc le définir). Sans doute pour tester la "validité" d'une instance de T (?), par exemple

Code :

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

1
2
smart_ptr<T> p(...);
if (p,false) cout << "Object non utilisable" << endl;

Ce code est trouvé en interne. Je suis preneur de tout autre code qui se resume à un simple fichier header à inclure. Les usines à gaz me seront refusées (je ne décide pas).

Merci encore

**Médinoc** · 20/11/2008, 11h22

Envoyé par camboui

A propos du 3, l'auto-affectation est bien sûr un soucis, mais elle me semble ici traité par Copy().

Ah, en effet, Copy() évite l'auto-affectation pure. Mais je peux te donner un cas tordu contre lequel Copy() ne protège pas:

Code C++ :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
class UneClasse
{
public:
	smart_ptr< UneClasse > suiv;
	int i;
	UneClasse(int i) : i(i) {}
};
 
int main(void)
{
	smart_ptr< UneClasse > test(new UneClasse(1));
	test->suiv = new UneClasse(2);
	test = test->suiv; //Ici, 1 va être détruite, et détruire 2, avant que la nouvelle référence vers 2 soit créée.
	return 0;
}

**nikko34** · 20/11/2008, 14h35

Envoyé par camboui

Vouloir n'est pas pouvoir. Boost ne fait pas partie des choix proposés malheureusement. J'aurais dû le préciser dès le départ...

oui c'est pour ça que je t'ai parlé de tr1:: qui est peut être inclus à ton compilateur sans que tu n'ais rien à faire.

**camboui** · 20/11/2008, 15h59

Nikko,
tr1:: est inclus dans Visual C++ 2008 je crois. Je suis sous VC++6.0 et/ou 2003 (7.1 je pense). Le passage vers VC++ 2008 (v9.0?) est une option possible.
Se sera peut-être pour bientôt.
Merci.

Médinoc,
Les déclarations récursives mettent en péril Copy(), c'est juste. Voici une correction. J'en ai profité pour adapter Init() afin de satisfaire le point 6.

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
	static auto_counter<T> *Init(T *ptr)
	{
		std::auto_ptr<T> ap(ptr);
		auto_counter<T> *r=new auto_counter<T>(ptr);
		ap.release();
		return r;
	}
	static void Copy(auto_counter<T> **dpp,auto_counter<T> *sp)
	{
		if (sp)
			sp->Keep();
		if (*dpp)
			Done(*dpp);
		*dpp=sp;
	}

Reste le point4. Je pense qu'il suffit de remplacer ++f_cnt et --f_cnt dans Keep() et Release() par InterlockedIncrement() et InterlockedDecrement() (sous Windows). Ou en faut-il plus ?

**Médinoc** · 20/11/2008, 16h13

Généralement, on n'a pas besoin de plus. Mais je suis trop paresseux pour regarder à nouveau ce code de près.

**Matthieu Brucher** · 25/11/2008, 17h51

Envoyé par camboui

Ce code est trouvé en interne. Je suis preneur de tout autre code qui se resume à un simple fichier header à inclure. Les usines à gaz me seront refusées (je ne décide pas).

Tu prends l'en-tête shared_ptr et voilà.

De plus, les compilateurs tendent à proposer maintenant une implémentation de TR1, donc si tu as compilo récent qui le propose, fonce, si tu peux rester à jour, au moment où tu auras une implémentation de TR1, tu changes shared_ptr par le nouveau.

Et ne pas vouloir Boost alors qu'il est en standard sous Linux, c'est...

**Alp** · 26/11/2008, 14h02

Envoyé par Matthieu Brucher

Et ne pas vouloir Boost alors qu'il est en standard sous Linux, c'est...

La prochaine/nouvelle version de BCB fournit Boost, VC++ commence à bien aimer Boost (je parle de l'équipe VC++ de chez MS) en plus de tout cela.

Boost promet un avenir très correct au C++.

**camboui** · 26/11/2008, 14h15

Allez, un dernier pour la route

A défaut d'être pratique, puisque vous ne jurez que par du prêt-à-porter (avec raison), l'expérience est didactique, pédagogique.

Ainsi, comme vous reprochiez aux deux précédents smart pointers de n'être soit pas assez "léger", soit pas assez "direct", je viens d'en concocter un troisième qui ne souffre d'aucun de ces deux défauts (mais peut-être d'un autre...)

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
74
75
76
77
78
79
template<class T>
class detached_auto_counter
{
	struct counter_ptr
	{
		long s_cnt;
		T *s_ptr;
		counter_ptr(T *ptr=NULL)
			:s_cnt(0l),s_ptr(ptr) {}
		bool operator<(const counter_ptr & r) const
			{ return unsigned(s_ptr)<unsigned(r.s_ptr); }
	};
	std::set<counter_ptr> f_counterset;
public:
	detached_auto_counter()
		{ Keep(NULL); }
	~detached_auto_counter()
		{ Release(NULL); }
	long Keep(T *ptr)
	{
		counter_ptr cp(ptr);
		std::pair<std::set<counter_ptr>::iterator,bool> p=f_counterset.insert(cp);
		return ++p.first->s_cnt;
	}
	long Release(T *ptr)
	{
		counter_ptr cp(ptr);
		std::set<counter_ptr>::iterator it=f_counterset.find(cp);
		long r=--it->s_cnt;
		if (r==0)
			f_counterset.erase(it);
		return r;
	}
};
 
 
extern detached_auto_counter<void> g_detached_auto_counter;
 
 
template<class T>
class dsmart_ptr
{
	T *f_ptr;
public:
	explicit dsmart_ptr(T *ptr=NULL)
		:f_ptr(ptr)
	{
		std::auto_ptr<T> ap(ptr);
		g_detached_auto_counter.Keep(ptr);
		ap.release();
	}
	dsmart_ptr(const dsmart_ptr<T> & r)
		:f_ptr(r.f_ptr)
	{
		g_detached_auto_counter.Keep(f_ptr);
	}
	dsmart_ptr<T> & operator=(const dsmart_ptr<T> & r)
	{
		T *ptr=r.f_ptr;
		g_detached_auto_counter.Keep(ptr);
		if (g_detached_auto_counter.Release(f_ptr)==0)
			delete f_ptr;
		f_ptr=ptr;
		return *this;
	}
	~dsmart_ptr()
	{
		if (g_detached_auto_counter.Release(f_ptr)==0)
			delete f_ptr;
	}
	operator bool() const
		{ return f_ptr!=0; }
	T & operator*() const
		{ return *f_ptr; }
	T *operator->() const
		{ return f_ptr; }
	T *get() const
		{ return f_ptr; }
};

Le défaut ? Il faut déclarer une variable globale (g_detached_auto_counter). C'est elle qui s'occupe du comptage de référence en lieu et place du smart pointer lui-même. Mais est-ce vraiment un problème ?

Sinon, à vos commentaires

En fait je voudrais que vous m'aidiez à une chose: le multi-threading. Je suis assez novice en la matière. Supposez qu'il ne faille que vérouiller Keep() et Release() (ou Attach() et Detach() dans le précédant smart pointer), comment faire concrètement ? (sous Win32).
Merci.

Après je vous fiche la paix, promis

**Matthieu Brucher** · 26/11/2008, 14h19

Envoyé par camboui

Le défaut ? Il faut déclarer une variable globale (g_detached_auto_counter). C'est elle qui s'occupe du comptage de référence en lieu et place du smart pointer lui-même. Mais est-ce vraiment un problème ?

Sinon, à vous commentaires

En fait je voudrais que vous m'aidiez à une chose: le multi-threading. Je suis assez novice en la matière. Supposez qu'il ne faille que vérouiller Keep() et Realse() (ou Attach() et Detach() dans le précédant smart pointer), comment faire concrètement ? (sous Win32).

Une variable globale en multi-thread -> poubelle.

Sincèrement, prend le pointeur de Boost, tu n'es pas obligé de tout prendre, mais 3-4 en-têtes, c'est pas la mort, surtout que c'est testé robuste, ... tandis que le tien a des défauts de jeunesse qui peuvent être fatals.

[template] un smart pointer parmi d'autres

C++

Vue hybride

Discussions similaires

Partager

Partager