How to delete pointers of vector

Question

I want to delete properly all the elements of my vector. I need to fill a vector of vector of structs in the way you should see below, creating each time a new vector of struct invoking new method.

this is my code:

struct node
{
    int index;
    double value;
}

struct problem
{
    int l;
    struct node **x;
};

struct problem prob;    // set by read_problem
std::vector<node *> vettProbX;


int main(){
    node tmpValue;
    std::vector <node> *temp;

    for (int i=0; i<10; i++)
    {
        temp = new std::vector<node>;

        for (int i=0; i<10; i++)
        {
            tmpValue.index=i;
            tmpValue.value=i;
            temp->push_back(tmpValue);
        }

        vettProbX.push_back(temp->data());

    }

    prob.x=&vettProbX[0];

    //Destructor
    for(int CC=0; CC<vettProbX.size(); CC++)
    {
        delete  temp[CC];
    }
    return 0;
}

I want to delete all the new objects allocated with NEW, but i don't know how to do it correctly.

Store your pointers in: [`std::unique_ptr`](http://en.cppreference.com/w/cpp/memory/unique_ptr), it will handle deletion for you. — Borgleader, Feb 06 '14 at 14:47
I don't quite get your code, but the way I understand your question [this](http://stackoverflow.com/questions/21549136/how-to-deal-with-pointers-in-template-classes/21549320#21549320) could be helpful for you (simply replace BinaryTree with std::vector mentally). — gTcV, Feb 06 '14 at 14:59
`temp = new std::vector;` this line looks like a nice place for a memory leak. Make sure you delete temp at the end of the i loop. — Nav, Feb 06 '14 at 15:04

eerorika · Answer 1 · 2014-02-06T16:00:30.947

3

The only object that you have allocated with new is std::vector<node> *temp;. You delete that with delete temp; Vectors own destructor will handle the destruction of its contents. But you must do that for every time that you call new. In this case you allocate inside loop so you need to call delete at the end of the loop.

But because temp is only used within scope, there is no need to allocate it dynamically. You can allocate it on the stack like this: std::vector<node> temp;. Then it will be destructed automatically when it goes out of scope (at the end of function in this case).

In your code, you allocate temp 10 times, but you leak first 9 because you don't store their pointer anywhere (And thus cannot delete them anymore). You always overwrite the pointer of previous temp. At the end of your code you try to delete contents of the last temp (which you can't because the contents are not pointers) rather than the vector itself.

To achieve what you're trying to do, what you need to do is to store all the temp pointers in a std::vector<std::vector<node>* > and in the end delete each vector inside. But you would have much nicer code if you didn't use any pointers at all.

Also, you have two nested loops that use the same loop variable. It probably won't work as you have intended.

edited Feb 06 '14 at 16:00

answered Feb 06 '14 at 15:00

eerorika

232,697
12
197
326

this is only the main content of my full code.Anyway i need to allocate it dinamically for other purposes. – zzari Feb 06 '14 at 15:04
@user2988113, what other purposes? – Shoe Feb 06 '14 at 15:06
i need to store inside the vettProbX, at each iteration, a pointer that points to a different segment of memory. The problem is how to delete correctly all the pointer created with new(). – zzari Feb 06 '14 at 15:12
@user2988113 As this answer explains, you are only creating one object with `new` and this answer also tells you how to delete that. So what is the problem? – Gorpik Feb 06 '14 at 15:37
@Gorpik. In my code it seems that i create 10 times the object temp, and i want to destroy them only at the end of my code. Am i wrong? – zzari Feb 06 '14 at 15:44
1

@user2988113 In your code, you allocate `temp` 10 times yes, but you leak first 9 because you don't store their pointer anywhere (And thus cannot delete them anymore). You always overwrite the pointer of previous `temp`. At the end of your code you try to delete contents of the last `temp` (which you can't because the contents are not pointers) rather than the vector itself. – eerorika Feb 06 '14 at 15:46

score 2 · Answer 2 · answered Feb 06 '14 at 15:15

I want to delete properly all the elements of my vector

It's not possible to free everything because of the strange way you allocated memory. You dynamically allocate a vector pointed to by temp, then overwrite your pointer to it in the next iteration of the loop. That's it, once your record of the pointer is gone you have no way to free it.

I need to fill a vector of vector of structs

In that case you should first create a vector<vector<node>>. You can't fill a vector of vectors if you don't have one, and your current code doesn't have one.

ciamej · Answer 3 · 2014-02-06T16:19:41.557

If you don't save a pointer to the newly created vector, that you create here:

temp = new std::vector<node>;

then, there is no way to delete it and free the memory.

Change

std::vector<node *> vettProbX;

vettProbX.push_back(temp->data());

to

std::vector<std::vector<node> *> vettProbX;

vettProbX.push_back(temp);

then you can delete every item stored in vettProbX one by one

for (int i = 0; i < vettProbX.size(); i++)
    delete vettProbX[i];

if you need to access these memory segments instead of doing

vettProbX[i]

you will have to call the data method

vettProbX[i]->data()

How to delete pointers of vector

3 Answers3