memcpy et stl incompatibles ?

**Tanyia** · 18/12/2007, 02h37

Bonjour a tous,
Je viens vous embeter avec une petite colle que je n'ai toujours pas resolue. Il y a sans doute une erreur grosse comme une maison, mais impossible de la trouver...
Il ne s'agit pas d'utiliser memcpy pour recopier des structures stl, ou des choses compliquees. Le probleme se pose lors de l'appel d'une methode stl sur un tableau recopie en utilisant la methode memcpy.

Voici les deux fichiers composant mon projet :

Photon.h : une classe simple contenant 3 flottants

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
 
#ifndef PHOTON_H
#define PHOTON_H
 
class Photon
{
public :
	float position[3];
	float direction[3];
	float power[3];
	float &x;
	float &y; 
	float &z;
 
	Photon() : x(position[0]), y(position[1]), z(position[2])
	{
		position[0] = 0.0f;
		position[1] = 0.0f;
		position[2] = 0.0f;
	}
 
	Photon(Photon& p) : x(position[0]), y(position[1]), z(position[2])
	{
		position[0] = p.x;
		position[1] = p.y;
		position[2] = p.z;
	}
 
	Photon& operator=(const Photon&	p)
	{
		position[0] = p.x;
		position[1] = p.y;
		position[2] = p.z;
		return *this;
	}
 
};
 
#endif // PHOTON_H

main.c : le code qui me pose une colle

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
#include <iostream>
#include <algorithm>
#include <functional>
#include "Photon.h"

using namespace std;

#define ARRAY_SIZE 10

template <class T> class CompareAlongDimension : public std::binary_function<T*, T*, bool>
{
public:
	int dimension;
	CompareAlongDimension( int d ) : dimension(d) {}
     bool operator()(T const & A, T const & B) { return A.position[dimension] < B.position[dimension]; }
} ;

void showPhotonArray(Photon* array, int axis){
	for(unsigned int i = 0; i < ARRAY_SIZE; i++)
	{
		cout << array[i].position[axis] << " ";
	}
	cout << endl;
}


int main(int argc, char** argv)
{

	Photon* array = new Photon[ARRAY_SIZE];
	Photon* copy = new Photon[ARRAY_SIZE];

	for(unsigned int i=0;  i<ARRAY_SIZE; i++)
	{
		array[i].x = (float)rand()/(RAND_MAX+1);
		array[i].y = (float)rand()/(RAND_MAX+1);
		array[i].z = (float)rand()/(RAND_MAX+1);
	}

	//memcpy(copy, array, sizeof(Photon)*ARRAY_SIZE);
	for(unsigned int i=0;  i<ARRAY_SIZE; i++)
	{
		copy[i] = array[i];
	}

	cout << "Tirage Aleatoire original : " << endl;
	showPhotonArray(array, 0);
	sort(array, array+ARRAY_SIZE, CompareAlongDimension<Photon>(0) );
	cout << "Tirage Aleatoire trie : " << endl;
	showPhotonArray(array, 0);
	cout << endl;


	cout << "Copie du tirage originale : " << endl;
	showPhotonArray(copy, 0);
	sort(copy, copy+ARRAY_SIZE, CompareAlongDimension<Photon>(0) );
	cout << "Copie du tirage triee: " << endl;
	showPhotonArray(copy, 0);

	return 0;
}

L'application cree 10 Photons aleatoires, puis les reorganise en fonction de leur coordonnee suivant l'axe X. J'utilise pour cela stl::sort. Hors, si je copie le tirage aleatoire original, puis cherche a le trier de la meme facon, des choses magiques se passent...

Si j'utilise une boucle for pour faire la recopie du tableau, j'obtiens la sortie logique :

Code :

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

1
2
3
4
5
6
7
8
9
Tirage Aleatoire original :
0.00125122 0.808716 0.350281 0.746582 0.71048 0.0149841 0.147308 0.445679 0.00891113 0.571167
Tirage Aleatoire trie :
0.00125122 0.00891113 0.0149841 0.147308 0.350281 0.445679 0.571167 0.71048 0.746582 0.808716

Copie du tirage originale :
0.00125122 0.808716 0.350281 0.746582 0.71048 0.0149841 0.147308 0.445679 0.00891113 0.571167
Copie du tirage triee:
0.00125122 0.00891113 0.0149841 0.147308 0.350281 0.445679 0.571167 0.71048 0.746582 0.808716

Mais si j'utilise memcpy pour faire la recopie... le tri est fait n'importe comment :

Code :

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

1
2
3
4
5
6
7
8
9
Tirage Aleatoire original :
0.00125122 0.808716 0.350281 0.746582 0.71048 0.0149841 0.147308 0.445679 0.00891113 0.571167
Tirage Aleatoire trie :
0.00125122 0.00891113 0.0149841 0.147308 0.350281 0.445679 0.571167 0.71048 0.746582 0.808716

Copie du tirage originale :
0.00125122 0.808716 0.350281 0.746582 0.71048 0.0149841 0.147308 0.445679 0.00891113 0.571167
Copie du tirage triee:
0.00125122 0.0149841 0.00891113 0.746582 0.147308 0.350281 0.445679 0.571167 0.808716 0.746582

Je n'arrive pas du tout a voir de quoi cela pourrait venir... Pouvez vous m'aider ?
Merci !

Tan'

**Pacorabanix** · 18/12/2007, 03h03

memcpy est une fonction C, il y a peut-être un souci avec des tableaux d'objets.

Puisqu'on est dans la stl je te conseillerai d'utiliser un std::vector<Photon> pour tes tableaux. Il y a des fonctions intégrées pour faire des copies proprement.

D'ailleurs : memcpy n'appelle certainement pas le constructeur des objets, alors que ta boucle for oui (elle utilise ton operator = ). Le problème vient probablement de là.

De plus il n'y a quelquechose que je ne comprend pas bien : pour quoi tes variables membres x, y et z sont de type float& et pas simplement float ?

**Pacorabanix** · 18/12/2007, 03h19

Code :

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

1
2
3
4
5
6
Photon() : x(position[0]), y(position[1]), z(position[2])
	{
		position[0] = 0.0f;
		position[1] = 0.0f;
		position[2] = 0.0f;
	}

On m'a dit que les intialiseurs étaient exécuté avant le code du constructeur (ce qui semble logique). Donc il me semble que ton tableau position n'est pas initialisé lorsque tu attribues les valeurs à x, y et z :$ .

**Laurent Gomila** · 18/12/2007, 04h24

De plus il n'y a quelquechose que je ne comprend pas bien : pour quoi tes variables membres x, y et z sont de type float& et pas simplement float ?

On m'a dit que les intialiseurs étaient exécuté avant le code du constructeur (ce qui semble logique). Donc il me semble que ton tableau position n'est pas initialisé lorsque tu attribues les valeurs à x, y et z :$ .

C'est juste un raccourci syntaxique, pour les gens qui préfèrent écrire p.x plutôt que p.position[0]. Donc x, y et z sont des références simplement car elles ne servent que de raccourci pour accéder à position[0], position[1] et position[2]. La liste d'initialisation ne sert pas à initialiser leur valeur, mais à les faire pointer vers position[], donc peu importe que position[] soit initialisé ou non. D'ailleurs on ne peut initialiser une membre référence que dans la liste d'initialisation.

Bref pas de souci de ce côté, le problème vient juste du fait que memcpy n'appelle pas le constructeur et que x, y et z ne sont jamais initialisés et pointent donc vers n'importe quoi. Utilise lputôt std::copy, ou mieux, std::vector pour tes tableaux.

**yan** · 18/12/2007, 09h45

Juste en complément :

memcopy => copy bits à bits une zone mémoire vers une autre. Ainsi, les pointeurs et references pointerons exactement la même chose dans la zone copié que dans la zone d'originine => grosse erreur mémoire
std::copy => copy avec l'operateur = un ensemble d'objet vers un autre

**loufoque** · 18/12/2007, 11h45

std::copy fait la copie avec le constructeur de copie, pas avec l'opérateur d'affectation...

**yan** · 18/12/2007, 11h48

Envoyé par loufoque

std::copy fait la copie avec le constructeur de copie, pas avec l'opérateur d'affectation...

?????
Comment fait tu avec le constructeur de copie????

STL de visual

Code C++ :

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

1
2
3
4
5
6
7
8
9
10
template<class _InIt, class _OutIt, class _InOutItCat>
inline
	_OutIt __CLRCALL_OR_CDECL _Copy_opt(_InIt _First, _InIt _Last, _OutIt _Dest,
		_InOutItCat, _Nonscalar_ptr_iterator_tag, _Range_checked_iterator_tag)
	{	// copy [_First, _Last) to [_Dest, ...), arbitrary iterators
	_DEBUG_RANGE(_First, _Last);
	for (; _First != _Last; ++_Dest, ++_First)
		*_Dest = *_First;
	return (_Dest);
	}

**loufoque** · 18/12/2007, 11h59

Ouais en fait t'as raison, mais c'est plus vraiment de la copie au sens C++ du terme alors.

**yan** · 18/12/2007, 14h03

Envoyé par loufoque

Ouais en fait t'as raison, mais c'est plus vraiment de la copie au sens C++ du terme alors.

en quoi faire un egale n'est pas une copie au sens C++ du terme?
Je dit pas que t'as tord, mais je ne comprend ce que tu veut dire

**Pacorabanix** · 18/12/2007, 15h32

De plus il n'y a quelquechose que je ne comprend pas bien : pour quoi tes variables membres x, y et z sont de type float& et pas simplement float ?

On m'a dit que les intialiseurs étaient exécuté avant le code du constructeur (ce qui semble logique). Donc il me semble que ton tableau position n'est pas initialisé lorsque tu attribues les valeurs à x, y et z :$ .

C'est juste un raccourci syntaxique, pour les gens qui préfèrent écrire p.x plutôt que p.position[0]. Donc x, y et z sont des références simplement car elles ne servent que de raccourci pour accéder à position[0], position[1] et position[2]. La liste d'initialisation ne sert pas à initialiser leur valeur, mais à les faire pointer vers position[], donc peu importe que position[] soit initialisé ou non. D'ailleurs on ne peut initialiser une membre référence que dans la liste d'initialisation.

Bref pas de souci de ce côté, le problème vient juste du fait que memcpy n'appelle pas le constructeur et que x, y et z ne sont jamais initialisés et pointent donc vers n'importe quoi. Utilise lputôt std::copy, ou mieux, std::vector pour tes tableaux.

ok merci pour ces précisions, je comprend maintenant bien le sens du code

.

**ac_wingless** · 18/12/2007, 16h40

Je ne pense pas que l'utilisation de memcpy, bien qu'évidemment fausse sur une classe ayant des références internes, soit responsable de l'erreur reportée. En effet, memcpy ne fait que détruire les références x y et z (en les faisant pointer sur les adresses de l'autre tableau), mais x y ou z ne sont pas utilisés dans la fonction sort.
J'ai par curiosité copié/collé le code, et je n'ai effectivement pas l'erreur indiquée. Le message erroné correspond donc sans doute à une autre itération de développement, peut-être avec sort utilisant "x" dans la foncton de comparaison?

Ceci dit, il faut absolument éviter les références internes, car de nombreux conteneurs de librairies ne garantissent pas de ne pas déplacer en mémoire leurs éléments, par quelque moyen qu'ils jugent bon. Il est donc illusoire de se protéger par le constructeur de copie, car rien n'interdit à un utilisateur de votre classe d'utiliser indifféremment toute technique lors d'une sémantique de copie, en particulier memcpy, mais aussi indifféremment l'opérateur d'affectation ou la copie, qui ne sont pas exactement identiques dans votre code du point de vue de leur effet sur les valeurs xyz.
Une remarque au passage: "Objet ( Objet& )" n'est pas un constructeur de copie sous la forme typiquement attendue, et sera rejeté à la compilation lors de la recherche d'un constructeur automatique de copie. Il faut utiliser la signature "Objet ( const Objet& )".
Pour en revenir à l'utilisation de références internes, votre code est totalement exposé à:

Code :

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

1
2
3
4
5
6
7
 
Photon  P1,P2;
 
    P1.x = 1;
    memcpy ( &P2, &P1, sizeof(Photon) );
    P2.x = 2;
    cout << "P1.x = " << P1.x << endl;

qui affichera "P1.x = 2". Difficile de documenter facilement ce comportement pour les futurs utilisateurs de votre classe, en particulier ceux ayant conçu la STL en 1994, mais aussi ceux qui voudraient dériver PhotonBleu de Photon et utiliser memcpy pour leur propres traitements.

**Médinoc** · 18/12/2007, 17h18

J'ai bien peur que si, ces références membres soient utilisées dans la fonction sort() : sort() utilise sans doute l'opérateur d'affectation, qui utilise x, y et z --> Problème.

**ac_wingless** · 18/12/2007, 18h01

Oui, c'est tout à fait vrai. Ce qui est donc curieux, c'est que l'algorithme arrive apparemment quand même à trier correctement sur l'exemple donné.
Je pense qu'il se passe les choses suivantes:
- l'affichage de l'original non trié marche normalement.
- le tri de l'original marche normalement, et la copie pointe donc maintenant vers un tableau trié du point de vue des xyz de la copie, mais pas des tableaux position de la copie
- l'affichage de la copie affiche bien un tableau non trié, car l'affichage utilise le tableau position (si l'affichage utilisait les xyz, on aurait le gag dès ce moment).
- le tri de la copie est perturbé par des comparaisons vraies et des affectations fausses.
La séquence des comparaisons est au départ identique dans les 2 opérations de tri lors de la première passe, car avant qu'une affectation soit faite, les valeurs comparées sont bien correctes. Si le tri se fait en une seule passe, tout se passe bien, aucune affectation erronée ne peut troubler le résultat (sans doute le cas de l'exemple). Si par contre il faut plusieurs passes, toute valeur ayant subi une affectation verra sa prochaine comparaison incohérente avec sa valeur affectée, avec un gag au bout. Et effectivement, j'ai augmenté la taille du tableau, et peu à peu on voit apparaitre des surprises, avec quelques valeurs mal placées pour 50 éléments et un gros désordre pour 100+.

Bug intéressant!

**Tanyia** · 18/12/2007, 19h14

Coucou a nouveau par ici !
Alors la je suis epate, j'ai du passer a cote de cette histoire de reference une bonne 20aine de fois sans la voir... Je pensais que je pouvais me permettre d'utiliser memcpy puisque je n'avais pas de pointeurs... et j'ai oublie les references !
Felicitations pour l'analyse en tous cas, et la description de ce qui se passe est d'une clarte admirable

Pour une fois que je cree un bug interessant

Je m'en vais de ce pas changer quelques trucs donc...

Sinon, une remarque sur l'utilisation de la stl : j'utilise uniquement des tableaux C au lieu de conteneurs, car je travaille avec plusieurs millions de photons. La stl devient alors trop lourde pour gerer tout ca, en comparaison avec une implementation bien tunee a la main (mais evidemment cela a un cout de developpement). Un exemple, pour la fonction stl::nth_element, une implementation en C valable uniquement pour un tableau de type Photon[] permet de gagner entre 25 et 30% de temps de calcul (l'algorithme de selection peut etre code sans un seul appel recursif, cela aide sans doute).

Merci pour vos remarques donc !

**JolyLoic** · 18/12/2007, 22h31

Envoyé par ac_wingless

Je ne pense pas que l'utilisation de memcpy, bien qu'évidemment fausse sur une classe ayant des références internes

Formellement, l'utilisation de memcpy n'est possible qu'avec des POD. Et il y a d'autres points que les références qui font que cette classe n'est pas un POD (constructeur de coie, par exemple).

Envoyé par ac_wingless

Une remarque au passage: "Objet ( Objet& )" n'est pas un constructeur de copie sous la forme typiquement attendue, et sera rejeté à la compilation lors de la recherche d'un constructeur automatique de copie. Il faut utiliser la signature "Objet ( const Objet& )".

Faux. Bien qu'inhabituelle, cette écriture défini un constructeur de recopie :

Envoyé par Le standard §12.8.2

A nontemplate constructor for class X is a copy constructor if its first parameter is of type X&, const X&, volatile X& or const volatile X&, and either there are no other parameters or else all other parameters have default arguments

**yan** · 18/12/2007, 23h45

Que veut dire POD??

**Niark13** · 19/12/2007, 08h38

Plain Old Data.

**JolyLoic** · 19/12/2007, 22h13

Comme l'a dit Niark, Plain Old Data.

En gros, une structure telle qu'on pourrait la définir en C.

En plus précis, (j'en oublie peut-être, j'ai toujours du mal avec ça) :
- Pas de constructeurs, d'opérateurs= ou de destructeurs définis par l'utilisateur,
- Pas de donnée non statiques protégées ou privées,
- Pas de classe de base non vides,
- Pas de fonction virtuelles,
- Des données membre qui sont elles mêmes des POD

memcpy et stl incompatibles ?

C++

Discussions similaires

Partager

Partager