I have a customer node and an item node structure and I'm trying to test them. I make a customer node and add some items to its basket. However, when I'm deleting the nodes, the program mostly crashes. I'm trying to first delete the basket and then delete the customer node. The code sometimes runs fine but sometimes crashes. Why do you think it crashes? Your help is appreciated.

My code is as follows:

#include <iostream>
#include <string>

struct itemNode
{
    int id;
    char *itemName;
    int amount;

    itemNode *next;

    itemNode(const int id, const char *name, const int amount, itemNode *const next = nullptr)
        : id(id), amount(amount), next(next)
    {
        this->itemName = new char[strlen(name)];
        strcpy(this->itemName, name);
        std::cout << "Item " << this->itemName << " constructed." << std::endl;
    };

    ~itemNode()
    {
        std::cout << "Deleting " << itemName << std::endl;
        delete[] this->itemName;

        if (this->next != nullptr)
        {
            delete this->next;
        }
    }
};

struct customerNode
{
    int id;
    char *fullName;
    itemNode *basket;

    customerNode *next;
    customerNode *previous;

    customerNode(const int id, const char *name, customerNode *const previous, customerNode *const next)
        : id(id), previous(previous), next(next)
    {
        this->fullName = new char[strlen(name)];
        strcpy(this->fullName, name);
        std::cout << "Customer " << this->fullName << " constructed." << std::endl;
        this->basket = nullptr;
        this->next = next;
        this->previous = previous;
    };

    ~customerNode()
    {
        std::cout << "Deleting " << fullName << std::endl;

        delete[] this->fullName;
        if (this->basket != nullptr)
        {
            delete this->basket;
        }
    }
};

int main()
{
    customerNode *customer1 = new customerNode(14235, "Georges Politzer", nullptr, nullptr);

    itemNode *item3 = new itemNode(5355, "Pen", 2, nullptr);
    itemNode *item2 = new itemNode(1351, "Folder Case", 1, item3);
    itemNode *item1 = new itemNode(4412, "Paper Clips", 20, item2);

    customer1->basket = item1;

    delete customer1->basket;
    customer1->basket = nullptr;

    delete customer1;

    return 0;
}

CodePudding user response：

Error here

    this->itemName = new char[strlen(name)];
    strcpy(this->itemName, name);

should be

    this->itemName = new char[strlen(name)   1];
    strcpy(this->itemName, name);

The 1 is required for the nul terminator that C strings have. Without it you are corrupting the heap. Classic symptom of a corrupt heap is a crash when you free memory. Of course the better solution would be to use std::string. std::string is both simpler and more robust. There is no good reason not to use it.

Additionally in professional code your linked lists would probably be replaced by std::vector. But I guess you have no choice over that.

You should also have a close read of this link. Your current code isn't just breaking the rule of three, it's murdered it and buried the corpse in a shallow grave.