Used free() on each node but it's not emptying the list?-CodePudding

So first of all i have 2 linked lists one inside the other (like a matrix) and i made a function to delete an entire node. It seems to be freeing but when i print the value t it outputs weird characters.

Here are the structs used inside the list

typedef struct
{
    char codigo[LEN_CODIGO   1];
    char partidaID[LEN_ID   1];
    char chegadaID[LEN_ID   1];
    Data datapartida;
    Tempo horapartida;
    Tempo duracao;
    Data datachegada;
    Tempo horachegada;
    int capacidade;
    int ocupacao;
} Voo;

typedef struct r
{
    char *codReserva;
    int nPassangeiros;
    struct r *next;
} *ListaReservas;

typedef struct node
{
    Voo voo;
    ListaReservas nodeReservas; /*this is the head to a list inside this list*/
    struct node *next;
} *Node;

in the following function i pretend to delete one node and all the nodes of nodeReservas in it, like deleting an entire column of a matrix.

Node eliminaNode(Node head, char codigo[])
{
    Node n, prev;
    ListaReservas r, temp;
    for (n = head, prev = NULL; n != NULL; prev = n, n = n->next)
    {
        if (strcmp(n->voo.codigo, codigo) == 0) /*If it's the correct node*/
        {
            if (n == head)
                head = n->next;
            else
                prev->next = n->next;
            /*deletes nodeReservas*/
            r = n->nodeReservas;
            temp = r;
            while(temp != NULL)
            {
                temp = temp->next;
                free(r->codReserva);
                free(r);
                r= temp;
            }
            /*deletes the whole node*/
            free(n);
        }
    }
    return head;
}

I then use this code to tell me which reservations still exist in a node

for (r=n->nodeReservas; r != NULL; r= r->next)
    printf("%s %d\n", r->codReserva, r->nPassangeiros);

For example after adding 3 reservations to lets say Node X and deleting the Node with the reservations with eliminaNode(headofList, X). After recreating the node with that same name 'X' and printing its reservations, instead of getting a empty line i get this:

 -725147632
� ���U -725147632
@ ���U -725147632

So what is the free() freeing? Is this happening because Lista reservas is a pointer?

CodePudding user response：

free() returns the allocated block to the heap where it may be re-used for subsequent allocation requests. It does not (how could it?) modify the pointer to that block and if you retain such a pointer and re-use it after de-allocation, nothing good will happen.

What you should do is set the pointer to NULL (or a valid pointer such as that of the new next node) immediately after freeing the block so that you retain no reference to the now invalid block:

                free(r->codReserva);
                r->codReserva = NULL ;

                free(r);
                r= temp;
            }
            /*deletes the whole node*/
            free(n);
            n = NULL ;

Doing that should be a habit in C code. You could make things simpler by creating a function say:

void dealloc( void** ref )
{
    free( *ref ) ;
    *ref = NULL ;
}

Then instead of calling free( n ) you would call dealloc( &n ) for example.

There are other serious issues with this code. For example the code involving temp is somewhat over-complicated (and any code with a variable temp should raise alarm bells - you have given it scope over the entire function, and used it for more than one purpose - that is not good practice). Consider:

            r = n->nodeReservas;
            while( r != NULL)
            {
                ListaReservas new_next= r->next;

                free(r->codReserva);
                r->codReserva = NULL ;

                free(r);
                r = new_next;
            }

There new_next is very localised (literally "temporary") and named appropriately so it is clear what it is. The next problem is that having assigned the value r you do nothing with it! It is presumably n->nodeReservas that you intended to update not r? Perhaps:

            ListaReservas r = n->nodeReservas;
            while( r != NULL)
            {
                ListaReservas new_next= r->next;

                free(r->codReserva);
                r->codReserva = NULL ;

                free(r);
                n->nodeReservas = new_next;
            }

Note in each case the declaration of temporary variables at point of first use, to give the narrowest scope. Note that r is also temporary. However here it is not truly necessary - it is just a shorthand for n->nodeReservas - personally I'd eradicate it - if only to avoid exactly teh bug described above. Having multiple references to a single allocation is a recipe for bugs. Instead:

            while( n->nodeReservas != NULL)
            {
                ListaReservas new_next = n->nodeReservas->next;

                free(n->nodeReservas->codReserva);
                n->nodeReservas->codReserva = NULL ;

                free(n->nodeReservas);
                n->nodeReservas = new_next;
            }

I cannot say for sure there are not other bugs - that is just the part that had an obvious "code smell".