c++ double free or corruption (fasttop) -- not sure where the error is

Question

Based on valgrind I believe my error stems from these guys because the error happens after I assign a new set to another set. Set Z -> A^B (intersection operation that returns a Set). I am just not sure what I did wrong, any help would be greatly appreciated!

Set::~Set()
{
        Cap = 0;
        Num = 0;
        delete [] Pool;
        Pool = NULL;
}
Set::Set(const Set &A)
{
        Cap = A.capacity();
        Num = A.size();
        Pool = A.Pool;
}
Set& Set::operator=(const Set &X)
{
        Cap = X.capacity();
        Num = X.size();
        Pool = X.Pool;
        return *this;
}

`Pool = A.Pool` it seem you need to make a copy of pool not just point to another Set's pool. this may be your error. — andre, Nov 01 '12 at 17:30
if someone could give me an example of how to do a deep copy in this instance that would be great, i have tried multiple things but nothing seems to work. — sharkman, Nov 01 '12 at 18:08

juanchopanza · Accepted Answer · 2012-11-01T17:53:55.690

2

You have a dynamically allocated array Pool, which you are shallow copying in your copy constructor and assignment operator. So you will have more than one object trying to delete the same array.

You need to make a "deep copy" of Pool, that is, create a new dynamically allocated array containing copies of the elements of the original. For this, you need to know the size of the original array. The simplest solution is to use an std::vector instead. Then you wouldn't even need to provide your own copy constructor and assignment operator. The compiler-synthesized ones would suffice.

edited Nov 01 '12 at 17:53

answered Nov 01 '12 at 17:30

juanchopanza

223,364
34
402
480

so i could do something like `Set Z; Z.Num = A.size(); Z.Cap = A.capacity(); for(unsigned Y=0;Y – sharkman Nov 01 '12 at 17:59
@sharkman If you used a vector, you wouldn't even have to do most of that. If `Cap` corresponds to `capacity()` and `Num` corresponds to `size()`, you can get rid of those data members. And you don't have to loop over the vector copying it either. If you keep the array, then use `std::copy` to do the deep copy. – juanchopanza Nov 01 '12 at 21:25

score 0 · Answer 2 · edited May 23 '17 at 12:03

You are doing a shallow copy, the result of which is that both the source and destination objects involved in the copy share the same reference to a single block of memory. Naturally, when the two objects are destroyed, the first one will delete[] this block of memory, and the second one will then do the same resulting in the "double free".

You can either allocate a new Pool in the destination of the copy and then use memcpy (or similar) to duplicate the contents, or use an existing container (eg, a std::vector<>) to store the contents of the pool and rely on the standard library's copying implementation. If a shallow copy was really what you wanted, with the same Pool shared by both instances, you could use some sort of shared handle to the memory pool (see this example of using a std::shared_ptr to wrap an array).

score 0 · Answer 3 · answered Nov 01 '12 at 19:40

0

So you guys were all right. But for anyone with the same problem as me here is what I did to solve it.

Pool = new int[A.capacity()];
*Pool = *A.Pool;
Num = A.size();
Cap = A.capacity();

answered Nov 01 '12 at 19:40

sharkman

175
1
5
11

c++ double free or corruption (fasttop) -- not sure where the error is

3 Answers3