CODEWITHSUNDEEP
cpointerslinked-liststructurefree
Romin Tomasetti
Romin Tomasetti

Reputation: 21

Free a structure.... I'm not sure if I do it right

My program manage a linked list of structure.

Here is my struc:

typedef struct wagon wagon;
typedef struct wagon{
    wagon *next;
    marchandise_ptr *liste;
    double volume_courant;
}train_ptr;

Where wagon *next is a pointer to the next "cell" of my linked list, marchandise_ptr *list is a pointer to another linked list. To free my strucure, I've proceed as followed:

In int main():

train_ptr *train=creer_un_train(...)//so train is the beginning of my linked list
liberer_train(&train);

My functions are:

Creates a "wagon" 

wagon *creer_wagon(marchandise_ptr *liste,double volume_courant){ //it creates a wagon
    assert(volume_courant>=0);
    wagon *wag=malloc(sizeof(wagon));
    if(wag==NULL)
        return NULL;
    wag->next=NULL;
    wag->volume_courant=volume_courant;
    wag->liste=liste;
    return wag;
}

Add the created "wagon" at the end of my chained list:

train_ptr *ajouter_wagon_a_la_fin_du_train(train_ptr *train,double volume_courant, marchandise_ptr *liste){
    wagon *wag=creer_wagon(liste,volume_courant);
    if(wag==NULL)
        return NULL;
    if(train==NULL)
        train=wag;
    else{
        train_ptr *wag_current=train;
        while(wag_current->next!=NULL)
            wag_current=wag_current->next;
        wag_current->next=wag;
        }
    return train;
}

Creates a train:

train_ptr *creer_un_train(unsigned int nombre_de_wagons,marchandise_ptr *liste){
    assert(nombre_de_wagons>=0);
    int i;
    train_ptr *train=NULL;

    for(i=0;i<nombre_de_wagons;i++){
        train=ajouter_wagon_a_la_fin_du_train(train,rand()%10,liste);
        if(train==NULL)
            return NULL;
        }
    return train;
}

Free a train:

void liberer_train(train_ptr **train){
    train_ptr *p_current = *train;
    while(p_current!=NULL){
                *train = p_current->next;
                p_current->next=NULL;
                free(p_current->liste);
                free(p_current);
                p_current = *train;
    }
}

P.S.: liste is a pointer to the beginnig of a linked list:

typedef struct marchandise marchandise;
typedef struct marchandise{
    double volume;
    double volume_total;
    char nom;
    marchandise *suivant;
}marchandise_ptr;

Thanks for your attention! (and sorry for my english, I'm not a native speaker.... :D )

Upvotes: 0

Views: 146

Answers (1)

Iharob Al Asimi
Iharob Al Asimi

Reputation: 53006

From your creer_wagon function, it doesn't seem like the liste should be freed by the liberer_train function, because it's not allocated by the creer_wagon function.

Following that logic, the function where you do the call to creer_wagon should be responsible for the liste member, because you will have a valid pointer to it in the scope of the caller function, and you risk a double free of the liste member.

If each train needs only to have a refrence to a liste but doesn't need to modify it you could define your struct like this

typedef struct wagon{
    wagon *next;
    const marchandise_ptr *liste;
    double volume_courant;
}train_ptr;

this will prevent accidentally trying to modify or free the liste member.

This design makes sense if many trains could point to the same liste, though it increases the responsabilities of the struct wagon users, because they should take care of the memory management for the liste member. If this is the case, I double recommend the const qualifier.

In my opinion this is a valid way of reasoning regardles of whether liste was allocated by you or not.

I suggest you fix the liberer_train function like this

void liberer_train(train_ptr **train) {
    train_ptr *p_current = *train;
    train_ptr *suivant   = NULL;
    while (p_current != NULL) {
        suivant = p_current->next;
        free(p_current);        
        p_current = suivant;
    }
    /* make the pointer NULL, so it's not a dangling pointer in the caller function anymore. */
    *train = NULL; 
}

and also, i suggest you change

typedef struct wagon train_ptr;

to 

typedef struct wagon *train_ptr;

or

typedef struct wagon train;

for example, because the suffix _ptr makes one thinkg that in train_ptr x; x is a pointer, while it's not.

Upvotes: 1

Related Questions