Problème segfault copie de map

Bonjour,

je rencontre un problème de segmentation fault que je ne comprend pas.

J'ai la structure suivante :

Code:

---

1 2 3 4 5 6

typedef struct _dbus_order{ int nUserId; int nChannelId; std::map < int , int > mapArticlesId; }dbus_order;

---

Et je m'en sert comme ca :

Code:

---

1 2 3 4 5 6 7

dbus_order *pData = (dbus_order *)malloc(sizeof(pData)); std::map < int , int > mapTmp; for(unsigned int i=0 ; i < p_basket->getList().size() ; i++){ mapTmp.insert(std::pair<int , int>(p_basket->getList()[i].id , p_basket->getList()[i].quantity)); } pData->mapArticlesId = mapTmp;

---

La suite n'a pas d'importance.
et la copie (derniere ligne) me crée une erreur de segmentation.
J'ai essayé aussi :

Code:

---

1 2	pData->mapArticlesId.insert(mapTmp.begin(), mapTmp.end());

---

mais meme résultat.

merci d'avance pour votre aide

Bonjour,
Voila le problème:

Code:

---

dbus_order *pData = (dbus_order *)malloc(sizeof(pData));

---

Cette ligne alloue bien a zone mémoire pour le dbus_order, mais n'appel pas le constructeur de pData. Ce qui fait que pData n'est pas construit.
C'est pourquoi en c++ on utilise new au lieu de malloc, qui a le double emploie de faire une allocation ET une construction.
Essayez avec:

Code:

---

dbus_order *pData = new dbus_order();

---

Ce qui vous évite, en plus de faire un cast 'c-style' (ou 'a la C', comme disent les anglophones)

Remarquez que si vous n'avez pas de bonne raison de passer par un pointeur, il est encore mieux de s'en passer:

Code:

---

dbus_order pData;

---

Merci mille fois !

En effet j'ai fait pas mal de C mais je débute en C++.


Dans mon esprit l'opérateur new n'était applicable qu'a un objet et non a une simple structure.

merci encore :ccool:

Salut, et bienvenue sur le forum.

Je ne voudrait pas que cette remarque passe pour agresseive, mais... y a de la place pour le progès :aie:

Citation:

---

Envoyé par kashikai

Bonjour,

je rencontre un problème de segmentation fault que je ne comprend pas.

J'ai la structure suivante :

Code:

---

1 2 3 4 5 6

typedef struct _dbus_order{ int nUserId; int nChannelId; std::map < int , int > mapArticlesId; }dbus_order;

---

---

Contrairement à C, la définition d'une classe, d'une structure ou d'une union entraine automatiquement la définition du type correspondant...

Tu peux donc parfaitement te contenter d'un

Code:

---

1 2 3 4 5 6

struct dbus_order { int nUserId; int nChannelId; std::map < int , int > mapArticlesId; };

---

Comme std::map est une classe, il ne sera pas possible de travailler en C avec une telle structure (du moins en l'état), et tu ne dois donc pas t'inquiéter d'assurer une quelconque compatibilité avec ce langage ;)
Et je m'en sert comme ca :

Citation:

---

Code:

---

1 2 3 4 5 6 7

dbus_order *pData = (dbus_order *)malloc(sizeof(pData)); std::map < int , int > mapTmp; for(unsigned int i=0 ; i < p_basket->getList().size() ; i++){ mapTmp.insert(std::pair<int , int>(p_basket->getList()[i].id , p_basket->getList()[i].quantity)); } pData->mapArticlesId = mapTmp;

---

La suite n'a pas d'importance.
et la copie (derniere ligne) me crée une erreur de segmentation.
J'ai essayé aussi :

Code:

---

1 2	pData->mapArticlesId.insert(mapTmp.begin(), mapTmp.end());

---

---

Primo : pourquoi utiliser un pointeur et l'allocation dynamique :question:
Je ne dis pas que ce n'est pas intéressant dans ton cas, mais, d'expérience je peux t'assurer que, s'il n'est pas question que ta variable survive à la portée dans laquelle elle est déclarée, il est beaucoup plus facile de se passer de l'allocation dynamique.

De plus, pourquoi diable t'enquiquiner à remplir une std::map temporaire pour en copier le contenu dans la structure ou pour afecter cette map temporaire à celle de la structure :question: pourquoi ne pas directement insérer les éléments... là où tu en aura besoin :question::

Code:

---

1 2 3 4 5

dbus_order data; for(unsigned int i=0 ; i < p_basket->getList().size() ; i++){ data.mapArticleId.insert( std::pair<int , int>( p_basket->getList()[i].id , p_basket->getList()[i].quantity)); }

---

serait beaucoup plus facile.

Secundo: Si tu as de bonnes raisons d'avoir recours aux pointeurs et à l'allocation dynamique (ce qui peut malgré tout arriver :D), il vaut mieux abandonner les fonction malloc, calloc et autres *alloc issues du C au profit de new.

En effet, les fonctions C *alloc ne font que... réserver un espace mémoire de la taille indiquée alors que new:

1. a une syntaxe beaucoup plus facile : dbus_order * ptr = new dbus_order;
2. fait quelques trucs en plus, comme appeler le constructeur de ta structure et affecter effectivement chaque partie de la mémoire au membre correspondant

En utilisant un pointeur, nous aurions donc:

Code:

---

1 2 3 4 5

dbus_order * pdata = new dbus_order; for(unsigned int i=0 ; i < p_basket->getList().size() ; i++){ data->mapArticleId.insert( std::pair<int , int>( p_basket->getList()[i].id , p_basket->getList()[i].quantity)); }

---

Tertio : A moins qu'il n'y ait de sérieuses raisons de tout laisser visible, il serait sans doute préférable d'encapsuler un peu tous ces détails d'implémentation...

C++ est un langage (entre autre:D) orienté objet: autant en profiter:

Il serait donc peut être intéressant d'envisager de transformer ta structure en classe avec tout ce que cela implique sous une forme proche de

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14

class dbus_order{ public: dbus_order(int user, int chan):nUserId(user),nChannelId(chan){} int userId() const{return nUserId;} int channelId() const{return nChannelId;} void addArticles(int key, int value) { mapArticlesId.insert(std::make_pair(key,value); } private: int nUserId; int nChannelId; std::map < int , int > mapArticlesId; };

---

qui serait remplie sous la forme de

Code:

---

1 2 3 4 5

dbus_order data(idUser, idChannel); for(unsigned int i=0 ; i < p_basket->getList().size() ; i++){ data.addArticles(p_basket->getList()[i].id , p_basket->getList()[i].quantity)); }

---

Cela te permettra entre autres, pour autant que tu garde le même prototype pour addArticles, de décider "n'importe quand" de changer la manière dont tes articles sont gardés en mémoire ;)

[EDIT]Enfin, ce n'est qu'un détail, mais je ne suis absolument persuadé de l'opportunité d'avoir défini une fonction getListe dans la classe don p_basket est un digne représentant...

Je ne peux, bien sur, que faire des suppositions, mais il semblerait que getList renvoie... un std::vector contenant une structure quelconque.

Si c'est le cas, il serait des plus intéressant de remplacer cette fonction par:

1. la définition (dans la classe) d'un alias de type proche de std::vector<la_structure>::const_iterator const_iterator;
2. la définition d'une fonction begin() renvoyant l'itérateur sur la_liste.begin();
3. la définition d'une fonction end() renvoyant l'itérateur sur la_liste.end();

(le tout en accessibilité publique).

Le code pourrait alors être modifié sous la forme de
d

Code:

---

1 2 3 4

bus_order data(idUser, idChannel); for(LaClass::const_iterator it=p_basket->begn(), ; it != p_basket->end() ; ++it){ data.addArticles( (*it).id, (*it).quantity); }

---