Linked list destructor C++

Question

I'm learning to implement Stack using linked list. This is the node class:

class StudentInfo {
    public:
        string id, name, course;
        double GPA;
        StudentInfo *next;
};

This is the Stack class:

class StackLinkedList {
    public:
        StudentInfo* top; //pointer to point to the top node
        int size; //variable to keep the size of the stack

    //constructor
    StackLinkedList() {
        this->size = 0;
        this->top = NULL;
    }

    //destructor
    ~StackLinkedList() {
        StudentInfo *current = top;
        while (top) {
            current = current->next;
            delete top;
            top = current;
        }
    }

    //to add item into stack - push on top
    void push(StudentInfo *newStudent) {
        if (!top) {
            top = newStudent;
            return;
        }

        newStudent->next = top;
        top = newStudent;
        size++;
    }

void main() {
    StudentInfo s1("phi", "123", "computer science", 4.0);
    StudentInfo s2("abc", "123", "software engineer", 4.0);
    StudentInfo s3("zxc", "123", "business management", 4.0);

    StackLinkedList list;
    StudentInfo *ptr;
    ptr = &s1;
    list.push(ptr);
    ptr = &s2;
    list.push(ptr);
    ptr = &s3;
    list.push(ptr);

};

When I try to run unit test on push() and printAll(), everything is okay. However, after destructor() has been called, and error is showed up Debug Assertion Failed … is_block_type_valid(header-> _block_use). And the debugger triggered a breakpoint at delete top;

//destructor
~StackLinkedList() {
    StudentInfo *current = top;
    while (top) {
        current = current->next;
        delete top; //here
        top = current;
    }
}

If I put top = NULL; before delete top;, the error is gone. So, I have a little bit of confusion about the top = NULL; statement. Edit: Constructor for NodeType

 StudentInfo(string id, string name, string course, double gpa) {
        this->id = id; this->name = name; this->course = course; this->GPA = gpa; this->next = NULL;
}

Answer 1

You invoked Undefined Behavior by attempting to delete objects of automatic storage duration.

int main() {
    StudentInfo s1("phi", "123", "computer science", 4.0);
    StudentInfo s2("abc", "123", "software engineer", 4.0);
    StudentInfo s3("zxc", "123", "business management", 4.0);

    StackLinkedList list;
    StudentInfo *ptr;
    ptr = &s1;
    list.push(ptr);
    ptr = &s2;
    list.push(ptr);
    ptr = &s3;
    list.push(ptr);

};

As you can see, s1, s2, s3 are objects of automatic storage duration (aka, the compiler automatically invokes their destructors at the end of their lifetime).

Yet you pass their addresses to list, whose destructor deletes all pointers within its linked-list-detail, upon destruction.... Never call delete on a pointer to an object that wasn't created using new.

---

Some additional notes:

• void main() is illegal in C++. Are you using an older compiler? ..
• Every object should manage its resource. A std::forward_list for example manages the allocation of its nodes internally using allocators. I suggest you redesign StackLinkedList to manage its nodes internally, so that clients will never bother about lifetimes.
• You should read up on the Rule of Three, and The Rule of Five
• There are some other bugs in your code, I've not touched.