Discussion :

[kashikai]

Bonjour,

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

J'ai la structure suivante :

Code :

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

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 :

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

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 :

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

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

mais meme résultat.

merci d'avance pour votre aide

[Nogane]

Bonjour,
Voila le problème:

Code :

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

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 :

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

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 :

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

dbus_order pData;

[kashikai]

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

[koala01]

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

Bonjour,

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

J'ai la structure suivante :

Code :

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

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 :

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

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 :

Code :

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

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 :

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

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

Primo : pourquoi utiliser un pointeur et l'allocation dynamique
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 pourquoi ne pas directement insérer les éléments... là où tu en aura besoin :

Code :

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

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 ), 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 :

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

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) 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 :

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

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 :

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

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 :

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

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);
}