chlyr314
chlyr314

Reputation: 19

Memory leak in linked list using dynamic allocation for deleting elements

I have been working on a linked list problem, using two pointers to the head and the tail of the list respectively. The issue I have is the following:

When I want to remove from the front or the back of the list, I get memory leaks when the corresponding class methods are implemented using temporary dynamically allocated node pointer and I can't find out what exactly is the problem that is causing the leak.

#include <iostream>

class Node {
   public:
      Node():data(0),ptrNode(nullptr){};
      ~Node() {};

      // declaring the getters and setters
      float getData() const {return data;};
      void setData(float d) {data = d;};
      Node* getNodePtr() const {return ptrNode;};
      void setNodePtr(Node* n){ptrNode = n;};

   private:
      float data;
      Node* ptrNode;
    };


class List {

   private:
      Node * ptrHead;
      Node * ptrTail;


   public:
      List():ptrHead(nullptr),ptrTail(nullptr) {};
      ~List() {};

      void insertAtFront(float x) {
               Node * temp = new Node();
               temp->setData(x);

               if (ptrHead == nullptr) {
                     ptrHead = temp;
                     ptrTail = temp;
               } else {
                     temp->setNodePtr(ptrHead);
                     ptrHead = temp;
               }
      };

      void insertAtBack(float x) {
               Node * temp = new Node();
               temp->setData(x);

               if (ptrHead == nullptr) {
                     ptrHead = temp;
                     ptrTail = temp;
               } else {
                     ptrTail->setNodePtr(temp);
                     ptrTail = temp;
               }
      };

      void removeFromFront() {

               if (ptrHead==nullptr) {  // if list is empty
                    std::cout << "List is already empty" << std::endl;
                    return;
               }

               if (ptrHead==ptrTail) { // if list has one element
                    delete ptrHead;
                    ptrHead = nullptr;
                    ptrTail = nullptr;
                    return;
               }
               // assign current Head to temp, assign next node to head 
               // delete temp
               Node * temp = new Node();
               temp = ptrHead;
               ptrHead = ptrHead->getNodePtr();
               delete temp;
               temp = nullptr;
      };

      void removeFromBack() {

               if (ptrHead==nullptr) {  // if list is empty
                    std::cout << "List is already empty" << std::endl;
                    return;
               }

               if (ptrHead==ptrTail) { // if list has one element
                    delete ptrHead;
                    ptrHead = nullptr;
                    ptrTail = nullptr;
                    return;
               }

               // create two temp Node pointers for this one
               Node * sec2last = new Node();
               Node * last = new Node();

               sec2last = ptrHead;
               last = ptrTail;

               // locate second to last element and assign it to sec2last
               while (sec2last->getNodePtr() != ptrTail) {
                     sec2last = sec2last->getNodePtr();
               }

               ptrTail = sec2last;
               ptrTail->setNodePtr(nullptr);

               delete last;
               delete sec2last;
               last = nullptr;
               sec2last = nullptr;
      };

   };

I run the following in main():

// global function that dynamically allocates memory in its scope
// for a linked list
void generateList(int x) {
   if (x<=0) {
      cout << "Give a positive integer!" << endl;
      return;
   }

   List * aList = new List();

   for (int i = 0;i<x;i++) {
      aList->insertAtFront(i+1);
   }

   aList->removeFromBack();  // this causes leaks

   delete aList;
}

// MAIN
int main() {

   for (int i = 0;i<80000000;i++)    
       generateList(1);   // just repeatedly generate dynamic list 
                          // with one element
   return 0;
}

I should point out that if we don't use dynamically allocate memory for the temporary nodes within the removeFromFront() and removeFromBack() methods, then the program works fine. But, like I said, It kills me that I can't see why we have leaks with the code above.

Thanks!

Upvotes: 1

Views: 203

Answers (1)

David Schwartz
David Schwartz

Reputation: 182753

           Node * temp = new Node();
           temp = ptrHead;

This is a memory leak right here. You allocate a new Node and store a pointer to it in temp. Then you overwrite that pointer and leak the node you just allocated.

           // create two temp Node pointers for this one
           Node * sec2last = new Node();
           Node * last = new Node();

           sec2last = ptrHead;
           last = ptrTail;

And here you do it again. Why are you allocating new Nodes here?

Upvotes: 5

Related Questions