CODEWITHSUNDEEP
c++memory-managementvector
Sara
Sara

Reputation: 823

Reserving and deleting vectors?

I'm having a problem deleting data of a vector.

When I create the data, I first reserve space in an array, and then resize the vector and copy the addresses of the array:

//Create the vertices
verts.reserve(vn); verts.resize(vn);
TriVertex *vertsaux = new TriVertex[vn];

for(int i=0, c=0; i<vn; i++, c+=3)
{
     vertsaux[i].SetId(i);
     vertsaux[i].SetCoords0(Vector3(vs[c], vs[c+1], vs[c+2]));

     //Inicializate texture vertices
     vertsaux[i].SetTextureCoords(Vector2(0.0f, 0.0f));
}

for(int i=0; i<vn; i++)
{
     verts[i] = &vertsaux[i];
}

But in the destructor of my class, it gives me a runtime error when I do this:

for (i=0; i < this->verts.size(); i++) {
     delete this->verts[i];
}

Anybody know why can this is happening?

By the way, I can't just create new TriVertex inside the for, because of some implementation details...

Upvotes: 2

Views: 272

Answers (6)

Joe
Joe

Reputation: 42627

There are two things here:

  1. As a number of people have pointed out, allocating an array requires deleting as an array. However, I think it's masking what you're really trying to do here. Change your allocation to do a "new TriVertex" for each item.

  2. Assuming your vector is storing TriVertex* pointers, you indeed have to delete each item in your destructor (as you have written.)

Thus, fixing your initial allocation strategy for TriVertex objects should solve the problem.

for(int i=0, c=0; i<vn; i++, c+=3)
{
    TriVertex *vertsaux = new TriVertex;

    vertsaux->SetId(i);
    vertsaux->SetCoords0(Vector3(vs[c], vs[c+1], vs[c+2]));

    //Initializing texture vertices
    vertsaux->SetTextureCoords(Vector2(0.0f, 0.0f));
    verts[i] = vertsaux;
}

Upvotes: 0

Tony The Lion
Tony The Lion

Reputation: 63200

delete this->verts[i];

This deletes one item inside the dynamically allocated array you created and is actually UB to delete it that way.

An array like that should be deleted with delete[] statement, and that deletes the entire array.

Upvotes: 0

James Kanze
James Kanze

Reputation: 153919

You're not really giving us enough information. What it the type of verts? And why are you using versaux? From what little I can see, the most logical solution would be to have std::vector <TriVertex> verts; as a class member, and initialize it directly, without the intermediate dynamic allocations. Maybe:

verts.reserve(vn);

for ( int i = 0; i < vn; ++ i ) {
    verts.push_back( TriVertex() );
    TriVertex& current = verts.back();
    current.SetId( i );
    current.SetCoords( Vector3( *vs, *(vs+1), *(vs+ 2) ) );

    current.SetTextureCoords( Vector2( 0.0, 0.0 ) );
    vs += 3;
}

I don't see any reason for the intermediate, dynamically allocated array. (If there is, Joe's response is correct.)

Upvotes: 1

quamrana
quamrana

Reputation: 39354

Since you create an array of TriVertex, pointed to by vertsaux, then you should delete the array using:

delete[] vertsaux;

You can't delete the individual elements of the array.

Having done the above delete[], you should remove all the elements of verts like this:

verts.clear();

This is because since you have deleted vertsaux, the pointers inside verts are invalid.

I'm assuming that verts is something like: std::vector<TriVertex*>

Upvotes: 0

Bo Persson
Bo Persson

Reputation: 92261

You allocate an array of objects with

TriVertex *vertsaux = new TriVertex[vn];

To delete that again you need to do

delete[] vertsaux;

You can not delete the elements individually.

Upvotes: 3

Oswald
Oswald

Reputation: 31647

When you allocate an array using

TriVertex *vertsaux = new TriVertex[vn];

you have to deallocate using delete[] by calling

delete[] vertsaux;

Upvotes: 0

Related Questions