C++ Segmentation Fault (core dump) when Using dynamic 2D array

Question

For an assignment, I need to be able to multiply user inputted matrices. To hold the matrices, I'm using 2D arrays, their sizes inputted by the user. I initialize the size and create the array as follows:

note, I'm trying to use short to limit the amount of memory used on the values (test values won't likely be large).

In the source file, a constructor (the default constructor looks identical except no parameters and rows/cols are initialized to 1).

Header File :

short int** value_array;

Source File :

 Matrix::Matrix(unsigned int x, unsigned int y)
 {
    rows = x;
    cols = y;
    value_array = NULL;
    value_array = new short int*[rows];
    for (unsigned int i = 0; i < cols; i++)
        value_array[i] = new short int[cols];
 }

In the initializing function :

void Matrix::init(unsigned int x, unsigned int y, short int value)
{
     value_array[x][y] = value;
     return;
}

The destructor looks like this :

Matrix::~Matrix()   // destructor
{
    for (unsigned int i = 0; i < rows; i++)
        delete value_array[i];
    delete [] value_array;
}

In the main.cpp file, the only pointer-related code is calling these functions. The value_array is a private-variable.

The core dump happens either when the matrix is being initialized or (if it's small) when the matrix containing the product after multiplication is created, which leads me to think it's somewhere in the creation code.

I'm using Ubuntu 16.04, running this in terminal with the g++ compiler.

Answer 1

Your error is in the loop you use in the Matrix constructor. You're looping over the cols value and not rows.

What ends up happening is that in your Matrix destructor, you loop over the rows in the inner loop, and doing so results in calling delete on rows that may not exist.

Live Example of Bug

Therefore this:

for (unsigned int i = 0; i < cols; i++) 
    value_array[i] = new short int[cols];

Should be:

for (unsigned int i = 0; i < rows; i++) 
    value_array[i] = new short int[cols];

In addition in your destructor, this:

delete value_array[i]; 

Should be:

delete [] value_array[i];