Discussion :

[memedplay]

Bonjour,
J'essaie de créer une file en c et je bloque à cause d'une erreur que je ne comprend pas.
Quelqu'un saurait il d'où vient le problème ?

Voici le code :

file.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
26
27
28
29
30
31
 
#ifndef FILE_H
#define FILE_H
 
typedef struct tagQueueElt{
  int data;
  struct tagQueueElt* next;
}QueueElt;
 
typedef struct tagQueue{
  QueueElt* first;
  QueueElt* last;
  int size
}Queue;
 
// Initialisation de la file
void initialization(Queue*);
 
//Taille de la file
int size(Queue*);
 
//Ajout d'un élément dans la file
int push(Queue*, int);
 
//Supression du premier élément de la file
int pop(Queue*);
 
//Affichage de la liste
void affiche(Queue*);
 
#endif

file.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
#include <stdlib.h>
#include <stdio.h>
#include "file.h"

// Initialisation de la file
void initialization(Queue* file){
  file->first = NULL;
  file->last = NULL;
  file->size = 0;
}

//Taille de la file
int size(Queue* file){
  return file->size;
}

//Ajout d'un élément dans la file
int push(Queue* file, int data){
  QueueElt* newelt;
  if ((newelt = (QueueElt*) malloc (sizeof (QueueElt))) == NULL)
    return -1;
 
  newelt->data = data;

  QueueElt* courant = file->first;
  if(courant == NULL){
    file->last = newelt;
    newelt->next = file->first;
    file->first = newelt;
  }else{
    if(courant->next == NULL){
      file->last = newelt;
    newelt->next = courant->next;
    courant->next = newelt;
    }
    else{
      file->last->next = newelt;
      newelt->next = NULL;
    }
  }
  file->size++;
  return 0;
}


//Supression du premier élément de la file
int pop(Queue* file){
  QueueElt* eltsupp;
  if (file->size == 0)
    return -1;
  eltsupp = file->first;
  file->first = file->first->next;
  free(eltsupp);
  file->size--;
  return 0;
}


//Affichage de la liste
void affiche(Queue* file){
  QueueElt* courant = malloc(sizeof(QueueElt));
  int i;
  courant = file->first;
  
  for(i = 0 ; i < file->size ; i++){
    printf(" %d ", courant->data);
    courant = courant->next;
  }
}

Le test

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
 
#include <stdlib.h>
#include <stdio.h>
#include "file.h"
 
int main(){
  Queue* file = malloc(sizeof (Queue*));
  printf("taille de file vide : %d\n", size(file));
  push(file,10);
  push(file,20);
  push(file,30);
  push(file,40);
  push(file,50);
  affiche(file);
  printf("\n");
  pop(file);
  affiche(file);
  printf("\n");
}

J'obtient 2 erreurs :

1. file.h:14:1: warning: no semicolon at end of struct or union [enabled by default]
   }Queue;

1. Erreur de segmentation portant sur le printf(" %d ", courant->data); en gras

Merci

[Médinoc]

Ta fonction push() est vraiment bizarre.
Par moments j'ai l'impression qu'elle chaîne à l'envers.

De plus, tu devrais éviter les noms push() et pop() pour une file: Ils sont plus appropriés pour une pile. Tu devrais appeler tes fonctions enqueue() et dequeue() à la place.

Un bon truc pour éviter les bugs, c'est de faire un fonction qui vérifie l'intégrité de la file. Par exemple, une fonction qui compte les éléments, et vérifie la cohérence avec last et size.

PS: Pourquoi ta fonction d'affichage fait-elle un malloc()?

[memedplay]

je vois pas ce que tu veux dire "chaine à l'envers".

Le malloc sert à rien effectivement, mais il ne crée pas d'erreur.

[Médinoc]

Après plus de réflexion, j'ai l'impression que ta file est, en fait, circulaire (le dernier élément a son pointeur next qui pointe sur le premier, au lieu d'être nul).
Y a-t-il une raison à cela?

[memedplay]

C'était pour mettre faire une boucle lorsque la liste est vide (pas une très bonne idée en fait)

[Bktero]

Quel est ton compilateur ? Parce que le warning pour me semble plutôt est une erreur de syntaxe :

Code :

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

1
2
3
4
5
typedef struct tagQueue{
  QueueElt* first;
  QueueElt* last;
  int size // il manque le ;
}Queue;

As-tu corrigé ce problème pour commencer ?

[Sve@r]

je vois pas ce que tu veux dire "chaine à l'envers".

Salut
Comme ton code déborde de commentaires, on a un peu de mal à faire le tri. Et donc ce n'est pas facile de bien voir comment l'élément est "enfilé" dans la file...

Le malloc sert à rien effectivement, mais il ne crée pas d'erreur.

Ben voyons. Et ce sont les mêmes qui râlent quand un programme consomme trop de mémoire...

Code C :

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

1
2
3
4
5
typedef struct tagQueue{
  QueueElt* first;
  QueueElt* last;
  int size
}Queue;

Ca fait plaisir de voir que tu as pensé à créer un type pour la queue elle-même. Toutefois le mot clef "tagQueue" est inutile. Et si tu nommais ton type "t_Queue" tu serais plus conforme avec les conventions...

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
 
#include <stdlib.h>
#include <stdio.h>
#include "file.h"
 
// Initialisation de la file
void initialization(Queue* file){
  file->first = NULL;
  file->last = NULL;
  file->size = 0;
}
 
//Taille de la file
int size(Queue* file){
  return file->size;
}
 
//Ajout d'un élément dans la file
int push(Queue* file, int data){
  QueueElt* newelt;
  if ((newelt = (QueueElt*) malloc (sizeof (QueueElt))) == NULL)
    return -1;
 
  newelt->data = data;
 
  QueueElt* courant = file->first;
  if(courant == NULL){
    file->last = newelt;
    newelt->next = file->first;
    file->first = newelt;
  }else{
    if(courant->next == NULL){
      file->last = newelt;
    newelt->next = courant->next;
    courant->next = newelt;
    }
    else{
      file->last->next = newelt;
      newelt->next = NULL;
    }
  }
  file->size++;
  return 0;
}
 
 
//Affichage de la liste
void affiche(Queue* file){
  QueueElt* courant = malloc(sizeof(QueueElt));
  int i;
  courant = file->first;
 
  for(i = 0 ; i < file->size ; i++){
    printf(" %d ", courant->data);
    courant = courant->next;
  }
}

Erreur de segmentation portant sur le printf(" %d ", courant->data); en gras

Code c :

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

1
2
3
4
5
6
7
8
//Affichage de la liste
void affiche(Queue* file){
  QueueElt* courant;
  int i;
 
  for (i = 0, courant=file->first ; i < file->size ; i++, courant=courant->next)
    printf(" %d ", courant->data);
}

Sinon tu as créé une belle fonction "initialization()". Tu l'utilises quand ???

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
 
#include <stdlib.h>
#include <stdio.h>
#include "file.h"
 
int main(){
  Queue* file = malloc(sizeof (Queue*));
  printf("taille de file vide : %d\n", size(file));
  push(file,10);
  push(file,20);
  push(file,30);
  push(file,40);
  push(file,50);
  affiche(file);
  printf("\n");
  pop(file);
  affiche(file);
  printf("\n");
}

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
 
#include <stdlib.h>
#include <stdio.h>
#include "file.h"
 
int main(){
  Queue file;
  printf("taille de file vide : %d\n", size(&file));
  push(&file,10);
  push(&file,20);
  push(&file,30);
  push(&file,40);
  push(&file,50);
  affiche(&file);
  printf("\n");
  pop(&file);
  affiche(&file);
  printf("\n");
}

Ca ne fonctionnera pas mieux mais au-moins ça t'évite

1. un malloc inutile (pourquoi les gens font un malloc de "1" ça je ne le comprendrai jamais !!!)
2. d'oublier de le nettoyer après.
3. et surtout de te vautrer dans ton malloc (ce n'est pas sizeof(Queue*) mais sizeof(Queue) !!!)

En fait, tout bien réfléchi, peut-être bien que ça fonctionnera en réalité beaucoup mieux...

C'était pour mettre faire une boucle lorsque la liste est vide (pas une très bonne idée en fait)

Ben non. Te suffit de tester size == 0 pour détecter si ta liste est vide ou pas. Parce que boucler sur soi-même ça peut vite devenir synonyme de boucle infinie...

[Médinoc]

Toutefois le mot clef "tagQueue" est inutile. Et si tu nommais ton type "t_Queue" tu serais plus conforme avec les conventions...

En fait, typedef struct tagXXXXX { ... } XXXXX; est une convention, qui est utilisée pour pratiquement toutes les structures sous Windows...

[memedplay]

Merci à tous!