butterflyknife
butterflyknife

Reputation: 1584

Setting a pointer to `nullptr` doesn't prevent double deletes?

I have a linked list implementation, in which I have a LinkedList class which contains a member called head_ which is a pointer to a LinkedListNode. The LinkedListNode obviously contains a member called next_, which I can use to step through the list.

I have a destructor ~LinkedList, which starts at the head_ and steps through the list delete-ing all the pointers to LinkedListNode as it goes:

~LinkedList() {
  LinkedListNode *thru = head_;
  while (thru != nullptr) {
    LinkedListNode* toDelete = thru;
    thru = thru->next;
    delete toDelete;
    toDelete = nullptr; // LINE *
  }

This all works fine. My question is about what happened when I thought I'd be safe and stick in a destructor for the LinkedListNode class like so:

~LinkedListNode() { 
    if (next_ != nullptr) // LINE **
        delete next_;
}

This is giving me a stack overflow error, which I'm guessing is because I'm trying to double delete the next_ pointers, once from the LinkedList and once from the LinkedListNode.

My question is: shouldn't setting the toDelete pointer to nullptr in line (*), plus the if() check in line (**), prevent the double delete error?

Upvotes: 0

Views: 99

Answers (1)

Yunfei Chen
Yunfei Chen

Reputation: 626

You problem is the following:

~LinkedListNode() { 
    if (next_ != nullptr) // LINE **
        delete next_;
}

Imagine you had a linked list 5, 4 , 3, 2, 1 and you have your head to be 5, your next would be 4, 3, and so on. When you call the function, what you will do is delete 4, and everything else will stay in your system, there is no way for you to delete 3, 2, 1 and so on, you deleted the only thing pointing to them, you have no access to them anymore. Many people are suggesting a recursive solution, well honestly it wouldn't work if you do it in the destructor if you really wanted to you would need another function like so:

void deleteLinkedList(Node* head){
    if(head == nullptr)return;
    Node* temp = head->next;
    delete head;
    deleteLinkedList(temp);
}

Upvotes: 2

Related Questions