CODEWITHSUNDEEP
c++c++11pointersmemory-leaksnew-operator
Nicolas H
Nicolas H

Reputation: 545

C++: potential memory leak in linked list

I'm writing a class of linked list, I feel that for the member function that used to delete specific element might cause the memory leak. The code is below.

struct node
{
    int data;
    node *next;
};

class linked_list
{
private:
    node *head,*tail;
public:
    linked_list()
    {
        head = NULL;
        tail = NULL;
    }
    void add_node(int n)
    {
        node *tmp = new node;
        tmp->data = n;
        tmp->next = NULL;

        if(head == NULL)
        {
            head = tmp;
            tail = tmp;
        }
        else
        {
            tail->next = tmp;
            tail = tail->next;
        }
        
    }
    void DelElem(int locat)
    {
        int j{1};
        node* tmp = new node;     
        if (locat == 1)
        {      
            tmp  = head->next;
            head = tmp;
            delete tmp;
        }
        else
        {
            node* n = head;
            while (j < locat - 1)
            {
                n = n->next;
                j++;
            }
            tmp = n->next;
            n->next = tmp->next;
            delete tmp; 
        } 
    }

For function 'DelElem', I firstly created a pointer tmp by new operator. However, I assign different address for it which means I lost the original one at the initialization.

How can I fix this problem?

Upvotes: 0

Views: 339

Answers (1)

Abhishek Dutt
Abhishek Dutt

Reputation: 1437

There are few issues with your instance of code, I have corrected that:-

  1. As pointed by others, you are not required to use `new` keyword to declare a pointer.
  2. When one tries to delete the first node of the linked list, then according to your code, it will delete the second node, because of the following 
    tmp  = head->next;
head = tmp;
delete tmp;

    Here, tmp is initially pointing to second node,because head->next refers to 2nd node. So instead of that, it should have been like this:- 

    tmp = head;
head = head->next;
delete tmp;

    Now, tmp will point to 1st node, in second line, head will point to 2nd node, and then the first node, pointed by tmp gets deleted.

Here is the corrected version of code:-

struct node {
    int data;
    node* next;
};

class linked_list {
private:
    node *head, *tail;

public:
    linked_list()
    {
        head = NULL;
        tail = NULL;
    }
    void add_node(int n)
    {
        node* tmp = new node;
        tmp->data = n;
        tmp->next = NULL;

        if (head == NULL) {
            head = tmp;
            tail = tmp;
        }
        else {
            tail->next = tmp;
            tail = tail->next;
        }
    }
    void DelElem(int locat)
    {
        int j{ 1 };
        node* tmp;
        if (locat == 1) {
            tmp = head;
            head = head->next;
            delete tmp;
        }
        else {
            node* n = head;
            while (j < (locat - 1)) {
                n = n->next;
                j++;
            }
            tmp = n->next;
            n->next = tmp->next;
            cout << tmp->data;
            delete tmp;
        }
    }
};

Upvotes: 1

Related Questions