Drew Kay
Drew Kay

Reputation: 23

Freeing a singly linked list in c

I've come across what seems to be a strange problem when implementing a singly linked list. I call a list_destroyer and pass the pointer to the head of the list, however when the method returns, the pointer that is passed still points to a full list. I don't believe I've passed a struct anywhere.

Here is my struct list, and typedef

typedef struct list list_t
struct list{
    void* datum;
    list_t* next;
};

And here is the code that is causing problem

void list_destroy(list_t *head){
    list_t *destroy = head;
    while(head){
        //printf("%d \n", list_size(head));
        head = head->next;
        free(destroy);
        destroy = head;
    }
    //printf("%d \n", list_size(head));
    head = NULL;
    //printf("%d \n", list_size(head));
}

The list_size functions have been commented out because they aren't necessary, but I use them to see the output of the code. The printf output shows that the size is decreasing. The two printf's surrounding the "head = NULL;" statement both print a size of zero. This is also confirmed with gdb. However, when I have this code (following) calling list_destroy, the pointer that is passed through is unchanged.

int main(){
    list_t *test = NULL;
    int a = 1;
    int b = 2;
    list_append(test,&a);
    list_append(test,&b);
    printf("%d \n", list_size(test));
    list_destroy(test);
    printf("%d \n", list_size(test));
}

I still get the printf above and below the list_destroy to both output 2. I haven't initialized a new list_t anywhere, so I don't see how the printf after the list_destroy would still output 2, (especially when the printf within the list_destroy says the list_t* passed in has a size of 0 at the end.

Upvotes: 1

Views: 215

Answers (1)

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726859

however when the method returns, the pointer that is passed still points to a full list.

That's incorrect: when the function returns, the pointer points to what used to be a full list. Chances are, your system would let you traverse the entire list without a break. However, dereferencing this pointer after the call is undefined behavior, so the same code could crash on other systems.

The problem has a name - head becomes a dangling pointer.

Fixing the problem is easy - pass a pointer to pointer, and set it to NULL upon completion:

void list_destroy(list_t **headPtr){
    list_t *head = *headPtr;
    list_t *destroy = head;
    while(head){
        head = head->next;
        free(destroy);
        destroy = head;
    }
    *headPtr = NULL;
}

Upvotes: 3

Related Questions