0

I'm writing a program that implements some functions of matrix but get into trouble at the beginning stage.

When debugging goes to c = c.transpose(); in the main function,it steps into the copy constructor and throws an exception at the statement:delete[]elements;. And it says:

Unhandled exception at 0x7CBDDB1B (ucrtbased.dll) in Exercise2.2.exe: 0xC0000005: Access violation reading location 0xCCCCCCBC.

enter image description here

I have no idea what happens. I would appreciate it if someone could take a look at my code.

Btw, I'm a C++ beginner, so besides the error, if there are codes that are not of standard, please point it out.

header:

#include <iostream>
using namespace std;
class Matrix
{
    int row;
    int col;
    double **elements;
public:
    Matrix();
    Matrix(int row, int col);
    Matrix(const Matrix& srcMatrix);
    Matrix& operator=(const Matrix& srcMatrix);
    void setNum(double Num, int row,int col);
    Matrix transpose();
    void display();
};

Matrix::Matrix():elements(0),row(0),col(0){}

Matrix::Matrix(int row, int col) :row(row), col(col) {
    elements = new double*[row];
    for (int i = 0;i < row;i++) {
        elements[i] = new double[col];
    }
}

Matrix::Matrix(const Matrix& srcMatrix){
    row == srcMatrix.row;
    col == srcMatrix.col;
    if (elements != NULL) {
        delete[]elements;
    }
    elements = new double* [row];
    for (int i = 0;i < row;i++) {
        elements[i] = new double[col];
    }
    for (int i = 0;i < row;i++) {
        for(int j = 0;j < col;j++) {
            elements[i][j] = srcMatrix.elements[i][j];
        }
    }
}

Matrix& Matrix::operator=(const Matrix& srcMatrix) {
    row == srcMatrix.row;
    col == srcMatrix.col;
    if (elements != NULL) {
        delete[]elements;
    }
    elements = new double* [row];
    for (int i = 0;i < row;i++) {
        elements[i] = new double[col];
    }
    for (int i = 0;i < row;i++) {
        for (int j = 0;j < col;j++) {
            elements[i][j] = srcMatrix.elements[i][j];
        }
    }
    return *this;
}

void Matrix::setNum(double Num, int row, int col) {
    elements[row][col] = Num;
}

Matrix Matrix::transpose() {
    Matrix temp(col,row);
    for (int i = 0;i < row;i++) {
        for (int j = 0;j < col;j++) {
            temp.elements[j][i] = elements[i][j];
        }
    }
    return temp;
}

void Matrix::display() {
    for (int i = 0;i < row;i++) {
        for (int j = 0;j < col;j++) {
            cout << elements[i][j] << " ";
            if (j == col - 1) {
                cout << endl;
            }
        }
    }
}

Main function:

#include<iostream>
#include "Matrix.h"
using namespace std;

int main(){
    double a[3] ={ 1.0,3.0,2.0 };
    double b[3] ={ 2.0,3.0,4.0 };
    Matrix c(2,3);
    for (int i = 0;i < 3;i++) {
        c.setNum(a[i], 0, i);
        c.setNum(b[i], 1, i);
    }
    c = c.transpose();
    c.display();
    return 0;
} 
James Z
  • 12,209
  • 10
  • 24
  • 44
BlackieMia
  • 67
  • 5
  • 1
    `0xCCCCCCBC` looks like Uninitilized stack memory. Very close to`0xCCCCCCCC`: [https://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations](https://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations) – drescherjm Mar 19 '20 at 14:07
  • In the line `delete[] elements;`, somehow `elements` has not been initialized. – user253751 Mar 19 '20 at 14:08
  • 1
    `row == srcMatrix.row;` `col == srcMatrix.col;` ->`s/==/=/` – KamilCuk Mar 19 '20 at 14:08
  • 1
    Enable your compiler warnings. There are a lot of mistakes in the code that the compiler would point out. – Eljay Mar 19 '20 at 14:15
  • Several memory leaks. Try to count of `new` and `delete`. Are these counts equal? – 273K Mar 19 '20 at 16:15

2 Answers2

3
Matrix::Matrix(const Matrix& srcMatrix){
    ...
    if (elements != NULL) {
        delete[]elements;
    }

You never initialised elements in this constructor. You leave it with indeterminate value, but read its value when comparing to null, and then attempt to delete it, both of which result in undefined behaviour.

You are constructing a new object. It cannot possibly own any elements before you create them, so there is nothing to delete either.

row == srcMatrix.row;
col == srcMatrix.col;

These comparisons, and later use as array size also read members whose values are indeterminate due to lack of initialisation. I suspect that you instead intended to assign the members. Assignment operator is single = not two. That said, it's better to initialise the members instead of assigning later.


Also, unless you need to support ancient compilers, use nullptr instead of NULL. Also, if you use NULL, you should include the header which defines it.

eerorika
  • 232,697
  • 12
  • 197
  • 326
  • After I delete `if (elements != NULL) {delete[]elements; }` , it throws another exception:std::bad_array_new_length. But if I write the function like this:`Matrix::Matrix(const Matrix& srcMatrix) :row(srcMatrix.row), col(srcMatrix.col) {this->elements = new double* [row]; ......}` It goes well. Aren't they the same? – BlackieMia Mar 20 '20 at 03:29
0

Your copy and constructor and operator=are deleting an unitialised variable. There is no need to delete elements in any case as it has never been allocated in the first instance - you do that later.

Remove the part:

if (elements != NULL) {
    delete[]elements;
}

from both. Or better since your copy constructor and operator= contain identical code, create a copy function and use that from both. The operator= should also have a self assignment check so that expressions such as x = x are safe:

Matrix::Matrix(const Matrix& srcMatrix)
{
    this->copy( srcMatrix ) ;
}

Matrix& Matrix::operator=(const Matrix& srcMatrix) 
{
    if( this != &srcMatrix ) this->copy( srcMatrix ) ;

    return *this
}
Clifford
  • 88,407
  • 13
  • 85
  • 165