Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

c++ constructor with new

I'm making a very dumb mistake just wrapping a pointer to some new'ed memory in a simple class.

class Matrix
{
  public:
    Matrix(int w,int h) : width(w),height(h)
    {           
        data = new unsigned char[width*height];
    }

    ~Matrix() { delete data;    }

    Matrix& Matrix::operator=(const Matrix&p)
    {  
            width = p.width;
            height = p.height;
            data= p.data;
            return *this;
    }
    int width,height;
    unsigned char *data;
}

.........
// main code
std::vector<Matrix> some_data;

for (int i=0;i<N;i++) {
   some_data.push_back(Matrix(100,100)); // all Matrix.data pointers are the same
}

When I fill the vector with instances of the class, the internal data pointers all end up pointing to the same memory ?

like image 876
Martin Beckett Avatar asked Nov 30 '22 10:11

Martin Beckett


1 Answers

1. You're missing the copy constructor.

2. Your assignment operator should not just copy the pointer because that leaves multiple Matrix objects with the same data pointer, which means that pointer will be deleted multiple times. Instead, you should create a deep copy of the matrix. See this question about the copy-and-swap idiom in which @GMan gives a thorough explanation about how to write an efficient, exception-safe operator= function.

3. You need to use delete[] in your destructor, not delete.

like image 118
John Kugelman Avatar answered Dec 19 '22 03:12

John Kugelman