Invalid Allocation Size (in derived class copy constructor)

Question

I have narrowed down my issue to a derived classes copy constructor, but I am unsure of the cause. EDIT: M, N and Data are Private. The error I recieve is 'Invalid allocation size: 4294967295 bytes' - which I understand is caused when passing a -1 to new. I'm unsure why this would occur unless the data is lost when the class comunicate.

BinaryMatrix::BinaryMatrix(const BinaryMatrix& copy) : Matrix(copy)
{
    //cout << "Copy Constructor\n";

    M = copy.M;
    N = copy.N;

    data = new double[M*N]; //This line causes the allocation error

    for (int i = 0; i < M; i++)
    {
            for (int j = 0; j < N; j++)
            {
                    data[i*N+j] = copy.data[i*N+j];
            }
    }
}

The above is my derived copy constructor which causes the error. I have marked the allocation line.

I can only assume that M and N are not being read correctly. Though I am unsure why. I'll include both derived and base constructors, and the base copy as well.

Thanks for any assistance.

MATRIX (BASE) CONSTRUCTOR

Matrix::Matrix(int M, int N, double* input_data)
{
    this->M = M;
    this->N = N;

    //cout << "Matrix Constructor\n";
    data = new double[M*N];

    for (int i = 0; i < M; i++)
    {
            for (int j = 0; j < N; j++)
            {
                    data[i*N+j] = input_data[i*N+j];
            }
    }

    delete [] input_data;
}

MATRIX (BASE) COPY CONSTRUCTOR

Matrix::Matrix(const Matrix& copy)
{
    //cout << "Copy Constructor\n";

    M = copy.M;
    N = copy.N;

    data = new double[M*N];

    for (int i = 0; i < M; i++)
    {
        for (int j = 0; j < N; j++)
        {
            data[i*N+j] = copy.data[i*N+j];
        }
    }
}

BINARYMATRIX (DERIVED) CONSTRUCTOR

BinaryMatrix::BinaryMatrix(int M, int N, double* input_data) : Matrix(M, N, input_data)
{
    data = new double[M*N];

    for (int i = 0; i < M; i++)
    {
        for (int j = 0; j < N; j++)
        {
            this->data[i*N+j] = this->getRead(i, j);
        }
    }

    double thr_val = this->Mean();

    for (int i = 0; i < M; i++)
    {
        for (int j = 0; j < N; j++)
        {
            if (this->data[i*N+j] > thr_val)
                this->data[i*N+j] = 1;

            if (this->data[i*N+j] < thr_val)
                this->data[i*N+j] = 0;
        }
    }
}

Answer 1

Why do you create a new copy of the matrix data in the BinaryMatrix copy constructor? The copy constructor of Matrix you call from the BinaryMatrix copy constructor already does this.

In the BinaryMatrix copy constructor you discard the copy of the matrix data the Matrix copy constructor already made (without deleteing it) and create a new one. This is a memory leak - the memory will be exhausted if you do that often enough.

Answer 2

If the error is that M and N are private. then you must change there protection level to protected or public or provide an access method that is. Vairables defined in a base class that are private are innacceassable to the derived classes.

class A
{
    int x;
}

class B : public A
{
    int DoSomething()
    { 
        return x; //This is an error X is private to class A and as such inaccessible. 
    }

}

Answer 3

If M and N are private to Matrix and BinaryMatrix derives from Matrix, I am not sure why your code compiles (you should not be able to access M, N in BinaryMatrix). If your declaration of BinaryMatrix also includes members M and N (as well as Matrix::N and Matrix::M) then this could be the source of the problem.

If BinaryMatrix does not declare M and N, then I think we still don't have enough data to diagnose your problem. To guess a bit, perhaps M*N does not fit into the type used for M. So you have an arithmetic overflow. The array size is specified in size_t, so a cast will work OK.

Also, you probably want to delegate the management of the data to exactly one of the classes. That is, do either this:

BinaryMatrix::BinaryMatrix(const BinaryMatrix& copy) : Matrix(copy)
{
    // M, N, data already set in Matrix::Matrix(const Matrix&)
    // The data has also been copied, so there is nothing to do here.
}

or this:

#include <algorithm>

BinaryMatrix::BinaryMatrix(const BinaryMatrix& copy) 
 : Matrix(), M(0), N(0), 
   data(0) // null in case new throws an exception (avoid bad delete in dtor).
{
    const size_t nelems(size_t(copy.M)*size_t(copy.N));
    data = new double[nelems];
    M = copy.M;
    N = copy.N;
    stl::copy(copy.data, copy.data+nelems, data);
}

I think generally it is not a good idea to use int for iterating over dynamic data structures, since nothing guarantees that the actual size of the structure fits into int. That guarantee does exist however for size_t (any existing object must have a size representable in size_t, so you can iterate over any contiguous object using size_t).

In fact, I'm not really sure what distinction (of purpose or behaviour) you're trying to get from Matrix and BinaryMatrix. The data seems to have the same representation. If it is a difference of behaviour but not representation you are looking for, I think it might be better to use composition (that is, a separate un-inherited representation class) rather than inheritance. See What is the Liskov Substitution Principle? for an explanation of how to think usefully about when to use inheritance.

However, if none of the answers you have seen so far actually helps to solve your problem, you should put some time into cutting down your example: what is the smallest complete example program which demonstrates your problem? Post that.