C++ exception 0xC00000FD: Stack overflow (parameters: 0x00000001, 0x00C02F30)

Question

This is for a school project. I have a class called buddy and a linked list buddyList. While debugging, I encountered some nullptr exceptions that I think I fixed correctly, but last I tried to debug, I got this exception. What can I do to fix this? Keep in mind some of this code was provided via the instructor and to goal of the project was to add specific methods to the existing classes. Thanks!

//buddy.h

#pragma once
#include <iostream>
#include <iomanip>
#include <string>
using namespace std;

class buddy {
friend class buddyList;

public:
    buddy() {
        first = last = phone = ""; next = NULL;
        cout << "Buddy allocated\n";
    }

~buddy() { 
    if (next != NULL) {
        delete next;
        cout << "Buddy " << first << " " << last << " deallocated!\n";
    }
}

void set(string fname, string lname, string tele) {
    first = fname;
    last = lname;
    phone = tele;
}
void print() {
    cout << left << setw(15) << last << setw(15) << first << "  " << phone << endl;
}
private:
    string first;
    string last;
    string phone;
    buddy * next;   
};

//buddyList.h

#pragma once
#include <string>
#include "buddy.h"
using namespace std;

class buddyList {
public:
    buddyList();
    ~buddyList();
    void add(string fname, string lname, string phone);
    void print();
    int drop(string fname, string lname);
    void sort();
    void read();
private:
    buddy * head;
    buddy* search(string fname, string lname);
    buddy* maxByName();
    void remove(buddy * r);
    void add(buddy * n);
};

//buddyList.cpp

#include "stdafx.h"
#include <iostream>
#include <fstream>
#include "buddyList.h"



//------------------------------------------------------------------
//                    constructor and destructor
//------------------------------------------------------------------
buddyList::buddyList() {
    head = NULL;
    cout << "List allocated!\n";
}

buddyList::~buddyList() {
    if (head != NULL)
        delete head;
    cout << "List destroyed!\n";
}

//------------------------------------------------------------------
//                            add
//------------------------------------------------------------------
void buddyList::add(string fname, string lname, string phone) {
    buddy * b = new buddy;
    b->first = fname;
    b->last = lname;
    b->phone = phone;
    add(b);
}

void buddyList::add(buddy * p) {
    p->next = head;//Error here
    head = p;
}

  //------------------------------------------------------------------
  //                            print
  //------------------------------------------------------------------
void buddyList::print() {
    cout << "\nBuddy List: =========================================\n";
    buddy * p = head;
    while (p != NULL) {
        p->print();
        p = p->next;
    }
    cout << "======================================================\n\n";
    delete p;
    p = NULL;
} 
//------------------------------------------------------------------
//                            search
//------------------------------------------------------------------
buddy* buddyList::search(string fname, string lname) {
    buddy * p = head;
    while (p != NULL) {
        if (p->first == fname && p->last == lname)
            return p;
        p = p->next;
    }
    delete p;
    p = NULL;
    return NULL;
}
//------------------------------------------------------------------
//                            read
//------------------------------------------------------------------
void buddyList::read() {
    ifstream f;
    f.open("buddyList.txt");
    if (f.fail()) {
        cout << "Error opening input file!\n";
        return;
    }

    string fname, lname, tele;
    while (!f.eof()) {
        f >> fname >> lname >> tele;
        add(fname, lname, tele);
    }
    f.close();
}
//------------------------------------------------------------------
//                            maxByName
//------------------------------------------------------------------
buddy* buddyList::maxByName() {
    buddy * p = head;
    if (p == NULL)
        return NULL;
    buddy * q = p->next;
    while (q != NULL) {
        if (p->last > q->last) {
            p = q;
        }
        else if (p->last == q->last)
            if (p->first > q->first)
                p = q;
        q = q->next;
    }
    return p;
}
//------------------------------------------------------------------
//                            remove
//------------------------------------------------------------------
void buddyList::remove(buddy * r) {
    if (head == NULL)
        return;
    if (r == NULL)
        return;
    if (r == head)
        head = head->next;
    else {
        buddy * b4 = head;
        while (b4->next != r && b4 != NULL) {
            b4 = b4->next;
        }
        if (b4 == NULL)
            return;
        b4->next = r->next;
    }
    r->next = NULL;
    delete r;
    r = NULL;
}
//------------------------------------------------------------------
//                            drop
//------------------------------------------------------------------
int buddyList::drop(string fname, string lname) {
    buddy * p = search(fname, lname);
    if (p == NULL)
        return -1;
    else {
        remove(p);
        return 0;
    }
}
//------------------------------------------------------------------
//                            sort
//------------------------------------------------------------------
void buddyList::sort() {
    buddyList tempList;
    buddy * p = head; 
    while (p != NULL) {
    buddy * q = maxByName();
    remove(q);
    tempList.add(q);
}
delete p;
head = tempList.head;
tempList.head = NULL;

}

Answer 1

A stack overflow is commonly caused by recursion gone mad. By that, I mean you're doing something recursive such as delete-ing the next (in a list) element in the destructor of the current element but, for some reason, the integrity of the list is compromised.

Since your buddy class only contains information about an element in the list, you should probably be looking at the list code itself. This is, rather tantalisingly, indicated in the line:

friend class buddyList;

There's nothing inherently wrong with a destructor for an item first cleaning up all items it "owns" (such as the rest of the list) but you have to do it properly. From the code you've given, it looks okay, but everything is predicated on the fact that buddyList is working as expected.

---

And, in fact, now that you've added some buddyList code, that appears to be the exact problem. Looking at your code for adding an item:

buddy * b = new buddy;
b->first = fname;
b->last = lname;
b->phone = phone;
add(b);
delete b;          // Hmm!!

You have a serious problem here. You allocate a new buddy, add a pointer to it into the list, then free the memory that it refers to. That's not going to end well if, for example, that memory gets reused for something else (like another node in the list) while still being referenced by the list.

---

So, here's my advice, such as it is.

1. Separate the responsibilities correctly. No given node should ever have to concern itself with the other nodes in the list. The list itself should be responsible for cleaning itself up.

2. Don't use recursion for cleanup. The ideal use case for recursion is where the problem-space is reduced quickly. For example, in a binary search, you remove half the remaining problem space at each level of recursion. With a list of a million nodes, removing one node per level, you're almost certainly going to overflow your stack with a million separate frames.

3. If you're not using C++ smart pointers^(a), learn how to follow the flow of ownership and don't "give" an object to some other object then immediately make it unusable :-)

---

For example, here's an implementation that, while still using raw pointers, addresses the first two points above:

#include <iostream>
#include <memory>

class Node {
public:
    Node(const std::string &str): m_str(str), m_next(nullptr) {}

private:
    friend class NodeList;
    std::string m_str;
    Node *m_next;
};

class NodeList {
public:
    NodeList(): m_head(nullptr), m_tail(nullptr) {};
    ~NodeList() { clear(); }

    void print() {
        Node *node = m_head;
        std::cout << "List:";
        while (node != nullptr) {
            std::cout << " " << node->m_str;
            node = node->m_next;
        }
        std::cout << "\n";
    }

    void add(const std::string &str) {
        auto newNode = new Node(str);
        if (m_head == nullptr) {
            m_head = m_tail = newNode;
        } else {
            m_tail->m_next = newNode;
            m_tail = newNode;
        }
    }

    // List is responsible for cleaning itself up, and
    // it does it iteratively to mimimise chance of
    // stack blowout.

    void clear() {
        while (m_head != nullptr) {
            Node *save = m_head->m_next;
            delete m_head;
            m_head = save;
        }
        m_tail = nullptr;
    }

private:
    Node *m_head, *m_tail;
};

int main() {
    NodeList list;
    list.print();
    list.add("paxdiablo"); list.add("george"); list.add("bob");
    list.print();
    list.clear();
    list.print();
}

As expected, the output is:

List:
List: paxdiablo george bob
List:

---

^(a) There are very few cases nowadays where you should ever use the new or delete keywords. They're okay if you totally understand and control the ownership of an object but, if there's any doubt, modern C++ provides smart pointers for making it a lot easier to manage.