Search code examples
c++dynamic-memory-allocation

Dynamically alloceted array, double free or corruption


I understand that this error came from double deleting the allocated memory and in theory I know what should be done. But it seems to doesn't work the way it should. Or I do sth wrong. Please help. Here's code of my class:

typedef int SIZE_TYPE;

template<typename T>
class CArrays{
private:
    SIZE_TYPE size;
    T * tab;
public:
        // methods...
    };


template<typename T>
CArrays<T>::CArrays(T value, SIZE_TYPE argsize){

    size = argsize;
    tab = new T[size];
    for(SIZE_TYPE i = 0; i < size; i++)
        *(tab+i) = value;
    }

template<typename T>
CArrays<T>::~CArrays(){

    delete [] tab;

}

template<typename T> template<typename J>
CArrays<T> & CArrays<T>::operator=(const CArrays<J> &rhs){
    if(&rhs != this){
        this->size = rhs.size;
        delete [] this->tab;
        this->tab = new T[this->size];
        memcpy(this->tab, rhs.tab, sizeof(J) * rhs.size);

    }
    return *this;
}

And when i do in my main.cpp sth like that:

CArrays<int> a(3, 10), b(0, 10);
b = a;

*** glibc detected *** ./out: double free or corruption (fasttop): 0x000000000087b010 ***

Solution

  • There are several things wrong with your code:

    Usage of memcpy to copy data.

    If the template type T is a non-POD type, you cannot use memcpy to copy over the data. You have to copy the items one-by-one, or use std::copy

    If you haven't coded one, you are missing the copy constructor.

    Here is what it should look like:

    template<typename T> 
    CArrays<T>::CArrays<T>(const CArrays<T> &rhs)
    {
        tab = new T[rhs.size];
        for (size_t i = 0; i < rhs.size(); ++i )
             tab[i] = rhs[i];
        size = rhs.size;
    }
    

    The assignment operator is peculiar.

    Maybe not wrong, but strange. An assignment operator would/should look like this (given that the copy constructor and destructor are working correctly):

    #include <algorithm>
    
    template<typename T> 
    CArrays<T>& CArrays<T>::operator=(const CArrays<T> &rhs)
    {
        CArrays<T> temp(rhs);
        std::swap(temp.size, size);
        std::swap(temp.tab, tab);
        return *this;
    }
    

    The above works, due to copying and swapping out the internals of a temporary object with the existing objects, and letting the temporary die off.

    In the last two items, I'm assuming that the only members are tab and size. If there are other members, please copy them also (and swap them out) in the code above.