Allocation problem in STL vector
Hi
Can anybody tell me why an allocated (STL) vector in Visual C++ only calls the default constructor of the vector-element once (not once for each element) meanwhile then the vector is destructed the destructor of the vector-element is called once for each elements in the vector.
In the example below the constructor of Element is called once and the destructor is called twice. Second time the destructor is called it tries to delete the undefined pointer p and the system crash.
class Element {
int *p;
public:
Element() {
p = new int[10];
};
~Element() {
delete p;
};
};
int main(int argc, char* argv[])
{
std::vector<Element> vElements(2);
return 0;
}
# 1 Re: Allocation problem in STL vector
This is not a bug in the vector class, but it is an oversight in the class that you've coded.
A basic rule in designing a C++ class is this:
If you have a class, and one or more of the class members is a a pointer to dynamic memory, it is your responsibility to code a sensible copy constructor and operator = for the class, especially if you want to use it for any of the standard containers (and if you want to make your program stable, regardless of whether you use the containers or not).
Since the vector<> class relies on your copy construction and op= to be working correctly, you better change your class to add correct versions of these. Right now, you can't copy construct or assign safely because you have no code to handle the dynamic pointer. For example, this code will not work:
void SomeCode()
{
Element E1;
Element E2 = E1;
}
BOOM!! E2 and E1 share the same pointer! On destruction, you will get a double deletion error. Note that there is no vector here, and you are still in trouble.
Also, it is "delete[]", not "delete". You allocated with new[ ], you deallocate with delete [ ]
Here is the way it should look:
-------------
class Element
{
int *p;
Element( ) { p = new int [10];
}
Element(const Element& rhs)
{
p = new int[10];
memcpy(p, rhs.p, sizeof(int) * 10);
}
Element& operator=(const Element& rhs)
{
if ( &rhs == this )
return *this;
delete [] p;
p = new int [10];
memcpy(p, rhs.p, sizeof(int) * 10);
}
};
-------------
Also, consider using vector<int>. There really was no need to use int*. If you used vectors, you wouldn't have had to code your own copy constructor and op = since the vectors know how to copy correctly without any help.
Regards,
Paul McKenzie
