C : Values of both objects changes after a Copy Constructor-CodePudding

I have written a simple c code to understand the concepts of Copy Constructor/Operator Overloading. A snippet of the code is shown below.

In the code I am creating an object vec v2 and then creating a new object v4 and assign vec v2. Next I called the overloaded operator[] to change the values of v4[0] and v4[1].

My issue is that after assigning these values, the values of vec v2 has also changed. I am not quite sure how this could have happened. Hope anyone can help me with this.

class vec {
private:
    // Variable to store the number of elements contained in this vec.
    size_t elements;
    // Pointer to store the address of the dynamically allocated memory.
    double* data;

public:

    vec(size_t size) : elements{ size }, data{ new double[size] } {std::cout << "First constructor" << "\n"; };
    
    vec(size_t size, double ival) : elements{ size }, data{ new double[size] } {
        std::cout << "Second constructor" << std::endl;
        for (int i = 0; i < elements; i  ) {
            data[i] = ival;
        }
    }
    
    vec(std::initializer_list<double> iList): vec(static_cast<size_t>(iList.size())) {
        std::cout << "Third constructor" << std::endl;
        auto count{ 0 };

        for (auto element: iList) {
            data[count] = element;
            count  ;
        }
    
    }
    
    /// Copy constructor that creates a copy of the vec variable 'v'.
    vec(const vec& v) : elements{ v.elements }, data{ new double[v.elements] }{
        std::cout << "Copy constructor " << "\n";
        std::memcpy(&data, &(v.data), v.elements);
    }
    
    /// Copy assignment operator. Creates a copy of vec variable 'v'.
    vec& operator=(const vec& v) {
        std::cout << "Copy assignment " << "\n";
        if (this != &v) {
            const auto new_data{ new double[v.elements] };
            delete[] data;

            data = new_data;
            elements = v.elements;
            std::memcpy(&data, &(v.data), v.elements);
        }
        return *this;
    }
    
    double& operator[](size_t idx){
        return this->data[idx];
    }
    
    friend std::ostream& operator<<(std::ostream& os, const vec& v) {

        for (int i = 0; i < v.elements; i  ) {
            os << v.data[i] << "\n";
        }

        return os;
    }
};

int main(void) {

vec v2 = {4, 5, 6};
vec v4 = v2

v4[0] = 11;                                          // call to operator[]
v4[1] = 12;                                          // call to operator[]

return 0;
}

CodePudding user response：

The issue is the misuse of the std::memcpy function:

std::memcpy(&data, &(v.data), v.elements);

Since data and v.data are already pointers to the data, getting the address of those pointers results in the incorrect pointer values being used for those arguments.

The other issue is that the third argument v.elements should denote the number of bytes, not the number of elements, to copy.

The correct call to std::memcpy should have been:

std::memcpy(data, v.data, v.elements * sizeof(double));

But having said all this, do not use std::memcpy, and instead use std::copy. That will work with the number of elements, plus can work with types that are not trivially-copyable (such as std::string):

#include <algorithm>
//...
std::copy(v.data, v.data   v.elements, data);