Discussion :

[Mc geek]

Bonjour tout le monde !

Pour m'entraîner à la programmation en C, j'ai voulu recoder une structure de type pile avec une bonne partie des fonctions associées. Mais il se trouve que j'ai un problème de modification de références que je ne m'explique pas : dans la fonction enleverElement je n'arrive pas à enlever un élément du haut de la pile. C'est comme si la ligne n'était pas interprétée (après compilation) par la machine .

Je vous donne mon code :

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
73
74
75
76
 
#include <stdlib.h>
#include <stdio.h>
 
#include "main.h"
 
int main(void) {
  int compte = 0;
  int valeur = 0;
 
  Pile *pile = creerPile(1);
 
  Pile *element2 = (Pile*) malloc(sizeof(Pile));
  Pile *element3 = (Pile*) malloc(sizeof(Pile));
 
  element2->valeur = 2;
  element2->element_precedent = NULL;
 
  element3->valeur = 3;
  element3->element_precedent = NULL;
 
  printf("Adresse de la pile : %d\n", pile);
  printf("Adresse de l'element 2 : %d\n", element2);
  printf("Adresse de l'element 3 : %d\n\n\n", element3);
 
  ajouterNouvelElement(pile, element2);
  pile = element2;
 
  printf("Adresse de l'ancienne pile (element 1) : %d\n", pile->element_precedent);
  printf("Adresse de la nouvelle pile (element 2) : %d\n\n\n", pile);
 
  ajouterNouvelElement(pile, element3);
  pile = element3;
 
  printf("Adresse de l'ancienne pile (element 2) : %d\n", pile->element_precedent);
  printf("Adresse de la nouvelle pile (element 3) : %d\n\n\n", pile);
 
  valeur = enleverElement(pile);
  printf("Adresse de l'ancien element empile : %d\n", pile->element_precedent);
  printf("Adresse du haut de la pile : %d\n", pile);
 
  free(pile);
 
  return 0;
}
 
 
 
Pile *creerPile(int valeur) {
  Pile *nouveauPointeur = (Pile*) malloc(sizeof(Pile));
  // Pile *nouveauPointeur; Fait planter le programme, mais pourquoi ?
  nouveauPointeur->valeur = valeur;
  nouveauPointeur->element_precedent = NULL;
  return nouveauPointeur;
}
 
void ajouterNouvelElement(Pile *pile, Pile *nouvelElement) {
  nouvelElement->element_precedent = pile;
}
 
int enleverElement(Pile *pile) {
  if (pile == NULL) {
    exit(EXIT_FAILURE);
  }
 
  int nombreDepile = 0;
 
  if (pile != NULL && pile->element_precedent != NULL) {
    nombreDepile = pile->valeur;
 
    pile = pile->element_precedent;
    printf("%d\n", pile);
  }
 
  return nombreDepile;
}

Je pense que mon problème se situe au niveau de cette ligne :

Code :

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

pile = pile->element_precedent;

mais je ne comprends pas sa source car la logique me semble bonne.


Je vous donne le .h qui va bien (les #ifndef ne sont pas mis ici mais ils existent bien dans mon code) :

Code :

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

1
2
3
4
5
6
7
8
9
10
typedef struct Pile Pile;
 
struct Pile {
  int valeur;
  struct Pile *element_precedent;
};
 
Pile *creerPile(int valeur);
void ajouterNouvelElement(Pile *pile, Pile *nouvelElement);
int enleverElement(Pile *pile);

Sauriez-vous expliquer ce comportement ?

Merci

[dalfab]

Bonjour,

Une chose importante est d'avoir une objectif clair. Le type défini et les fonctions prévues indiquent un point de départ déjà flou; la struct Pile semble plutôt désigner un élément de pile. Je l'aurais appelée Element. D'ailleurs tu utilises bien Pile* pour désigner ta variable pile.
La fonction creerPile() n'a aucune raison d'y mettre un premier élément, on devrait pouvoir créer une pile vide.
Il ne devrait pas y avoir des malloc() et des free() dans main(), ils devraient être encapsulés dans les fonctions gérant les piles. Il faudrait donc une fonction detruirePile(Pile*); et la fonction ajouter élement devrait être void ajouterNouvelElement( Pile* , int valeur );.
La fonction int enleverElement( Pile* ) est correctement prototypée car elle peut modifier pile, mais tu t'y es embrouillé, on ne peut pas modifier ce type d'objet sans y utiliser des pointeurs. Remarque qu'une indirection est apparue, on pourrait aussi l'écrire int enleverElement( Element** ). Il faut cependant commencer par définir les bons types pour pouvoir la définir. Je te propose de partir de:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
typedef struct Element Element;
typedef Element *Pile; // Pile n'est pas un Element, elle pointe sur le dernier Element
 
struct Element {
  int valeur;
  struct Element *element_precedent;
};
 
Pile creerPile() { // crée une pile vide
   return NULL;
}

La fonction int enleverElement(Pile *ppile) doit :
- vérifier que la pile n'est pas vide (facile)
- trouver le dernier élément (très facile ici)
- lire le nombre dépilé (facile)
- enlever de la pile le dernier élément (facile si on fait un dessin)
- détruire le dernier élément (facile si son adresse est connue)
- retourner le nombre dépilé.

La fonction ajouterNouvelElement() est un peu plus complexe.

[sambia39]

Bonjour,
Il n'y a pas mal de choses à rectifier et surtout à ne pas faire.
Premièrement, une variable pointeur n'est pas une variable de type entier, mais plutôt une variable qui pointe sur un objet du type "entier". Et pour afficher l'adresse d'un objet, on utilise le format suivant %p. Le format dont vous utiliser est fait pour afficher la valeur d'une variable exemple une variable X = 10 en faisant printf("%d", x ) elle affichera la valeur de la variable donc 10.

Autre a chose à ne pas oublier quand on sollicite de la mémoire surtout en programmation avec le langage C, il faut vérifier que celui-ci, a été bien alloué ou pas et prendre les mesures adaptées en fonction du résultat de l'allocation exemple, s'il elle échoue, il faut prendre connaissance de la cause de l'échec et prendre les mesures adaptées l'instruction la plus simple et correcte serait de faire:

Code C :

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
 
int main( void ){
 
    void * p = NULL;
    if( NULL == (p = malloc( 10 * sizeof(char) )) ){
        /*
         *  Échec prendre les mesures nécessaires
         *  et quitter avec un message d'erreurs
         */
        return EXIT_FAILURE;
    }
    /*
     * Tout est Ok continue
     * avec les instructions suivantes
     */
    ........
    /*
     * Libère la mémoire allouée, mais l'adresse
     * est toujours contenue dans le variable pointeur
     */
    free( p );
 
    /*
     *  Mettre à NULL le pointeur
     *  pour qu'il pointe sur rien
     */
    p = NULL;
    return EXIT_SUCCESS;
}

et non comme vous l'avez fait dans votre code source, car le programme pourrait avoir un comportement indéterminé à la prochaine instruction qui utilisera l'objet. Il faut également et toujours initialiser un pointeur à NULL s'il l'on n'utilise pas dans l'immédiats cela évite de gros souci.

Quelques questions avant d'aller plus en détail dans votre code source concernant le crash de votre programme : quand en fait un POP de la pile, on retire l'élément de la pile ? à quel moment dans votre code source vous effectuez le retrait sur la pile ?

Et pourquoi quitter systématiquement avec un code erreur EXIT_FAILURE si vous avez NULL qui veut tout simplement dire que vous avez une pile vide (par exemple)?

à bientôt

[Mc geek]

Bonjour à tous,

Tout d'abord merci de vos réponses rapides ^^.

Je vais répondre à toutes vos remarques :

• Je vais modifier la définition de ma pile selon ton idée pour être plus cohérent avec la définition formelle d'une pile ;
• La fonction detruirePile est prévue, elle n'est simplement pas implémentée car je préfère procéder par itération plutôt que d'un seul coup.
• Je comprends mieux pourquoi la fonction ajouterNouvelElement doit être modifiée car je ne dois pas avoir des malloc() et des free() pour des raisons de propreté et de fiabilité du code.
• Je ne comprends pas par contre pourquoi j'ai provoqué une indirection dans enleverElement(Pile *pile) et aussi pourquoi je n'utilise pas les pointeurs alors que si je me souviens bien :
  Quand on crée un pointeur on met une *. Si on veut modifier l'adresse pointée par le pointeur, on utilise le pointeur comme si c'était une variable classique. Si on veut modifier la valeur de la variable désignée par le pointeur, on utilise à nouveau l'*.
• Je ne suis pas sûr de bien comprendre l'utilisation du typedef dans le fichier d'en-tête à cet endroit : typedef Element *Pile; quand on écrit Pile ça correspond à un pointeur sur Element ? Dans ce cas, je crois que je ne vais pas l'appliquer pour me forcer à mieux comprendre ce que je fais.
• Enfin la fonction Pile creerPile(), c'est bien sûr à moi de la coder et elle n'est pas réellement censée renvoyer NULL ?
• Pour afficher une adresse, je ne connaissais pas %p, je l'appliquerai maintenant (ça me paraissait cohérent de me dire qu'une adresse est un entier, et donc l'afficher comme tel).
• Merci du conseil pour éviter les comportements inattendus, je vais être plus rigoureux maintenant grâce à ça pour l'utilisation des pointeurs.
• Pour le POP de la pile, je voulais faire pointer la pile vers l'avant-dernier élément (c'est là que je vois que ma structure n'était pas clair) et à partir du moment où j'aurais réussi à coder ce comportement, passer par une variable intermédiaire pour ensuite vider la zone mémoire.

Enfin, je me demande si ce ne serait pas plus simple pour moi de faire deux vrais définitions de structures qui ressembleraient à ça :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
typedef struct Pile Pile;
typedef struct Element Element;
 
struct Element {
  int valeur;
  struct Element *element_precedent;
};
 
struct Pile {
  struct Element *dernier_element;
};

(Je me doute que c'est similaire à ce que tu proposes dalfab, mais je trouve cela plus clair pour moi).

[dalfab]

Ta proposition de structure est en effet plus propre que la mienne.

Enfin la fonction Pile creerPile(), c'est bien sûr à moi de la coder et elle n'est pas réellement censée renvoyer NULL ?

Dans la structure que je proposais, il suffisait de renvoyer la valeur d'une pile vide et comme Pile était un pointeur, le code était complet. Pour ta nouvelle définition de Pile, il faut un peu plus de code, et on pourrait plutôt l'écrire void initialiserPile(Pile*);.

Je ne comprends pas par contre pourquoi j'ai provoqué une indirection dans enleverElement(Pile *pile) et aussi pourquoi je n'utilise pas les pointeurs

Je me suis mal exprimé, dans ton code Pile désignait un Element, pour pouvoir modifier Pile il fallait passer en paramètre un pointeur sur une Pile alors qu'elle ne recevait qu'un pointeur sur un Element. Ici il faut passer aux fonctions l'adresse d'une Pile, car l'objet Pile peut être modifié par les fonctions.

[Mc geek]

J'ai retravaillé le code en tenant compte de vos remarques (et aussi pour certains points de cppcheck ) et maintenant ma fonction depiler marche bien, mais maintenant c'est la suppression de la pile qui me pose un problème, je vous explique :

La fonction dépiler doit effectuer un certain nombre de choses :

• si la pile en paramètre ne pointe vers rien, rien à faire ;
• si la pile pointe vers un élément NULL, alors libérer la mémoire et supprimer l'adresse enregistrée par le pointeur ;
• sinon détruire progressivement les éléments et finir le tout en détruisant la pile.

Le problème c'est que quand je teste cette fonction, le programme s'affole et affiche en boucle une liste d'adresse mémoire, et ne s'arrête qu'avec Ctrl+C.

Voilà le code de la fonction incriminée :

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
int detruirePile(Pile *ppile) {
  if (ppile == NULL) {
    return EXIT_SUCCESS;
  }
 
  if (ppile->dernier_element == NULL) {
    free(ppile);
    ppile == NULL;
    return EXIT_SUCCESS;
  }
  else {
    Element *pelementTraite = ppile->dernier_element;
 
    while (pelementTraite != NULL) {
      while (pelementTraite->element_precedent != NULL) {
        Element *ptemp = pelementTraite;
        pelementTraite = pelementTraite->element_precedent;
        free(ptemp);
        ptemp = NULL;
      }
      free(pelementTraite);
      pelementTraite = NULL;
    }
    ppile->dernier_element = NULL;
    free(ppile);
    ppile = NULL; // Note : cppcheck m'avertit que ça n'a pas d'effet en-dehors de la fonction mais je ne comprends pas pourquoi même si c'est sans doute lié à mon problème.
    return EXIT_SUCCESS;
  }
 
  return EXIT_FAILURE;
}

Vous auriez une idée pour régler le problème ?



Note :
Pour plus de lisibilité voici l'ensemble de main.c :

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
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
#include <stdlib.h>
#include <stdio.h>
 
#include "main.h"
 
int main() {
  int resultat = 0;
  Pile *ppile = creerPile();
  initialiserPile(ppile, 5);
 
  empiler(ppile, 2);
  empiler(ppile, 6);
  empiler(ppile, 7);
 
  afficherPile(ppile);
 
  resultat = depiler(ppile);
 
  printf("%d\n", resultat);
  printf("-----------------------------------\n");
  afficherPile(ppile);
 
  detruirePile(ppile);
  // afficherPile(ppile);
  return 0;
}
 
Pile *creerPile() {
  Pile *ppile = (Pile*)malloc(sizeof(Pile));
  ppile->dernier_element = NULL;
  return ppile;
}
 
int initialiserPile(Pile *ppile, int nombre) {
  Element *nouveau = (Element*)malloc(sizeof(Element));
  if (nouveau == NULL || ppile == NULL) {
    free(nouveau);
    return EXIT_FAILURE;
  }
 
  nouveau->valeur = nombre;
  nouveau->element_precedent = NULL;
  ppile->dernier_element = nouveau;
  return EXIT_SUCCESS;
}
 
int empiler(Pile *ppile, int valeur) {
  Element *nouveau = (Element*)malloc(sizeof(Element));
  if (nouveau == NULL || ppile == NULL) {
    free(nouveau);
    return EXIT_FAILURE;
  }
 
  nouveau->valeur = valeur;
  nouveau->element_precedent = ppile->dernier_element;
  ppile->dernier_element = nouveau;
  return EXIT_SUCCESS;
}
 
int depiler(Pile *ppile) {
  int valeurRetour;
 
  if (ppile == NULL) {
    return EXIT_FAILURE;
  }
 
  if (ppile->dernier_element == NULL) {
    printf("Rien a depiler\n");
    return EXIT_SUCCESS;
  }
 
  Element *pdernierElement = ppile->dernier_element;
  ppile->dernier_element = pdernierElement->element_precedent;
  valeurRetour = pdernierElement->valeur;
  free(pdernierElement);
  pdernierElement = NULL;
  return valeurRetour;
}
 
int detruirePile(Pile *ppile) {
  if (ppile == NULL) {
    return EXIT_SUCCESS;
  }
 
  if (ppile->dernier_element == NULL) {
    free(ppile);
    ppile == NULL;
    return EXIT_SUCCESS;
  }
  else {
    Element *pelementTraite = ppile->dernier_element;
 
    while (pelementTraite != NULL) {
      while (pelementTraite->element_precedent != NULL) {
        Element *ptemp = pelementTraite;
        pelementTraite = pelementTraite->element_precedent;
        free(ptemp);
        ptemp = NULL;
      }
      free(pelementTraite);
      pelementTraite = NULL;
    }
    ppile->dernier_element = NULL;
    free(ppile);
    ppile = NULL; // Note : cppcheck m'avertit que ça n'a pas d'effet en-dehors de la fonction mais je ne comprends pas pourquoi
    return EXIT_SUCCESS;
  }
 
  return EXIT_FAILURE;
}
 
void afficherPile(Pile *ppile) {
  if (ppile == NULL || ppile->dernier_element == NULL) {
    printf("Rien a afficher (la pile n'existe pas, ou alors elle n'a pas d'element enregistre).\n");
  }
 
  Element *pactuel = ppile->dernier_element;
 
  while (pactuel != NULL) {
    printf("%d\n", pactuel->valeur);
    pactuel = pactuel->element_precedent;
  }
  printf("\n");
}

et de main.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
#ifndef E__DOCUMENTS_CODE_C_TEST_MAIN_H_
#define E__DOCUMENTS_CODE_C_TEST_MAIN_H_
 
typedef struct Element Element;
typedef struct Pile Pile;
 
struct Element {
  int valeur;
  struct Element *element_precedent;
};
 
struct Pile {
  struct Element *dernier_element;
};
 
Pile *creerPile();
int initialiserPile(Pile *ppile, int nombre);
void afficherPile(Pile *ppile);
int empiler(Pile *ppile, int valeur);
int depiler(Pile *ppile);
int detruirePile(Pile *ppile);
 
#endif  // E__DOCUMENTS_CODE_C_TEST_MAIN_H_

[dalfab]

Commençons par le Warning ligne 26.
Tu modifies une variable locale juste avant un return, cela n'a aucun intérêt car la variable disparaît immédiatement. Un rappel, le seul moyen de modifier une variable hors du contexte de la fonction est d'utiliser un pointeur que l'on déréférence.
La ligne 24 devrait avoir le même warning, plus difficile à voir (tu modifies une structure qui est détruite immédiatement après.)
La ligne 19 aussi, tu modifies ptemp à la fin du bloc où elle est définie.

Pour détruire les éléments empilés, je comprends pas le besoin d'avoir deux boucles while imbriquées. Il suffit de parcourir les éléments et les détruire au fur est à mesure.

Code :

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

1
2
3
4
5
6
7
    Element *pelementTraite = ppile->dernier_element;
 
    while ( pelementTraite != NULL ) {
        Element *ptemp = pelementTraite->element_precedent;
        free( pelementTraite );
        pelementTraite = ptemp;
    }

Tu peux aussi supprimer les lignes 6 à 11 (surtout la 8 que le compilo doit trouver bizarre) car le cas où il n'y a pas d'élément n'est pas un cas particulier.

Indépendamment je n'ai pas vu dans ton code ce qui provoque un plantage, il faudrait le dérouler en pas à pas avec un debugger.