Retourner un pointeur sur un objet statique interne à une fonction ?

Bonjour à tous !

Je viens de tomber sur ce bout de code dans une librairie tierce que j'utilise :

Code:

---

1 2 3 4 5 6 7 8 9 10 11

template<class T> char const * ConvertToString( T const & t ) { std::stringstream str; static std::string strOut; str << t; strOut = str.str(); return strOut.c_str(); }

---

A votre avis, est-ce que ce genre de code (renvoi d'un pointeur sur un objet statique) est correct, robuste et sans vice caché ?

Y a pas mal de bibliothèques qui font ça. La seule chose c'est que chaque nouvel appel invalide l'adresse précédemment renvoyée. Donc, tu ne peux pas la garder pour un usage ultérieur.
Donc :
-> correct : il me semble que oui
-> robuste : non
-> vices cachés : la personne qui arrive après toi et qui n'a pas la curiosité de regarder l'implémentation et pense pouvoir garder le pointeur d'une fois sur l'autre :aie:

Quel est l'intérêt au final? Là j'avoue que je vois mal comment quelqu'un a pu se dire :
"tiens et si je renvoyais un pointeur sur une donnée statique interne à une fonction?"

Merci !

Citation:

---

Envoyé par 3DArchi

-> correct : il me semble que oui
-> robuste : non
-> vices cachés : la personne qui arrive après toi et qui n'a pas la curiosité de regarder l'implémentation et pense pouvoir garder le pointeur d'une fois sur l'autre

---

Ok, j'en étais arrivé à peu près aux mêmes conclusions, merci pour la confirmation. :ccool:

Citation:

---

Quel est l'intérêt au final? Là j'avoue que je vois mal comment quelqu'un a pu se dire :
"tiens et si je renvoyais un pointeur sur une donnée statique interne à une fonction?"

---

Alors, j'ai inspecté un peu plus le code de la bibliothèque. En fait, je suspecte que son auteur pensait avoir trouvé une superbe feinte pour économiser des copies, mais qu'il a oublié dans son analyse la RVO/NRVO. :mrgreen:

Il n'utilise la fonction ConvertToString que de deux manières :

Code:

---

1 2 3 4 5 6 7 8 9

// 1ere manière std::string s = ConvertToString(data);   // 2eme manière Truc truc( ConvertToString( data ) );   // avec Truc::Truc(const std::string& value):value_(value){}

---

Donc il n'a pas de problème de durée de vie du pointeur car il copie systématiquement le retour de ConvertToString dans une std::string. Mais au final, il fait une copie de plus que le code de la faq :

Code:

---

1 2 3 4 5 6 7 8 9 10 11

template<typename T> std::string to_string( const T & Value ) { // utiliser un flux de sortie pour créer la chaîne std::ostringstream oss; // écrire la valeur dans le flux oss << Value; // renvoyer une string return oss.str(); // RVO possible ici. }

---

qui exploite la RVO. :mrgreen:

(Enfin, si je ne me suis pas planté en testant. :aie: Pour référence, je laisse ci-dessous mon code de test)

Code:

---

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

#include <iostream> #include <string>   struct A { A(){std::cout << "default ctor" << std::endl;} A(A* a){std::cout << "ctor A*" << std::endl;} A(const A& a){std::cout << "copy ctor" << std::endl;} A& operator=(const A& x){std::cout << "copy op=" << std::endl; return *this;} ~A(){std::cout << "dtor" << std::endl;} };   struct B { B(const A& a):a_(a){} B(A* a):a_(a){}   A a_; };   // fake stringstream struct stringstream { A str() { A a; // do something... return a; } };   template<class T> A* ConvertToAPtr( T const & t ) { static A strOut; stringstream str; // str << t; strOut = str.str(); return &strOut; }   template<class T> A ConvertToA( T const & t ) { stringstream str; //str << t return str.str(); }     int main() { std::cout << "ConvertToAPtr(T); (Warm up static data)\n"; ConvertToAPtr(16);   std::cout << "\nA a(ConvertToAPtr(T)\n"; A a1(ConvertToAPtr(16));   printf("\nB b(ConvertToAPtr(T)\n"); B b1(ConvertToAPtr(15));   printf("\nA a(ConvertToA(T)); (Note : NRVO here)\n"); A a2(ConvertToA(16));   std::cout << "\nB b(ConvertToA(T));\n"; B b2(ConvertToA(15));   printf("\nEND\n");   return 0; }

---

Citation:

---

Envoyé par Arzar

Alors, j'ai inspecté un peu plus le code de la bibliothèque. En fait, je suspecte que son auteur pensait avoir trouvé une superbe feinte pour économiser des copies, mais qu'il a oublié dans son analyse la RVO/NRVO. :mrgreen:

---

C'est une technique que j'ai vu beaucoup employée dans du code C embarqué. Quelque chose du genre :

Code:

---

1 2 3 4 5 6 7

char const *do_something(/*...*/) { static char result[TAILLE]; /*...*/ return result; }

---

La zone mémoire est de taille fixe, il n'y a pas d'échec d'allocation possible et on connait l'occupation mémoire dès la compilation (et si c'est une variable globale, je crois qu'on peut même choisir son emplacement).
Ca peut donc être aussi quelqu'un qui a repris ses habitudes C en les appliquant à un code C++ où cela devient caduc voire, comme tu dis, pénalisant (car dans ton exemple il y aura allocation et recopie et on perd le bénéfice de la (N)RVO) .

En plus, ce n'est évidemment pas réentrant, et plus grave, avec une implémentation aussi "naïve", ce n'est pas thread-safe (il faudrait utiliser explicitement ou implicitement du Thread-Local Storage).

Le renvoi d'un pointeur sur un objet statique renvoie bien la même chose d'une fois sur l'autre, mais dans le code présenté, ce qui est renvoyé n'est pas un "pointeur sur un objet statique interne à une fonction", mais le résultat d'une requête sur un objet (qui en l'occurrence est statique à une fonction, mais le problème ne vient pas de là).

Le résultat de cette requête peut être invalidé, comme pour bien des objets; c'est un peu comme si on renvoyait un vector<T>::begin(): cela n'est ni bon ni mauvais en soi, il faut simplement tenir compte de la durée de vie du pointeur en question. En l'occurrence, la valeur de retour deviendra fausse dès l'appel suivant, non pas parce que l'objet en question était local à une fonction, non pas parce que la procédure n'est pas thread-safe, mais tout simplement parce que le contenu de l'objet change, et donc que la résultat de la requête précédente devient invalide. En gros, ce code est exactement aussi faux que:

Code:

---

1 2 3 4 5

vector<int> a; a.push_back(5); vector<int>::const_iterator pa = a.begin(); a.clear(); cout << *pa;

---

Le code présenté est mauvais sans aucun doute, mais d'abord pour des raisons de parallélisme, et non pas pour des raisons de durée de vie du résultat: il est difficile de croire qu'un utilisateur déduirait de la signature de la fonction ("char const *ConvertToString( T const & t )") que le pointeur retourné a une durée de vie dépassant la séquence d'appel.

Il me semble donc qu'il y a dans les commentaires quelques confusions quant aux causes du problème: la non-réentrance est effectivement due à l'utilisation d'une variable statique interne à la fonction, ce qui correspond au titre de la discussion. Par contre, la non-conservation de la valeur n'est pas supposable d'après la signature, n'est pas due au fait que l'objet est statique et interne à la fonction, et est donc sans rapport avec le titre de la discussion.