Linked list pop_back implementation ignores pointer-to-nullptr

Question

Let's assume I'm only allowed to use new and delete and no smart pointers in my implementation. I am not using sentinel nodes.

Below is my implementation for pop_back:

void sLinkedList::pop_back()
{

  if(begin == nullptr)
  {
    std::cerr << "There is nothing to pop." << std::endl;
  }
  else
  {
    node* nodeToDelete = begin;
    while(nodeToDelete->next != nullptr)
    {
      nodeToDelete = nodeToDelete->next;
    }
    delete nodeToDelete;
    nodeToDelete = nullptr;
    --mSize;
  }

}

What it does is it creates a pointer to a node nodeToDelete and it iterates through the whole list until it reaches the last node (the node that points to nullptr). Then I delete that last node, set it equal to nullptr, and everything should be fine because the node before this (previously the second-to-last node) now points to a nullptr, marking the end of the list.

When I ran main with the following instructions:

int main()
{
  sLinkedList newList;
  std::cout << "Length is " << newList.length() << std::endl;
  newList.print();
  newList.push_front(4);
  std::cout << "Length is " << newList.length() << std:: endl;
  newList.print();
  newList.pop_back();
  std::cout << "Length is " << newList.length() << std::endl;
  newList.print();
}

I get the output:

Length is 0
The list is empty.
Length is 1
4
Length is 0
4 // should print "There is nothing to pop."

Adding another pop_back directly after the first results in an Aborted (core dumped) signal.

Why doesn't the idea work?

Answer 1

Okay, but does it?

When you assign nodeToDelete = nodeToDelete->next; you're saying that your local pointer nodeToDelete now refers to the same location in memory as does nodeToDelete->next. There is no other relationship between the two pointers. So when you then assign nodeToDelete = nullptr; you're actually doing nothing: that local pointer is never used again, so it doesn't particularly matter what region of memory it points to.

Unfortunately, the should-be final node still points to where it used to, even though you deleted that node, and so your next pop_back call happily iterates into inaccessible memory.

I'm assuming this is homework so I probably shouldn't rewrite it for you. But you can fix it by actually modifying the should-be last node, rather than just your local copy of its next pointer.

Of course, if this is a real project, std::forward_list might help. :v