2

First I dynamically allocate an object:

Object* someObject = new Object();

Then I insert the address of this pointer into a vector:

std::vector<Object**> someVector0;
someVector0.push_back(&someObject);

Then I insert the address of an address living in someVector0 to another vector:

std::vector<Object***> someVector1;
someVector1.push_back(&someVector0[0]);

Lastly, I insert the address of the aforementioned someVector1 to another vector:

std::vector<std::vector<Object***>*> someVector2;
someVector2.push_back(&someVector1);

Then I use the vector obtained, now I need to free the memory allocated:

//Do some stuff

//Free memory

    for (int i = 0; i < someVector2.size(); i++) {
        for (int j = 0; j < (*someVector2[i]).size(); j++) {
            delete *(*(*someVector2[i])[j]);
            delete *(*someVector2[i])[j];
            delete (*someVector2[i])[j];
        }
        delete someVector2[i];
    }

I had suspected that this block of code would have been enough to deallocate but, I ran into some problems whilst deallocating. I suspect that I might have dereferenced a nullptr but I can't think of anything else, could there be a mistake in the way I delete the pointers?

Note: This is a lightened version of a problem I encountered with a program of a larger scale, none of the previously mentioned vectors is unnecessary and I do not want to handle any copies, do not change the types of the vectors.

EDIT:

Hi, after researching more and hearing out the comments I have come to my senses and even though I do not think there is a single soul that would find my code in any way sensible I strongly urge anyone that might see this as a valid way to do anything not to do it. As a comment already stated, my attempts at a solution showed that I did not have an idea of what I was doing, so please don't do it; it's practically unmaintainable. Instead, as another comment already pointed out use smart pointers or really be aware of what happens in your code when.

Di0n
  • 65
  • 4
  • 5
    What you need is to redesign your code. – Vlad from Moscow Mar 11 '23 at 08:38
  • 2
    May I recommend using some more `&` symbols than `*`s in c++ code? And even more important eliminate any use of `new` and `delete`. – πάντα ῥεῖ Mar 11 '23 at 08:41
  • 2
    `std::vector someVector1;` -- `std::vector*> someVector2;` -- This is unmaintainable. It seems you are trying to write code to defeat the whole purpose of using `vector`. – PaulMcKenzie Mar 11 '23 at 08:41
  • 2
    You only `new`ed the object pointed to by `someObject`, so that's the only object you need to `delete`. Remember, you don't `delete` pointers, you `delete` the objects they point to, and you only need to `delete` objects you create with `new`. – Miles Budnek Mar 11 '23 at 08:41
  • 1
    `std::vector*>` -- This maybe the first time I have seen a 4-star programmer. Note that addding more stars doesn't mean it makes the code better or "advanced" or "smarter", or even faster. It does the opposite. – PaulMcKenzie Mar 11 '23 at 08:47
  • 1
    Vectors of pointers a often a bad idea, but your code is worse, you allocate an object and then insert a pointer to the variable containing a pointer to the allocated object into the vector. This is **almost certainly** a mistake, and there is no right way to write this code. It gets even worse after that. However you haven't provided enough context to be sure. I don't think your problem is solvable without that context. – john Mar 11 '23 at 08:47
  • 2
    It must be said that your attempt at a solution shows you have no idea what you are doing, so I'm not sure why you are so convinced that all these pointers are necessary or beneficial. – john Mar 11 '23 at 08:52
  • 1
    I doubt that all those pointers are valid by the time you get to deleting, even if you managed to get them in the right place. – molbdnilo Mar 11 '23 at 08:58
  • 1
    Why are you calling `new` manually at all? Use smart pointers and forget about all this. – HolyBlackCat Mar 11 '23 at 08:59
  • 1
    Is the whole point of this code essentially attempting to store a 3 dimensional `Object` array inside a vector, and that the object "array" is of the form `Object***` using `new[]` to create this array? If so, then if you *had* to do this, doing something like [this](https://stackoverflow.com/questions/52068410/allocating-a-large-memory-block-in-c/52069368#52069368) would be a lot cleaner. Then all you would need is `std::vector>`. – PaulMcKenzie Mar 11 '23 at 09:36
  • 1
    For the sake of the next person to modify this code, please see [What is a smart pointer and when should I use one?](https://stackoverflow.com/q/106508/) – JaMiT Mar 11 '23 at 09:50

2 Answers2

5

For a single element, you called new once but delete 4 times. That's wrong. You can only delete something you created with new, and once it's deleted you cannot delete it again.

John Zwinck
  • 239,568
  • 38
  • 324
  • 436
2

For starters you should redesign your code because it is very confusing and unclear.

As each object of the type Object was dynamically allocated only once then for each operator new you need to call only one operator delete,

Here is a demonstration program that shows how it can be done.

#include <iostream>
#include <vector>

struct Object
{
    int i;
    ~Object()
    {
        std::cout << "deleting object " << i << '\n';
    }
};

int main()
{
    std::vector<Object **> someVector;
    Object *p_obj1 = new Object{ 1 };
    Object *p_obj2 = new Object{ 2 };

    someVector.push_back( &p_obj1 );
    someVector.push_back( &p_obj2 );

    std::vector<Object ***> someVector1;

    for (auto &p : someVector)
    {
        someVector1.push_back( &p );
    }

    std::vector<std::vector<Object ***> *> someVector2;
    someVector2.push_back( &someVector1 );

    for (const auto p_vector1 : someVector2)
    {
        for (auto ppp : *p_vector1)
        {
            delete **ppp;
        }
    }
}

The program output is

deleting object 1
deleting object 2

As the vector someVector2 contains pointers to vectors then this range based for loop

    for (const auto p_vector1 : someVector2)

provides each element of the pointer type std::vector<Object ***> * stored in the vector.

Dereferencing the pointers in this range based for loop

        for (auto ppp : *p_vector1)

you will get elements of the pointer type Object ***. So to free an object of the type Object you need to get a pointer to it by two times dereferencing the elements

delete **ppp;
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335