Confusion with delete functionality in c++

Question

So I wrote this code in c++

#include "ContactBook.hpp"

int main(){

    std::string name;
    ContactList *cl1 = new ContactList();

    while(true){

        std::cout << "Enter a name or press Q to quit" << std::endl;
        std::cin >> name;

        if(name=="Q"||name=="q")
            break;
        else{
            cl1->addToHead(name);
        }
    }

    cl1->print();
    delete cl1;

    return 0;
}

My header file definition ->

#ifndef ContactBook_hpp
#define ContactBook_hpp

#include <iostream>

class Contact{
    friend std::ostream &operator<<(std::ostream &os, const Contact &c);
    friend class ContactList;
public:
    Contact(std::string name);
private:
    std::string name;
    Contact* next;
};

class ContactList{
public:
    ContactList();
    void addToHead(const std::string &name);
    void print();
private:
    Contact *head;
    static int size;
};

#endif

Now here's my header files function definition. ContactList and Contact are two classes. Contact list is a friend class of Contact.

#include "ContactBook.hpp"

Contact::Contact(std::string name){
    this->name = name;
    next = NULL;
}
std::ostream &operator<<(std::ostream &os, const Contact &c){
    os << "Name: " << c.name << std::endl;
    return os;
}

ContactList::ContactList():head(NULL){};
int ContactList::size = 0;

void ContactList::addToHead(const std::string &name){

    Contact *newOne = new Contact(name);

    if(head==NULL){
        head = newOne;
    }
    else{
        newOne->next = head;
        head = newOne;
    }
    ++size;

}

void ContactList::print(){

    Contact *temp;
    temp = head;

    while(temp!=NULL){
        std::cout << *temp;
        temp = temp->next;
    }
}

The issue is whenever I add

delete newOne;

after ++size in the 3rd code snippet in addToHead definition.

I end up with an infinite loop on odd inputs of names (except 1)! I just cant understand why this is happening ! Some knowledge on this would be really appreciated :D !

Why would you want to delete the node you just added? This will leave you with some garbage memory at the front of your list. — BoBTFish, Jul 26 '17 at 19:00
Does the line `ContactList *cl1 = new ContactList();` compile? — Ed Heal, Jul 26 '17 at 19:01
Obligatory mention of [std::unique_ptr](http://en.cppreference.com/w/cpp/memory/unique_ptr) to do the memory management for you. — 0x5453, Jul 26 '17 at 19:02
If you delete that node, you are deleting the head in your list. In `main`, after `addToHead`, you call `print`. This function attempts to loop over your lists, starting with your deleted head. Using a deleted object is undefined behavior, so anything can happen. This includes looping forever. — François Andrieux, Jul 26 '17 at 19:02
@SergeyA Even with linked lists, then you could run into stack overflow issues but it would work for small lists. For larger lists you’d still need to write a destructor to avoid that, but it would be easier to make sure you don’t accidentally do something wrong. — Daniel H, Jul 26 '17 at 19:05
@SergeyA I'm curious as to why you wouldn't recommend `std::unique_ptr` for linked lists? — François Andrieux, Jul 26 '17 at 19:09
@FrançoisAndrieux, because you'd have to do complex dance to remove the node anyways, and at this time, you might as well delete the thing manually. — SergeyA, Jul 26 '17 at 19:48
Thank you guys :'D ! Just one query ! Since I created a newPtr dynamically, would it automatically be deallocated after the function addToHead() ends ? — Shubham Anand, Jul 27 '17 at 06:19
@SergeyA By complex dance, do you mean reassigning the `next` pointers? You have to do that either way. I don't understand the reasoning behind "might as well delete the thing manually". Is it that you see no value in using `std::unique_ptr` and consider it more complex than `delete`? — François Andrieux, Jul 27 '17 at 13:01
@FrançoisAndrieux, the beauty of unique_ptr comes from the fact that you need not do anything. If you need to do something, not so much. Also, consider double-linked lists. — SergeyA, Jul 27 '17 at 14:30
@SergeyA It's still free to use `unique_ptr` in this case. I haven't heard any reason why it should be avoided here. It saves you from deleting it yourself, avoids mistakes, improving exception safety and simplifying some operations. For example, consider clearing the list. — François Andrieux, Jul 27 '17 at 14:43
@FrançoisAndrieux, doesn't really prevent you at all. Imagine what an automatic destructor of double-linked list where the pointers are ` `std::unique_ptr<>` would do. How about move construtor for the same? Like it? — SergeyA, Jul 27 '17 at 14:50
@SergeyA For a double linked list, you can just have `unique_ptr` next and raw pointer prev. In that case each node is owned by the previous. You are changing the subject. The subject here is your original comment which opposes recommending `unique_ptr`, which was made in the context of a question about a single linked list. — François Andrieux, Jul 27 '17 at 14:55
@FrançoisAndrieux, if you know any implementation of standard library which uses `unique_ptr` for nodes in `std::list`, give me a shout and we will continue this discussion. — SergeyA, Jul 27 '17 at 15:04

HatsuPointerKun · Accepted Answer · 2017-07-26T19:25:20.443

Here, in your addToHead :

Contact *newOne = new Contact(name);

if(head==NULL){
    head = newOne;
}
else{
    newOne->next = head;
    head = newOne;
}
++size;

Can be wrote like this :

Contact *newOne = new Contact(name);

newOne->next = head; // if (head==NULL) newOne->next=NULL else newOne->next=head;
head = newOne;
++size;

You will assign the value of newOne to head.

But if you add delete, like you said, after ++size, head will point to something that is deleted.

What happen in your print method is that you dereference something deleted. What happen when you dereference something deleted is undefined behavior, which can cause strange output, or crashes.

You may want to use smart pointers to avoid accessing deleted memory and memory leaks.

Dude Thank you so much ! I just realised the problem :D – Shubham Anand Jul 27 '17 at 06:17 — Shubham Anand, Jul 27 '17 at 06:17

Confusion with delete functionality in c++

1 Answers1