IdentifiantMot de passe
Loading...
Mot de passe oublié ?Je m'inscris ! (gratuit)
Navigation

Inscrivez-vous gratuitement
pour pouvoir participer, suivre les réponses en temps réel, voter pour les messages, poser vos propres questions et recevoir la newsletter

 C Discussion :

Realloc() - Segmentation fault & clean_stdin


Sujet :

C

Vue hybride

Message précédent Message précédent   Message suivant Message suivant
  1. #1
    Membre Expert Avatar de Trademark
    Profil pro
    Inscrit en
    Février 2009
    Messages
    762
    Détails du profil
    Informations personnelles :
    Localisation : France

    Informations forums :
    Inscription : Février 2009
    Messages : 762
    Par défaut Realloc() - Segmentation fault & clean_stdin
    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

  2. #2
    Membre Expert
    Profil pro
    Inscrit en
    Août 2006
    Messages
    1 104
    Détails du profil
    Informations personnelles :
    Localisation : France

    Informations forums :
    Inscription : Août 2006
    Messages : 1 104
    Par défaut
    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. ^^

  3. #3
    Expert confirmé
    Avatar de diogene
    Homme Profil pro
    Enseignant Chercheur
    Inscrit en
    Juin 2005
    Messages
    5 761
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Essonne (Île de France)

    Informations professionnelles :
    Activité : Enseignant Chercheur
    Secteur : Enseignement

    Informations forums :
    Inscription : Juin 2005
    Messages : 5 761
    Par défaut
    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);

  4. #4
    Membre Expert Avatar de Trademark
    Profil pro
    Inscrit en
    Février 2009
    Messages
    762
    Détails du profil
    Informations personnelles :
    Localisation : France

    Informations forums :
    Inscription : Février 2009
    Messages : 762
    Par défaut
    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.

    Citation Envoyé par diogene Voir le message
    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.

    Citation Envoyé par jeroman Voir le message
    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 *’
    Citation Envoyé par jeroman Voir le message
    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 ?

    Citation Envoyé par jeroman Voir le message
    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) ?

    Citation Envoyé par diogene Voir le message
    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

  5. #5
    Expert confirmé
    Avatar de diogene
    Homme Profil pro
    Enseignant Chercheur
    Inscrit en
    Juin 2005
    Messages
    5 761
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Essonne (Île de France)

    Informations professionnelles :
    Activité : Enseignant Chercheur
    Secteur : Enseignement

    Informations forums :
    Inscription : Juin 2005
    Messages : 5 761
    Par défaut
    - 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)

  6. #6
    Membre Expert
    Profil pro
    Inscrit en
    Août 2006
    Messages
    1 104
    Détails du profil
    Informations personnelles :
    Localisation : France

    Informations forums :
    Inscription : Août 2006
    Messages : 1 104
    Par défaut
    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;
    }

  7. #7
    Membre Expert Avatar de Trademark
    Profil pro
    Inscrit en
    Février 2009
    Messages
    762
    Détails du profil
    Informations personnelles :
    Localisation : France

    Informations forums :
    Inscription : Février 2009
    Messages : 762
    Par défaut
    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

+ Répondre à la discussion
Cette discussion est résolue.

Discussions similaires

  1. Pb segmentation fault avec glutinit()
    Par pipistrelle dans le forum GLUT
    Réponses: 2
    Dernier message: 17/11/2004, 23h17
  2. [SDL_Image] Img_Load : segmentation fault ....
    Par Mathieu.J dans le forum OpenGL
    Réponses: 6
    Dernier message: 19/10/2004, 23h52
  3. [REDHAT] Segmentation fault systematique
    Par mela dans le forum RedHat / CentOS / Fedora
    Réponses: 2
    Dernier message: 21/09/2004, 06h05
  4. Réponses: 13
    Dernier message: 13/07/2004, 15h41
  5. Comment contrer la "segmentation fault" ?
    Par guillaume_pfr dans le forum C
    Réponses: 15
    Dernier message: 08/08/2003, 13h43

Partager

Partager
  • Envoyer la discussion sur Viadeo
  • Envoyer la discussion sur Twitter
  • Envoyer la discussion sur Google
  • Envoyer la discussion sur Facebook
  • Envoyer la discussion sur Digg
  • Envoyer la discussion sur Delicious
  • Envoyer la discussion sur MySpace
  • Envoyer la discussion sur Yahoo