Would not deleting the head in a linked list cause a memory leak?-CodePudding

I'm currently trying to make my own destructor for my Linked List class, and I know that I can't delete the head in the destructor because curr is using it in the code above, but would not deleting the head cause memory leaks in my code? Do I even need to set head equal to null?

    ~LinkedList(){//Destructor
      Node*curr = head;
      Node* next = nullptr;

      while(curr->next != nullptr){
        next = curr->next;
        delete curr;
        curr = next;
      }
      head = nullptr;
      cout<<"Destructor called"<<endl;
    }

CodePudding user response：

while (head != nullptr) {
  Node* curr = head;
  head = head->next;
  delete curr;
}

The main thing making it "complicated" is the while condition: it should not be on a next field, the possible cause for a null pointer.

Then where you do your pointer juggling for next and delete there is some freedom of choice.

I find manipulating head immediately makes the purpose quite clear.

CodePudding user response：

I know that I can't delete the head in the destructor because curr is using it in the code above

Then what you know is wrong, because you can and must free the head node, otherwise it will be leaked if the list is not empty. Just because curr points to the head node does not mean you can't free that node. Just don't use that pointer anymore until you reassign it to point at a different valid node.

but would not deleting the head cause memory leaks in my code?

Yes.

Do I even need to set head equal to null?

It is not strictly needed, no. But it doesn't hurt anything, either. Since the object being destructed is effectively dead to the outside world, any further access to it by outside code is undefined behavior, regardless of whether you set its head member to nullptr or not.

That said, the code you have shown is fine, except for 1 small mistake:

while(curr->next != nullptr)

needs to be changed to this instead:

while(curr != nullptr)

In your original code, if the list is empty, curr will be nullptr and so accessing curr->next will be undefined behavior. And if the list is not empty, the loop will skip freeing the last node in the list where curr->next will be nullptr.

The correct code should look like this:

~LinkedList(){//Destructor
  cout << "Destructor called" << endl;
  Node *curr = head, *next;
  while (curr != nullptr){
    next = curr->next;
    delete curr;
    curr = next;
  }
}

Which can be simplified to:

~LinkedList(){//Destructor
  cout << "Destructor called" << endl;
  while (head){
    Node *next = head->next;
    delete head;
    head = next;
  }
}