Seg error after item is placed in STL container

Question

typedef struct temp { int a,b; char *c; temp(){ c = (char*)malloc(10);}; ~temp(){free(c);}; }temp; int main() { temp a; list<temp> l1; l1.push_back(a); l1.clear(); return 0; } 

gives a segmentation error.

Answer 1

You do not have a copy constructor.

When you click 'a' into the list, it is copied. Since you do not have a copy constructor (to allocate memory for c and copy from old c to new c) c is the same pointer to and instance a in the list.

A destructor for both called, the first will succeed, the second will fail, because memory c points to already freed.

You need a copy constructor.

To find out what happens, add some paths to the constructors and destructors and execute the code.

Answer 2

You need a deep copy constructor to avoid double free (). You have a temp class (a) variable, then you add it to the list. The variable is copied. Then you clear the list, the element inside is destroyed and the free () function is called. Then the variable is destroyed, and the free () function is called again for the same address, which leads to a segmentation error.

You need a copy constructor for deep copies of temp variables, which will malloc () another buffer and copy the data.

Answer 3

At the time you call l1.push_back(a) , the second copy of 'a' is copied. As a result, there are currently two classes that believe that they own the memory from the original malloc call, and when the second is deleted, it will try to free the memory deleted by the first.

The solution is to add a copy constructor that deals with the fact that the class instance does not actually own the data. Usually you do this using some form of link count.

Answer 4

Besides fixes, you should avoid using malloc / free in C ++. In your particular case, I would go with a vector:

 #include <vector> typedef struct temp { int a,b; std::vector<char> c; temp(){ c.reserve(10);}; }temp; int main() { temp a; list<temp> l1; l1.push_back(a); l1.clear(); return 0; } 

Answer 5

If you do not want to add a copy constructor, you can consider a list of pointers to values ​​instead of a list of values.

 list<temp*> l1; l1.push_back( new temp() ); 

But then you must manually delete each object in the list to prevent memory leak.

Also a, b members in your structure are not initialized. Be careful.

Answer 6

In addition to the copy constructor, in this case it is also advisable to provide the = operator.

 struct temp { // typedef is implicit in C++ int a,b; char * c; // Constructor temp() { c = malloc(10); } // Destructor ~temp() { free(c); } // Copy constructor temp(const temp & x) { c = malloc(10); setTo(x); } // Operator = temp & operator = (const temp & x) { setTo(x); return *this; } // Initialize THIS to X void setTo(const temp & x) { a = xa; b = xb; memcpy(c,xc,10); } }; 

Answer 7

You need a copy constructor and an assignment operator — things stored in the STL collection are stored as copies and can be redefined as the size of the vector changes.

It’s hard to see from your code what the semantics of the copy constructor should be, but at least it's a crib to allocate some memory so that (if nothing else) the destructor has something free. An assignment operator is also difficult to specify without additional information about your class.

Answer 8

You need to define a copy constructor for temp . Now when you press a to the list, a copy of a is made. The copy (call it a2 ) is initialized by the product a2.c = ac . This means that a and a2 point to the same block of memory. When their destructors are called, this memory block is free twice, which leads to a segmentation error.

Given what you posted, the copy constructor should be something like this:

 temp::temp (temp const &rhs) { this->a = rhs.a; this->b = rhs.b; this->c = (char *) malloc (10); memcpy (this->c, rhs.c, 10); } 

This assumes that what c indicates is always 10 characters long ...

Answer 9

Another problem with this code is extra semicolons in

 temp(){ c = (char*)malloc(10);}; ~temp(){free(c);}; 

better to remove them:

 temp(){ c = (char*)malloc(10);} ~temp(){free(c);} 

Answer 10

Ironically, this has an "STL" tag, but the lack of an STL is what causes the problem.