my program just crash after for loop ?any solution for that?

Question

i am trying to make a program that create a list of custommers and then editing the list like i want by deleting , adding a customer before another etc , for some reason i didnt find out yet why my program crash after displaying custommers exact after using function afficher();

test:main

#include <stdio.h>
#include <stdlib.h>
#include "liste.h"
#include<string.h>

int main()
{
    liste data;
    printf("creation liste . . .\n");   /*list creation*/
    creer_liste(&data);
    if (data.premier == NULL)
    {
        printf("done!\n");
    }

    int nb, i;
    printf("combien de personnes souhaitez ajouter a la liste?\n");
    scanf("%d", &nb);   /*reading number of custommers*/
    personne p1;
    for (i=0; i < nb; i++)
    {
        printf("donner le nom de la personne %d:\n", i+1);
        scanf("%20s", p1.nom);
        printf("donner l'annee de naissance de la personne %d:\n", i+1);
        scanf("%d", &p1.annee_naissance);
        printf("donner le numero de telephone de la personne %d:\n", i+1);
        scanf("%20s", p1.tel);
        ajouter_en_tete(p1, &data); /*add custommer info in the begenning of list*/
    }
    afficher(data); /*displaying list*/

    int pos;
    printf("veuillez ajouter un element avant un autre?\n");
    printf("donner la position de cette element:\n");
    scanf("%d", &pos);  /*reading position of the custommer that you want to insert before it*/

    cellule *add_elt;
    add_elt = data.premier;
    for (i = 1; i < pos; i++)
    {
        add_elt = add_elt->suivant;
    }

    printf("donner le nom de la nouvelle personne:\n");
    scanf("%s", p1.nom);

    printf("donner l'annee de naissance de la nouvelle personne %d:\n", i+1);
    scanf("%d", &p1.annee_naissance);

    printf("donner le numero de telephone de la nouvelle personne %d:\n", i+1);
    scanf("%s", p1.tel);

    ajouter_avant(add_elt, p1, &data);  /*adding the new customer info*/

    afficher(data);
}/*displaying*/

Prototypes:

#ifndef LISTE_H_INCLUDED
#define LISTE_H_INCLUDED

typedef struct
{
    char nom[20];
    int annee_naissance;
    char tel[20];
} personne; /*every customer has a name , date of birth and phone */

typedef struct noeud
{
    personne p;
    struct noeud *suivant;
} cellule;  /*node contains the custommer info and the next address*/

typedef struct
{
    cellule *premier;
} liste;    /*list that has one entry*/

void creer_liste(liste *);  /*creation*/
void ajouter_en_tete(personne, liste *);    /*adding at begenening of list*/
void ajouter_en_queue(personne, liste *);   /*adding to the end of list*/
void supprimer_element(cellule *, liste *); /*deleting element*/
void afficher(liste);   /*displaying list*/
void ajouter_apres(cellule *, personne, liste *);   /*adding after a custommer*/
void ajouter_avant(cellule *, personne, liste *);   /*adding before a custommer*/

#endif // LISTE_H_INCLUDED

Implementation:

#include "liste.h"
#include <stdlib.h>
#include <assert.h>
#include <stddef.h>
#include <stdio.h>

void creer_liste(liste *ll)
{
    assert(ll);
    ll->premier=NULL;
}

void ajouter_en_tete(personne pp, liste *ll)
{
    assert(ll);
    cellule *nouveau;
    nouveau = (cellule *) malloc(sizeof(cellule));  /*allocating the new node to add*/
    nouveau->p = pp;    /*getting the new info*/
    if (ll->premier == NULL)
        ll->premier = nouveau;  /*case list is empty only list start's pointer updated*/
    else
    {
        nouveau->suivant = ll->premier; /*the next pointer of the new custommer take the old list start's pointer */
        ll->premier = nouveau;  /*list's start pointer get updated by th new node*/
    }
}

void ajouter_en_queue(personne pp, liste *ll)
{
    assert(ll);

    cellule *nouveau;
    nouveau = (cellule *) malloc(sizeof(cellule));
    nouveau->p = pp;
    if (ll->premier == NULL)
        ll->premier = nouveau;
    else
    {
        while (ll->premier->suivant != NULL)
            ll->premier = ll->premier->suivant;
        ll->premier = nouveau;
    }
}

void supprimer_element(cellule *sup_elt, liste *ll)
{
    assert(sup_elt && ll);

    if (ll->premier)
    {
        cellule *temp;
        if (sup_elt)
        {
            if (sup_elt->suivant == NULL)
            {
                free(sup_elt);
            }
            else
            {
                temp = sup_elt->suivant;
                *sup_elt = *temp;
                free(temp);
            }
        }
    }
}

void ajouter_avant(cellule *q, personne pp, liste *ll)
{
    assert(ll);

    cellule *temp;
    temp = (cellule *) malloc(sizeof(cellule));
    temp = q->suivant;
    *temp = *q;
    q->p = pp;
}

void afficher(liste ll)
{
    assert(ll.premier);
    if (ll.premier != NULL)
    {
        cellule *actuel;
        actuel = ll.premier;
        while (actuel != NULL)
        {
            printf("%s\t%d\t%s\n", actuel->p.nom, actuel->p.annee_naissance, actuel->p.tel);
            actuel = actuel->suivant;
        }/*parcouring the list and displaying every custommer till reaching adress NULL*/
    }
}

Is your keyboard playing up. The spacebar does not seem to be working — Ed Heal, Apr 02 '21 at 23:46
BTW - In this unreadable code I did notice you are casting `malloc`. The is bad. Please see https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc — Ed Heal, Apr 02 '21 at 23:49
@Ed Heal Sorry, I just dropped my code without significant comments now I edited it should be more readable and about the casting I learned at uni making it this way idk if not casting it make any difference. — Tahar Jaafer, Apr 02 '21 at 23:56
Please check the return values from `scanf`. Also `scanf("%s", p1.tel);` -> `scanf("%19s", p1.tel);` to prevent buffer overruns — Ed Heal, Apr 03 '21 at 00:08
I checked `scanf` return values should be good I am guessing the problem is in the `afficher() ` because when deleting this function from code, program doesn't crash — Tahar Jaafer, Apr 03 '21 at 00:16
`scanf("%20s", p1.nom);` Should be ` scanf("%19s", p1.nom);` - Null character — Ed Heal, Apr 03 '21 at 00:20
Yeah I edited to `%19s` in my own code and I checked by accessing the address of`p1. nom`and displaying it — Tahar Jaafer, Apr 03 '21 at 00:27
Check the others and check the return values i.e `if (scanf(..) != 1) ....` — Ed Heal, Apr 03 '21 at 00:34

John Bayko · Accepted Answer · 2021-04-03T01:13:53.753

1

In ajouter_en_tete*(), when you allocate nouveau, malloc() does not initialise any field, so suivante might not be NULL unless you explicitly set it.

More specifically, you check if (ll->premier == NULL), and:

set ll->premier = nouveau in both cases (move that out of the if block).
Only update nouveau->suivante in the second case, so it's never set to NULL. Might as well move that out of the if block too, so it will always be set (and in the case of NULL, it's set to NULL as it should be.

I'll show what I mean.

1 nouveau = (cellule *) malloc(sizeof(cellule));
2 nouveau->p = pp;
3 nouveau->suivant = ll->premier;
4 ll->premier = nouveau;

After line 1 and 2, nouveau->suivant is probably not NULL. If the list is empty, ll->premier is NULL, so in line 3 nouveau->suivant also gets set to NULL. In line 4 ll->premier is set to nouveau.

Next time through, same as before nouveau->suivant is not NULL at line 2, at line 3 nouveau->suivant is set to ll->premier which was the node allocated last time, and ll->premier is set to nouveau. The node from last time is unchanged, at this point ll->premier->suivant is the previous node, ll->premier->suivant->suivant is still NULL.

In all cases, it works this way.

edited Apr 03 '21 at 01:13

answered Apr 03 '21 at 00:13

John Bayko

746
4
7

I didn't understand you well can you demonstrate it with a portion of code, also in case `ll->premier=NULL`it means list is empty and in this case only the pointer `ll->premier` will be updated why you want me to move it out of `if` loop, in the second case when list not empty both pointers have to be updated the `nouveau->suivant` take the adress of the old `ll->premier` and then `ll->premier`get updated with the new adress to make sure the new node is on the head of list. Thanks in advance! – Tahar Jaafer Apr 03 '21 at 00:42
`nouveau->suivant` Should be set either way in the procedure – Ed Heal Apr 03 '21 at 00:44
How? by moving it out `if` loop? – Tahar Jaafer Apr 03 '21 at 00:47
Thank you so muchh, now I got what you mean `nouveau->suivant` will set to `NULL` when the list is empty so no need for `if` loop thank you so much now my program works like charm :) – Tahar Jaafer Apr 03 '21 at 01:12

my program just crash after for loop ?any solution for that?

1 Answers1