Discussion :

[ravloony]

Bonjour,

Je travaille actuellement sur un projet d'optimisation en C++. Mon code compile, mais ne tourne pas car il rencontre un segfault.
Voilà comment ca se passe. J'ai (entre autres) des clients et des requetes. Chaque client peut avoir plusieurs requetes, et chaque requete provient d'un seul client.
Dans la classe Request, j'ai donc un membre qui est un pointeur vers Client, et dans Client, un vector de pointeurs vers Request. Tout ca evidemment avec des forward-declaration pour que ca compile.
Maintenant le hic: quand j'importe mon fichier de conf, je cree un Client, puis les Request qui vont avec. J'appelle le constructeur de Client avec new, et je mets le pointeur dans un vector. Puis je boucle sur tous les Request, en appelant le constructeur de Request avec new et en ajoutant le pointeur dans un vecteur. Puis j'appelle la methode add_request de client, qui prend un pointeur vers Request, et l'ajoute dans le vector de Client, suivi de la methode add_client de Request, qui prend un pointeur vers Client et met a jour son membre correspondant. Et là, pour verifier, je regarde si l'id du client du dernier Request du vecteur de Request est egal à l'id du Client en cours (en principe j'aurais pu comparer les pointeurs, mais bon). Et j'ai un segfault sur l'accès au Client dans le Request. Valgrind me dit que la memoire n'est pas allouée.
Je mets le code correspondant en dessous, il y a peut-etre une erreur completement bete (c'est meme quasi sur )

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
 
using namespace std;
 
class Request;
 
class Client
{
  private:
    int _id;
    vector<Request*> _requests;
  public:
    Client();
    Client(int id) : _id(id) {}
    ~Client();
    void add_request(Request* r);
    int get_id() {return _id;}
    vector<Request*>& get_requests() {return _requests;}
};
 
void Client::add_request(Request* r)
{
  _requests.push_back(r);
}
 
using namespace std;
 
class Client;
 
class Request
{
  private:
    Client* _client;
  public:
    Request();
    Request(double early, double late, double s_time, double r_time, int skill, int day);
    ~Request();
    Client* get_client() { return _client;}
    void add_client(Client* cli);
};
 
void Request::add_client(Client* cli)
{
  _client = cli;
}
 
 
/******* debut code en question *******/
  vector<Client*> clientVec;
  vector<Request*> requestVec;
  for (unsigned int i=0; i<_coords.size(); i++)
  {
    Client* tmpClient = new Client(i+1);
    clientVec.push_back(tmpClient);
    for (int j=0; j<_num_days;j++) {
 
      /*
      * creation des valeurs de la requete
      */
 
      /*creation de la requete */
      Request* tmpRequest = new Request(early,late,s_time,r_time,skill,j);
 
      /* on ajoute les pointeurs */
      tmpClient->add_request(tmpRequest);
      tmpRequest->add_client(tmpClient);
 
      /* on ajoute la requete au vecteur */
      requestVec.push_back(tmpRequest);
      assert ( (*requestVec.end())->get_client()->get_id()==tmpClient->get_id() ) /*ICI se trouve le segfault */
    }
  }
/******* fin code en question *******/

Voilà, je crois que tout y est. Valgrind me dit que c'est lors de l'appel de get_client() que ca foire.
Honnetement je suis assez mystifié. Je suis persuadé de louper un truc, mais je ne sais pas quoi.
Si vous avez des pistes, je suis donc preneur!
Merci d'avance!

[Arzar]

Bonjour,

Code :

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

(*requestVec.end())->get_client()->get_id()==tmpClient->get_id() )

La STL utilise systématiquement des intervalles semi-ouverts [begin, end), ce qui veut dire que l'itérateur renvoyé par begin() pointe sur le premier élément et que l'itérateur renvoyé par end() pointe un cran plus loin que le dernier élément.

Si tu veux accéder au dernier élément d'un std::vector, il faut utiliser sa fonction membre back().

(Et t'as eu de la chance de te prendre un segfault rapidement, car déréférencer un itérateur end provoque un comportement indéterminé qui fait souvent planter le programme beaucoup plus loin dans l'exécution sans aucune raison apparente -> une horreur à debugger )

[ravloony]

Bonjour,


La STL utilise systématiquement des intervalles semi-ouvert [begin, end), ce qui veut dire que l'itérateur begin pointe sur le premier élément et que l'itérateur end pointe un cran plus loin que le dernier élément.

Si tu veux accéder au dernier élément d'un std::vector, il faut utiliser sa fonction membre back().

(Et t'as eu de la chance de te prendre un segfault rapidement, car déréférencer un itérateur end provoque un comportement indéterminé qui fait souvent planter le programme beaucoup plus loin dans l'exécution sans aucune raison apparente -> une horreur à debugger )

Merci pour ta reponse rapide! Je savais pour les intervalles, mais je confondais effectivement end() avec back(). Donc c'etait bien un truc bete...
Mais le probleme est revenu plus loin!
J'avais carrément fait le tour de tous mes clients en verifiant la meme chose ainsi:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
 
  for (unsigned int i=0; i<clientVec.size();i++)
  {
    int id = clientVec[i]->get_id();
    vector<Request*> reqVec = clientVec[i]->get_requests();
    for (unsigned int j=0; j<reqVec.size();j++)
    {
      Client* cli = reqVec[i]->get_client(); /*ici j'ai exactement la meme erreur*/
      assert(cli->get_id()==id);
    }
  }

Ici c'est pareil, ca devrait marcher, ce qui veut dire que j'ai pas les yeux en face des trous aujourd'hui!

[jblecanard]

Sans rentrer dans le grand débat, si tu mettais un j et pas un i ?

Code :

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

1
2
3
4
5
6
7
8
9
10
11
  for (unsigned int i=0; i<clientVec.size();i++)
  {
    int id = clientVec[i]->get_id();
    vector<Request*> reqVec = clientVec[i]->get_requests();
    for (unsigned int j=0; j<reqVec.size();j++)
    {
      Client* cli = reqVec[i]->get_client(); /*ici j'ai exactement la meme erreur*/
      assert(cli->get_id()==id);
    }
  }

Oui c'est une erreur bête, ça arrive à tout le monde même aux meilleurs

[ravloony]

Sans rentrer dans le grand débat, si tu mettais un j et pas un i ?

Code :

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

1
2
3
4
5
6
7
8
9
10
11
  for (unsigned int i=0; i<clientVec.size();i++)
  {
    int id = clientVec[i]->get_id();
    vector<Request*> reqVec = clientVec[i]->get_requests();
    for (unsigned int j=0; j<reqVec.size();j++)
    {
      Client* cli = reqVec[i]->get_client(); /*ici j'ai exactement la meme erreur*/
      assert(cli->get_id()==id);
    }
  }

Oui c'est une erreur bête, ça arrive à tout le monde même aux meilleurs

Merci!

[koala01]

Salut, et bienvenue sur le forum.

La première question (sans doute idiote) qui me vient à l'esprit suite à une première lecture (pas très attentive, il faut l'avouer) de ton problème est

pourquoi diable utiliser l'allocation dynamique manuelle de la mémoire

Si tu n'envisages pas un certain polymorphisme au niveau de tes objets clients ou request.

De plus, il ne me semble pas opportun de permettre "à n'importe qui" de modifier le client auquel appartient la requête, car, visiblement, elle restera attachée au client... qui l'a reçue en premier, et sera détruite, au plus tôt une fois qu'elle aura été traitée, au plus tard... lorsque le client sera détruit

Enfin, s'il est opportun (je ne dispose pas d'assez d'informations pour me faire une idée sur ce point) de pouvoir récupérer le client au départ d'une requête, j'ai la nette impression que tu devrais le récupérer sous une forme... constante.

Mais, de prime abord, je dirais que l'accès au client depuis la requête ne devrait avoir lieu... que de manière interne à la requête (pour respecter la loi demeter ). Évidemment, je pars du principe général, et il se peut que, dans ton contexte particulier, il en aille effectivement autrement.

Tout cela pour dire que l'amitié pourrait venir bien à point (cf cette entrée de la FAQ)

Enfin, bref... entre tous ces conseils, tu pourrais parfaitement envisager de maintenir dans ta classe Client un tableau de requêtes , au lieu d'un tableau de pointeurs sur requêtes, et que tu pourrais de même envisager de maintenir un tableau de clients (au lieu d'un tableau de pointeurs sur des clients) à l'endroit où tu veux les utiliser.

cela te permettrait d'en arriver à quelque chose comme
Request.h

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
class Client;
class Request
{
        friend class Client;
    public:
        Request(double early, double late, double s_time, double r_time, int
                skill, int day);
        /*...*/
        void execute() const;
    private:
        mutable Client * cl_; // à usage interne uniqument
};

Client.h

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
#include <Request.h>
#include <vector.h>
class Client
{
        /* les deux typedef ne sont là que pour la facilité et pour éviter
         * de devoir vérifier tout le code qui utilise le type Client si,
         * d'aventure, tu décider qu'un std::vector n'est pas la meilleure
         * manière de maintenir une collection de requêtes...
         */
        typedef std::vector<Request> vector;
    public:
        typedef typename vector::const_iterator const_iterator;
        Client(/* paramètres éventuels */);
        void addRequest(Request const & r)
        {
            assert(r.cl_==NULL); // Mechant, tu ne peut pas t'approprier la
                                 // requête d'un autre :D
            r.cl_=this;
            req_.push_back(r);
        }
        const_iterator beginRequest() const{return req_.begin();}
        const_iterator endRequest() const{return req_.end();}
    private:
        vector req_;
};

et au final, tu pourrais gérer tes clients 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
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
#include <Client.h>
#include <list>
int main()
{
    /* je prend ici le parti de donner toutes les requêtes au client avant
     * de l'insérer dans la liste... on pourrait faire autrement :D
     */
    std::list<Client> laliste; // j'aurais pu choisir un std::vector ou
                               // n'importe quelle autre collection utile :D
    while(/* il y a un nouveau client à ajouter */)
    {
         Client c(/* paramètres éventuels */);
         while(/* il y a une requete à ajouter pour ce client */)
         {
             c.addRequest(Request(/* paramètres éventuels */) );
         }
         lalist.push_back(c);
    }
    /* une fois que l'on a tous les clients, on exécute toutes les
     * requêtes de tous les clients
     */
    for(std::list<Client>::const_iterator itClient=laliste.begin();
        itClient!=laliste.end();++it)
    {
        Client const & cl= (*itClient); // j'ai pas envie de me trimbaler un
                                        // (*itClient) à tours de bras :P
        for(Client::const_iterator it=cl.beginRequest();it!=cl.endRequest();
            ++it)
            (*it).execute();
    }
 
}

Tu auras, il est vrai sous cette forme, un grand nombre de copie de requêtes et de clients, mais si les performances restent suffisantes, il ne faut pas forcément aller plus loin, et tu t'évitera toute la corvée de la gestion dynamique de la mémoire

[ravloony]

Whouah j'apprends plein de choses!

En fait je n'ai pas donné le grand schema. Il s'agit d'un projet de recherche. J'ai donc , une fois importés, ma collection de clients qui ne sont pas importants en soi, mais utiles pour l'objectifs de mon projet, qui est de faire en sorte que les requetes d'un client ne soient traitées que par une seule personne (infirmiere) si possible. Ensuite j'ai mes requetes, sur lesquelles j'applique plusieurs algorithmes a la suite, de maniere a trouver une combinaison de tournees qui minimise l'objectif precedemment cité, et ensuite le cout (chaque client a des coordonnees, ici le cout c'est la distance parcourue par la tournee)
Je cree donc un ensemble de tournees, une par infirmiere par jour, et j'insere les requetes dedans suivant une certaine heuristique. Si je m'en tenais là, je ne m'embeterais pas avec la gestion de la memoire, mais je dois ensuite appliquer une autre metaheuristique a partir des tournees de depart, qui echangera les requetes inter et intra tournées. Le cout des copies de requetes, clients etc devient vite prohibitif dans ce genre de cas, et c'est pour cela que je pense qu'ici, la gestion manuelle de la memoire devient utile (mais quand meme lourde...), puisqu'au lieu de copier/effacer de multiples fois les objets, je peux le faire avec les pointeurs. Après tout, le but de mon projet est d'etre le plus efficace possible!
En tout cas merci pour ces conseils, je vais approfondir mes connaissances sur l'amitié en particulier, cela pourrait bien me servir dans ce cas effectivement.
Pour ce qui est de la loi Demeter, je suis d'accord en principe. Je vais regarder s'il y a pas une meilleure manière de faire à ce niveau là...