Realloc() - Segmentation fault & clean_stdin

**Trademark** · 30/01/2010, 14h58

Bonjour à tous,

Programme en général :

Le programme que j'ai écrit est un programme de jeux Pile ou face. L'utilisateur rentre p ou f et grace aux fonctions "aléatoires" du C on génére pile ou face.

Problème realloc() : [RESOLU]

On nous a demandé d'optimiser l'espace mémoire, on doit stocker le tirage et les propositions du pile ou face, ce que je fais grâce a un pointeur. J'utilise malloc() au départ :

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
 
/* Allocation de tab_tirage et tab_proposition d'une taille mémoire d'un char.
*/
tab_tirage =  malloc( sizeof(char));	// Le retour du malloc pourrait être caster (void*) mais c'est inutile.
	if (tab_tirage == NULL)
	{
		printf("MALLOC : Erreur d'allocation\n");
		exit(1);
	}
tab_proposition =  malloc( sizeof(char));
	if (tab_proposition == NULL)
	{
		printf("MALLOC : Erreur d'allocation\n");
		exit(1);
	}

Et a chaque fois que l'utilisateur re-lance une pièce je realloc ces deux vecteurs d'une case :

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
 
 
/* 
A chaque fois que l'utilisateur lance une nouvelle piece on allonge notre vecteur de 1 case (de taille char) pour stocker le résultat. 
*/
 
void alloueMemoire(char*tab_tirage,char *tab_proposition){
 
/* /!\ realloc() renvoie un nouveau pointeur, problème ?
*/
 
	tab_tirage = realloc((char*)(tab_tirage),cpt_lancePiece* sizeof(char));	
	if (tab_tirage == NULL)
	{	
		printf("REALLOC : Erreur d'allocation\n");
		exit(1);
	}
 
	tab_proposition = realloc((char*)(tab_proposition),cpt_lancePiece* sizeof(char));
	if (tab_proposition == NULL)
	{
		printf("REALLOC : Erreur d'allocation\n");
		exit(1);
	}
 
}

J'ai consulté de nombreux forums d'aide et ils donnaient des solutions diverses et variées mais qui n'ont pas résolu mon problème.
Ha oui... le problème justement, c'est que au bout de 24 lancé (virtuelle) de pièce, par conséquent au bout de 24 realloc() j'ai une erreur de segmentation.
Il parait que ce n'est pas trop conseillé de reallocé autant mais bon sinon que faire ? Mais ce qui m'interresse c'est surtout de savoir pourquoi ca ne va pas.

Solution : Quelques améliorations dans la fonction realloc en cas de cr@sh ! Mon problème se situait au niveau de la variable pieceLance qui ne valait, lors du premier passage, 1. Donc la fonction re-alloc ré-allouait seulement 1 case (inutile & inefficace).

Problème 2 : Vider le buffer clavier. [RESOLU]

Ce n'est pas vraiment un problème plutot quelque chose d'agacant.
A un moment donné je demande à l'utilisateur d'entré P ou F :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
 
	char buffer=0;		// Variable locale à la fonction lancePiece.	
 
// Vérification de l'entrée de l'utilisateur.
 
	while (buffer != 70 && buffer != 80 && buffer != 102 && buffer != 112 )
	{
		printf("Quelle est votre proposition ? <Pour pile taper p ou f pour face> : ");
		clean_stdin();
		buffer = getchar();
	}

Fonction clean_stdin(), venant de la faq de ce forum.

Code :

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

1
2
3
4
5
6
7
8
9
10
 
 
void clean_stdin(void)
{
    int c;
 
    do {
        c = getchar();
    } while (c != '\n' && c != EOF);
}

Lorsque l'utilisateur se trompe, n'entre rien et clique directement sur enter, lorsqu'il va essayer de re-entré p ou f il devra l'entré 2 fois de cette facon :

p <enter> p <enter>

Comment puis-je arranger ca ?

Solution : Nouvelle fonction LireClavier();

Code :

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

1
2
3
4
5
6
7
8
 
int LireClavier(void)
{
  int car,c;
  car = c = getchar();
  while (c != '\n' && c != EOF)c = getchar();
  return car;
}

Merci d'avoir lu mon post et de vos prochaines aides =)

Bye bye

**jeroman** · 30/01/2010, 15h26

Lorsque tu appelles la fonction alloueMemoire, elle te crée localement deux pointeurs, qu'elle initialise avec les valeurs respectives des deux pointeurs que tu lui passes.
Lorsque ces deux pointeurs sont modifiés au sein de la fonction, ils sont détruits lorsqu'elle se termine.
Par conséquent, non seulement les deux pointeurs, dans la fonction appelante, ont une valeur inchangée qui, à cause de l'appel à realloc, correspondent alors ensuite à une adresse dont l'accès devient interdit (qui provoquera donc un crash lors d'un accès à cette adresse), mais cela provoque aussi à une fuite de mémoire car les nouvelles adresses renvoyées par realloc sont alors perdues.

Pour modifier ces pointeurs via une autre fonction, il faut passer l'adresse de ces pointeurs et non leur valeur.
Ta fonction alloueMemoire doit donc ressembler à ça :

Code :

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

1
2
3
4
5
6
7
void alloueMemoire(char const ** const tab_tirage,char const **const tab_proposition)
{
(...) /* code à changer */
}
 
(...) /* plus loin dans une autre fonction */
alloueMemoire(&tab_tirage, &tab_proposition);

Pour résumer :
tab_tirage correspond à l'adresse où est stocké en mémoire le pointeur de la fonction appelante (celui qui est passé en argument).
*tab_tirage correspond à sa valeur (celle qu'on veut changer "à distance").
**tab_tirage correspond à l'objet pointé (le char).

Sinon, deux, trois petits trucs aussi.
1) Il faut faire attention à realloc. Si tu écrases le pointeur, sans faire une copie, et que la fonction échoue, tu as une fuite de mémoire. Si cette fonction échoue, l'allocation existe toujours et rien n'est donc libéré.

2) A chaque malloc, il doit y avoir un free correspondant. Il faut donc structurer le code de manière à libérer proprement les zones de mémoire allouées si un des malloc échoue, ou lorsque le programme se termine normalement.
En gros : exit, les exit. ^^

**diogene** · 30/01/2010, 15h36

Pour le realloc() :

Le realloc peut changer l'adresse où se trouve les données et il faut donc la récupérer. Par exemple :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
void alloueMemoire(char** ptirage,char ** pproposition)
{
        char * p;
	p= realloc(*ptirage,cpt_lancePiece);	
	if (p == NULL)
	{	
        ....
	}
        else * ptirage = p;
.....
}
.....
alloueMemoire( &tab_tirage, &tab_proposition);

- cpt_lancePiece est une variable globale ? Ce n'est pas bien ! Ce devrait être un paramètre de la fonction.

- A noter qu'on peut aussi allouer initialement par cette fonction en mettant tab_tirage et tab_proposition à NULL.

Pour le vidage clavier :

La fonction clean_stdin() suppose que le buffer n'est pas vide et donc qu'il faut le vider. Si il est vide, on va faire des lectures pour rien ici

Code :

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

1
2
3
    do {
        c = getchar();
    } while (c != '\n' && c != EOF);

**Trademark** · 30/01/2010, 21h45

Je vous remercie de vos réponses et je suis parvenu à régler le problème 2.
Pour ce qui est du problème 1, celui du realloc, j'en suis au point de départ et je n'ai pas compris certaines choses dans vos messages.
J'ai tout de même modifier mon code qui ressemble à ça maintenant :

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
 
 
// Prototype : 
 
void alloueMemoire(char**,char**);
 
/* 
A chaque fois que l'utilisateur lance une nouvelle piece on allonge notre vecteur de 1 case (de taille char) pour stocker le résultat. 
*/
 
void alloueMemoire(char**ptirage,char**pproposition){
 
	char *p;
	p = realloc(*ptirage,cpt_lancePiece/** sizeof(char)*/);	
	if (p == NULL)
	{	
		printf("REALLOC : Erreur d'allocation\n");
		free(ptirage);
		free(pproposition);
		exit(1);
	}
	else *ptirage = p;
 
	p = realloc(*pproposition,cpt_lancePiece/** sizeof(char)*/);
	if (p == NULL)
	{
		printf("REALLOC : Erreur d'allocation\n");
		free(ptirage);
		free(pproposition);
		exit(1);
	}
	else *pproposition = p;
}

Bon voilà je pense avoir suivi vos conseils

, seulement c'est toujours au bout de 24 lancées que j'ai une segmentation fault.

Envoyé par diogene

cpt_lancePiece est une variable globale ? Ce n'est pas bien ! Ce devrait être un paramètre de la fonction.

Tu me dis ça mais je ne comprends pas pourquoi c'est mal ? Rajouter un paramètre n'alourdis t'il pas la fonction ? Et n'est-ce pas plus court de mettre cette variable en globale qui est utilisée dans d'autres fonctions ?
Elle est déclarée comme ça :

Code :

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

1
2
 
int static cpt_lancePiece;

Je sais que tu as raison et qu'une variable globale c'est vraiment moche mais je ne vois pas pourquoi je devrais m'en passer.

Envoyé par jeroman

void alloueMemoire(char const ** const tab_tirage,char const **const tab_proposition)

Je ne comprends pas très bien pourquoi tu mets des const :/
Lorsque je les rajoute j'obtient une erreur de compilation :

Code :

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

1
2
3
4
5
6
7
8
 
PileouFace.c:192: error: conflicting types for ‘alloueMemoire’
PileouFace.c:49: note: previous declaration of ‘alloueMemoire’ was here
PileouFace.c: In function ‘alloueMemoire’:
PileouFace.c:197: warning: passing argument 1 of ‘realloc’ discards qualifiers from pointer target type
/usr/include/stdlib.h:485: note: expected ‘void *’ but argument is of type ‘const char *’
PileouFace.c:207: warning: passing argument 1 of ‘realloc’ discards qualifiers from pointer target type
/usr/include/stdlib.h:485: note: expected ‘void *’ but argument is of type ‘const char *’

Envoyé par jeroman

Si tu écrases le pointeur, sans faire une copie, et que la fonction échoue, tu as une fuite de mémoire

Je pense que je corrige ça grâce au nouveau pointeur p, n'est-ce pas ?

Envoyé par jeroman

A chaque malloc, il doit y avoir un free correspondant. Il faut donc structurer le code de manière à libérer proprement les zones de mémoire allouées si un des malloc échoue, ou lorsque le programme se termine normalement.
En gros : exit, les exit. ^^

Ma manière marche t'elle ? C'est-à-dire faire deux free() avant le exit(1) ?

Envoyé par diogene

p= realloc(*ptirage,cpt_lancePiece);

Comme j'ai pu constater tu ne mets que le coefficiant multiplicateur de la taille, doit-on obligatoirement ajouter un : * sizeof(char) ? Est-ce que c'est sous-entendu selon le type de l'ancien (ou nouveau?) pointeur ? Ou alors est-ce simplement parce que realloc attends une taille en octet et que le type char ne fait de toutes facons que 1 octet ?

Je suis désolé, ca fait encore un post à rallonge mais enfin tant que j'y suis...

Merci à vous,
Tchao

**diogene** · 30/01/2010, 23h20

- Erreur

Code :

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

1
2
3
4
void alloueMemoire(char**ptirage,char**pproposition){
....
		free(*ptirage);
		free(*pproposition);

-

seulement c'est toujours au bout de 24 lancées que j'ai une segmentation fault.

Il faut voir le reste du code

-

Et n'est-ce pas plus court de mettre cette variable en globale qui est utilisée dans d'autres fonctions ?
....
mais je ne vois pas pourquoi je devrais m'en passer

Parce que tu peux t'en passer. Les variables globales ne doivent pas être utilisées simplement pour s'épargner un passage de paramètre. Elles sont néfastes pour la lecture et la maintenance du code parce que :
1- les fonctions cessent d'être une entité autonome et dépendent de quelque chose qui leur est étranger.
2- il est difficile lorsque le code devient un peu copieux de savoir qui les modifie et quand. Si il y a une valeur erronée dedans, qui la mise et quand ? Le debug risque d'être coton.

-

Je pense que je corrige ça grâce au nouveau pointeur p, n'est-ce pas ?

Oui
-

Ma manière marche t'elle ? C'est-à-dire faire deux free() avant le exit(1) ?

Mais tu ne fais pas les free() des bons pointeurs (voir début du post)

-

Ou alors est-ce simplement parce que realloc attends une taille en octet et que le type char ne fait de toutes facons que 1 octet ?

Oui. sizeof(char) vaut toujours 1 (byte)

**jeroman** · 31/01/2010, 01h21

Je ne comprends pas très bien pourquoi tu mets des const :/

Généralement, il est préférable de mettre des 'const' partout où l'on peut (ce qui veut bien dire qu'il ne faut pas en mettre sur des objets qu'on veut modifier), afin d'éviter de modifier accidentellement des variables (ou des pointeurs) que la fonction ne doit pas modifier. Modifier une valeur qui est 'const' provoque une erreur à la compilation, c'est donc sécuritaire.
Mais avec le premier 'const', il y a effectivement un binz que je ne comprends pas. Je replongerai dans la norme demain, il y a quelque chose qui n'est pas clair.

Ma manière marche t'elle ? C'est-à-dire faire deux free() avant le exit(1) ?

Oui, 'free', c'est bien... mais 'free' sans 'exit', c'est encore mieux. Enfin, c'est mon avis à moi.

Le problème de 'exit', c'est qu'il termine le programme sauvagement alors qu'en principe un programme doit être suffisamment bien structuré pour qu'il gère lui-même les erreurs qui pourraient se produire (fonctions qui échouent, etc) et que par conséquent il libère lui-même tout ce qu'il a alloué, même si l'O.S. le fait lui-même. C'est juste une question de principe, qui permet de coder proprement.
C'est notamment la raison pour laquelle je te conseillerais de faire retourner une valeur à ta fonction d'allocation, afin que le code appelant puisse lui-même gérer en cas d'erreur.

Voilà un code (une ébauche, qui ne sert pas à grand-chose en l'état, mais qu'il faudra compléter en fonction de ce que tu veux faire) que je te propose, il y a peut-être moyen de faire mieux :

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
#include<stdio.h>
#include<stdlib.h>
 
#define NOMBRE_COUPS_MAXI 50
 
int alloueMemoire(char ** const , char ** const , size_t const );
 
int alloueMemoire(char ** const ptirage , char ** const pproposition , size_t const cpt_lancePiece)
{
	char *p;
 
	p = realloc( *ptirage , cpt_lancePiece ); /* Si le 1er parametre de 'realloc' vaut NULL, alors la fonction se comporte comme 'malloc' */
	if (p == NULL)
	{	
		printf("REALLOC : Erreur d'allocation\n");
		free(*ptirage) , *ptirage = NULL;
		free(*pproposition) , *pproposition = NULL;
		return 0;
	} else
		*ptirage = p;
 
	p = realloc( *pproposition , cpt_lancePiece ); /* ... et ici aussi, forcément. */
	if (p == NULL)
	{
		printf("REALLOC : Erreur d'allocation\n");
		free(*ptirage) , *ptirage = NULL;
		free(*pproposition) , *pproposition = NULL;
		return 0;
	} else
		*pproposition = p;
 
	return 1;
}
 
int main(void)
{
	size_t cpt_lancePiece = 0;
	char * ptirage = NULL , * pproposition = NULL;
	int success;
 
	/* on boucle 'NOMBRE_COUPS_MAXI' fois, sauf si une erreur survient avant */
	while ( ++cpt_lancePiece <= NOMBRE_COUPS_MAXI &&  (success = alloueMemoire ( &ptirage , &pproposition , cpt_lancePiece )) )
	{
 
		/* ... ajouter du code ici (stocker en mémoire, dans la mémoire allouée, le dernier coup joué (pile ou face), etc) ... */
 
		printf("%d> %p %p\n", cpt_lancePiece , ptirage , pproposition); /* inutile, affiche simplement la valeur des pointeurs à chaque appel de 'alloueMemoire' */
	}
 
	free (ptirage) , ptirage = NULL; /* il n'y a aucun risque à utiliser 'free' même avec une valeur NULL en paramètre (au cas où le pointeur aurait été remis à NULL juste avant) */
	free (pproposition) , pproposition = NULL;
 
	getchar(); /* évite au programme de refermer sa fenêtre immédiatement, cela permet de lire les résultats */
 
	return 0;
}

**Trademark** · 31/01/2010, 11h43

Alors là je dois avouer que je suis perdu, à chaque fois que j'ai re-posté j'ai essayer de faire marcher ce programme pendant bien 2 heures.
J'ai fini par passer 3 variables sur 5 en paramètre et là c'est encore plus la galère pour comprendre (je pense?) et le problème des realloc n'est toujours pas résolu, bref je suis un

.

Bon en désespoir de cause je poste tout mon code qui a toujours le même problème. J'ai néanmoins suivis vos conseils, j'ai passé 3 de mes variables globales en paramètres, j'ai changé ma fonction realloc.

J'ai également pas mal commenté ce programme, c'est le premier programme que je commente autant donc si vous avez des conseils, ou si il est mal commenté etc. Hésitez pas a me descendre

J'espere quand même qu'il le sera assez parce que je pense que ca ne doit pas être facile de relire quelque chose de sale. Vu que c'est la première fois que je poste un code en entier, n'hésitez surtout pas à me dire ce qui ne va pas dans ma manière de coder.

Merci beaucoup, vos réponses m'ont déjà été d'une grande aide.

EDIT : si ca peut vous aider :

OS : Linux
compilateur : gcc
éditeur : VIM

Realloc() - Segmentation fault & clean_stdin

C

Vue hybride

Discussions similaires

Partager

Partager