Jump to content
  • Advertisement
Sign in to follow this  
papa

Storing objects in std::vector crashes why..

This topic is 5407 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi I have this simple A class sample where *p represents some additional information I wish (sometimes) to store in an object of class A. If I wish to store some info I allocate p otherwise I keep it NULL. Now I finally store this a object in std::vector. And whenever this object a has allocated p the program crashes. I know why. Its because the "a" goes out of scope of for() loop which calls a's destructor which deletes p but I thought it will be first stored in vector container then it will be destroyed so I thought its valid. NOTE: I have to store copy of A objects into std::vector as it is now.
class A
{ public:
   A(){n=0; p=NULL;}
   ~A(){if(p)delete p; p=NULL; }
   int *p;
   int n;
};

void main()
{
	vector<A>cont;
	for(int i=0; i<2; i++)
	{
		A a;          // New object of type A every loop
                a.n = i;      
                if( extraInfo )
		    a.p = new int;  // allocate pointer if needed 
                else 
                    a = NULL;
		cont.push_back(a);
	} // this is where it crashes. a is being destroyed here. 
          // I think container doest like that it is destroyed!
}






I thought that std::vector would store every time a copy of "a" in the container so when a is destroyed it wouldnt affect it in any way the copy stored in "cont" that I need for later use. How can I fix this?

Share this post


Link to post
Share on other sites
Advertisement
Even though when you store A in the vector it makes a copy, it's a memberwise copy, meaning the pointer p will point to the same address in the copy as well as the original. When a goes out of scope and the delete happens, you now have a dangling reference on the copy because the memory location was deleted and the pointer points to deallocated memory.

Magius

Share this post


Link to post
Share on other sites
Ok usual question. How can I overcome this problem? I dont really wanna remove stuff from a destructor as I use this class quite often and in most cases I need this delete stuf in the destructor. Any suggestions?

Share this post


Link to post
Share on other sites
One method for overcoming this is to store pointers to objects of type A in the vector. I know you said you have to store a copy, but based on the semantics of just that loop I don't see a problem storing pointers to A in the vector. The caveat with this is that you have to remember to deallocate the memory yourself (i.e. iterate through the vector and delete the objects when you are done with them). Hope that helps!

Magius

Share this post


Link to post
Share on other sites
Quote:
Original post by papa
Ok usual question. How can I overcome this problem?


either write a copy constructor & assigmenet operator that does a deep copy or if copying doesn't make sense then store pointer to A in vector. Also checking for null is redundant for delete.

Also important it can be a bad idea if type A deletes memory it never allocated it self.


struct A {

int* p;
int n;

A(int f)
: n(0), p(new int(f)) {}

A(const A& a): n(a.n), p(new int(*(a.p))) {}

A& operator=(const A& a) {
if(this != &a) {
int* temp = new int(*(a.p));
n = a.n;
clear();
p = temp;
}
return *this;
}

void clear() { delete p, p = 0; }

~A() { clear(); }
};

Share this post


Link to post
Share on other sites
Yes of course youre right. Its basically same thing. I just need to deallocate it in the end. yes now it doesnt crash. Thank you.


vector<A*>cont;

for(int i=0; i<2; i++)
{
A *a = new A;
a->p = new int(555);
cont.push_back(a);
}

for(int i=0; i<cont.size(); i++)
delete cont;

Share this post


Link to post
Share on other sites

class A
{
//...

// Copy constructor. . .
A(const A &ref){ /* set p to a copy of ref.p here */ }

//...

// Assignment operator. . .
const A & operator = (const A &ref)
{
if(p != ref.p)
{
int *copy = /* copy of ref.p */;

delete p;
p = copy;
}

return *this;
}

//...
};

Share this post


Link to post
Share on other sites
[edit]Doh, darn you smart_idiot. Darn you to eternal helperhood![/edit]

The other option is to write your own copy constructor, so that instead of just copying the pointer, it actually allocates new space and copies all the data stored in p.


class A
{ public:
A(){n=0; p=NULL;}
A(const A& a){n=a.n; if (a.p!=NULL){p=new int;*p=*a.p;}else{p=NULL}}
~A(){if(p)delete p; p=NULL; }
int *p;
int n;
};


Share this post


Link to post
Share on other sites
Quote:
Original post by papa
smart_idiot hehe nice name. I am not sure how to use your code in my code.


Look at the example i've written its exactly the same but using your example, you don't need to store pointer in vector if you use are method.

Share this post


Link to post
Share on other sites
smart_idiot hehe nice name. I am not sure how to use your code in my code. Why would I need a copy contructor. As far Ias I know its useful for this;

A a;
B b(a);

I am not sure how else I could apply to my main program

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!