2

I have the following class, where I force the compiler to generate all the copy/move constructors and assignment operators.

class foo {
public:
    float *x;
    size_t size;
    foo(int m){
        size = m;
        x = new float[size];}

    foo(const foo&) = default;
    foo(foo&&) = default;
    foo& operator =(const foo&) = default;
    foo& operator =(foo&&) = default;

    ~foo(){delete [] x;}

    void fill(const float& num)
    {
        std::fill(x,x+size,num);
    }

    void print()
    {
        for (auto i=0;i<size;++i)
            cout << x[i] << endl;
        cout << endl;
    }
};

Then I call it from the main function, like this 

int main()
{
    foo x(2);
    x.fill(6);
    x.print();


    foo y(2);
    y = x; // causes the error

    return x;
}

Now I know I am freeing the memory twice by assigning y = x; so once one is freed the other is null, am I right? I went ahead and implemented my own copy assignment operator

foo& operator=(const foo& other)
{
    if (other.x!=x)
        x = other.x;
    return *this;
}

However, I guess here again I am doing what the default constructor is doing anyway. My question is how to make a proper copy assignment operator so that this problem does not happen?

  • possible duplicate of [What is The Rule of Three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – NathanOliver Sep 17 '15 at 19:02
  • `std::unique_ptr x;` would solve your double-free and provide correct destruction, move construction and move assignment automatically. And force you to be intentional about defining the copy operation (it would be deleted by default). – Ben Voigt Sep 19 '15 at 01:11

1 Answers1

1

You need to copy not the pointer, but the contents of the pointer. A good approach to use is the copy and swap idiom since your copy constructor should already do the work of copying the contents of x:

friend void swap(foo& first, foo& second)
{
    using std::swap; 
    swap(first.x, second.x); 
    swap(first.size, second.size);
}

foo& operator=(foo other) // note pass by value
{
    swap(*this, other);
    return *this;
}
Community
  • 1
  • 1
Claudiu
  • 224,032
  • 165
  • 485
  • 680
  • Is it also possible to memcpy the pointer instead? Would that also solve the problem? –  Sep 17 '15 at 19:44
  • 1
    @chhadd: Not in the general case, because `size` may be different. Consider: `foo f(10), g(2); g = f;`. You would be able to do it if you assign to an equal size or less, but you'd still have to be able to account for the general case. – Claudiu Sep 17 '15 at 19:51