Vals Eman
Vals Eman

Reputation: 15

deleting all the nodes in a linked list

Anyone know what's wrong with this recrusive function? It doesn't delete all the nodes

struct contact
{
    char FirstName[41];
    char LastName[41];
    int id;
    struct contact *next;
};

void ClearList (struct contact *person)
{
    struct contact *temp = person;
    if (person == NULL) return;
    else
    {
        person = person->next;
        free(temp);
        ClearList(person);
    }
}

this is my main function

void main()
{
    struct contact *person = malloc(sizeof(struct contact));
    strcpy (person->FirstName, "John");
    strcpy (person->LastName, "Doe");
    person->id = 10;

    person->next = malloc(sizeof(struct contact));
    strcpy (person->next->FirstName, "Will");
    strcpy (person->next->LastName, "Smith");
    person->next->id = 20;
    person->next->next = NULL;

    PrintList(person);
    ClearList(person);
    PrintList(person);
}

when I call PrintList after calling ClearList it still prints out some messy stuffs, how do I fix this?

Upvotes: 2

Views: 4181

Answers (2)

Hashtag
Hashtag

Reputation: 85

I really don't like this recursive delete on your linked list. If your linked list has 100 elements you will go 100 functions deep in your stack and probably crash.

I suggest a re-write like this:

void ClearList (struct contact *person)
{
   while( person != NULL )
   {
        struct contact * temp = person
        person = person->next;
        free(temp);
   }
}

Joachim has the correct answer though. Although we have cleared the memory that person points to, "ClearList" does not have the right to set the original pointer to NULL. So either you need to make ClearList take a double pointer so it can set the pointer to NULL, or just set "person" to NULL after calling ClearList.

Double pointer example, call with ClearList(&person);

void ClearList (struct contact ** list)
{
   struct contact *person = *list;
   while( person != NULL )
   {
        struct contact * temp = person
        person = person->next;
        free(temp);
   }
   *list = NULL;
}

Upvotes: 2

Some programmer dude
Some programmer dude

Reputation: 409482

All the nodes are deleted, but you never clear any pointers. So what you're doing is dereferencing invalid pointers leading to undefined behavior.

The free function doesn't automatically set pointers to NULL.

Upvotes: 4

Related Questions