C++: delete[] error, pointer not allocated

Question

I'm working on a program for a lab, and I need some help with the memory management. I'm new to C++ as a whole, and while I have experience in other languages, dynamic memory management is confusing me. Also, because this is for a lab, I can't use std::vector or smart pointers, just arrays and pointers.

First, there's a "Vehicle" class, with some attributes describing it (such as make, model, etc). Next, there's a "Showroom" class, which contains an array of Vehicles:

Vehicle * m_vehicles;

And later, in the constructor:

m_vehicles = new Vehicle[m_maxCapacity];

Next there's a "Dealership" class, which has an array of Showrooms:

Showroom * m_showrooms;
...
m_showrooms = new Showroom[m_maxCapacity];

My main() method creates some vehicles, adds those to showrooms, and then adds those to a dealership. Everything works without issue until the program ends. When the program finishes and the objects are deleted, some problems come up. As the dealership was created last, it is deleted first. It's destructor calls delete[] m_showrooms; , and the destructor in the Showroom class calls delete[] m_vehicles;. So, I expect that when the program ends, the OS deletes the dealership object, which removes the showroom objects, which in turn remove the vehicle objects.

However, it doesn't work properly. The lab requires that we use a helper that has been provided which marks memory leaks as errors. When my main() creates a showroom, adds vehicles to it, creates a dealership, and then adds the showroom to the dealership, when the program ends the memory leak detector gives an error, saying:

delete[] error: pointer was not allocated!

I get this error once for every showroom I add to the dealership. Also, there's another bizarre error: I get a segfault when the program ends if the dealership contains any two showrooms which have the same capacity.

If I say "screw it" and remove all of the delete[]'s from the destructors, the program runs without a segfault, but the memory leak program detects the leak and prevents me from moving on.

What am I doing wrong? Do I have some sort of fundamental misunderstanding of dynamic memory in C++? I read that every new[] should match with a delete[], and that's what I have. I've already asked my TA about this, and she didn't have any idea about how to solve it.

EDIT: Here's some relevant code:

main.cpp:

//array of vehicles to store
Vehicle vehicles[] =
        {
                Vehicle("Ford", "Mustang", 1973, 9500, 113000),
                Vehicle("Mazda", "CX-5", 2017, 24150, 5900),
                Vehicle("Dodge", "Charger", 2016, 18955, 9018),
                Vehicle("Telsa", "Model S", 2018, 74500, 31),
                Vehicle("Toyota", "Prius", 2015, 17819, 22987),
                Vehicle("Nissan", "Leaf", 2016, 12999, 16889),
                Vehicle("Chevrolet", "Volt", 2015, 16994, 12558),
         };
    // Showrooms to store the vehicles
        Showroom showroom("Primary Showroom",2);
        showroom.AddVehicle(&vehicles[0]);
        showroom.AddVehicle(&vehicles[1]);
        //showroom.AddVehicle(&vehicles[2]);

        Showroom secondary("Storeroom 2",4);
        secondary.AddVehicle(&vehicles[3]);
        secondary.AddVehicle(&vehicles[4]);
        secondary.AddVehicle(&vehicles[5]);
        secondary.AddVehicle(&vehicles[6]);

        // A "parent" object to store the Showrooms
        Dealership dealership("Dealership",2);
        dealership.AddShowroom(&showroom);
        dealership.AddShowroom(&secondary);
        //displays the showrooms and their contents
        dealership.ShowInventory();

Relevant functions from Vehicle.cpp:

Vehicle::Vehicle(std::string mk, std::string md, int yr, int pr, int ml) {
    make = mk;
    model = md;
    year = yr;
    price = pr;
    miles = ml;

}
Vehicle::~Vehicle() {}

Vehicle::Vehicle(const Vehicle &veh) {
    //year  = new int;
    make = veh.make;
    model = veh.model;
    year = veh.year;
    price = veh.price;
    miles = veh.miles;
}
Vehicle& Vehicle::operator=(const Vehicle &veh) {
    make = veh.make;
    model = veh.model;
    year = veh.year;
    price = veh.price;
    miles = veh.miles;
    return *this;
}
Vehicle::Vehicle() {}

From Showroom.cpp:

//copy constructor
Showroom::Showroom(const Showroom &s)
{
    m_name = s.m_name;
    m_maxCapacity =s.m_maxCapacity;
    m_currentNumberOfVehicles = s.m_currentNumberOfVehicles;
    m_vehicles = new Vehicle[m_maxCapacity];

    for (int i = 0; i< s.m_currentNumberOfVehicles;i++)
    {
        m_vehicles[i] = *new Vehicle(s.m_vehicles[i]);
    }
}
//normal constructor
Showroom::Showroom(std::string name, unsigned int maxCapacity) {
    m_name = name;
    m_maxCapacity = maxCapacity;
    m_vehicles = new Vehicle[m_maxCapacity];

    m_currentNumberOfVehicles = 0;

}
Showroom::~Showroom() {
    delete[] m_vehicles;

}
Showroom::Showroom(){}

From Dealership.cpp:

//copy constructor
Dealership::Dealership(const Dealership &d)
{
    m_name = d.m_name;
    m_maxCapacity =d.m_maxCapacity;
    m_currentNumberOfShowrooms = d.m_currentNumberOfShowrooms;
    m_showrooms = new Showroom[m_maxCapacity];

    for (int i = 0; i< d.m_currentNumberOfShowrooms;i++)
    {
        m_showrooms[i] = *new Showroom(d.m_showrooms[i]);
    }
}
//normal constructor
Dealership::Dealership(std::string name, unsigned int capacity) {
    m_name = name;
    m_maxCapacity = capacity;
    m_currentNumberOfShowrooms = 0;
    m_showrooms = new Showroom[m_maxCapacity];
}
Dealership::~Dealership() {
    //std::cout<<"Deleting dealership " <<m_name << std::endl;
    delete[] m_showrooms;
    //m_showrooms = 0;

}

EDIT 2: Here is the smallest amount of code in main() that gives and error saying that I tried to deallocate a pointer that wasn't allocated:

Showroom sh("Name", 0);

    Dealership dealer("dealer", 1);
    dealer.AddShowroom(&sh);

Here's the smallest amount of code that segfaults:

Showroom sh("Name", 0);
    Showroom sh2("Showroom 2",0);
    Dealership dealer("dealer", 2);

    dealer.AddShowroom(&sh);
    dealer.AddShowroom(&sh2);

And here's AddShowroom(), for reference:

void Dealership::AddShowroom(const Showroom *showroom) {

    m_showrooms[m_currentNumberOfShowrooms] = *showroom;
    m_currentNumberOfShowrooms++;
}

Answer 1

There are many issues with your code, and they could all lead to segfaults in one way or another.

To add to what @ShadowRanger has pointed out, sh and sh2 are not dynamically allocated, and the dealer is keeping a reference to them. These two objects get destroyed automatically when they go out of scope. So when that happens dealer will be operating on objects that no longer exists and a segmentation fault will ensue.

Another cause of segmentation fault I see is due to m_showrooms not getting resized when its capacity is exceeded. The line m_showrooms = new Showroom[m_maxCapacity]; allocates m_maxCapacity number of slots for Showroom pointers. As you call AddShowroom, you need to check if this capacity is going to be exceeded. If so, you need to allocated a larger array, move the objects over, then destroy the old array (and not the objects it houses).

From what I can tell, you seem to be unfamiliar with the idea of how memory is handled in C++. I suggest spending time to figure this out because it will save you a lot of headaches down the road.

Answer 2

These lines are guaranteed memory leaks:

m_vehicles[i] = *new Vehicle(s.m_vehicles[i]);

and:

m_showrooms[i] = *new Showroom(d.m_showrooms[i]);

That's dynamically allocating, dereferencing, and using the dereferenced value to copy assign into the array. But the pointer to the original dynamically allocated value is immediately lost; you're leaking memory the instant you allocate it.

You really wanted:

m_vehicles[i] = Vehicle(s.m_vehicles[i]);

and:

m_showrooms[i] = Showroom(d.m_showrooms[i]);

Your segfaulting code is faulting for obvious reasons:

Dealership dealer("dealer", 1);

claims at most one showroom will be allocated, then you call AddShowroom twice, and write off the end of the allocated memory (likely corrupting the heap, and who knows what else).

I can't say for sure why it's complaining about unallocated pointers, but you've got so many leaks and out-of-bounds errors lying around, with so much of your code omitted, that it's hard to say for sure I haven't missed something, vs. the code with the error isn't there to check. The sort of error you're experiencing could be caused by heap corruption (which you're definitely causing at various times) or by double delete[]ing a pointer. No obvious code is doing this, but you haven't provided a MCVE (what you've provided is neither minimal nor complete), so it could be hiding.