Why it is a segmentation fault with this code fragment?-CodePudding

Here is my code about Matrix (I decided to practise OOP writing own Matrix class)

Matrix.hpp

#ifndef MATRIX_HEADER
#define MATRIX_HEADER

typedef unsigned int u_int;

class Matrix
{
    double **mtrx;
    u_int x, y;

public:
    Matrix(u_int a, u_int b);
    Matrix(const Matrix &);
    ~Matrix();

    double det();

    Matrix operator (const Matrix &) const;
    Matrix operator-(const Matrix &) const;                           
    Matrix operator*(const Matrix &) const;
    friend Matrix operator*(const Matrix &, const double &);
    Matrix operator/(const Matrix &) const;
    double *operator[](const u_int idx) const { return mtrx[idx]; }

    bool IsEqual(const Matrix &) const;
    u_int GetX() const { return x; }
    u_int GetY() const { return y; }
};

#endif

Matrix.cpp

#include "Matrix.hpp"

Matrix::Matrix(u_int a, u_int b) : x(a), y(b)
{
    mtrx = new double *[x];
    for (u_int i = 0; i < x; i  )
        mtrx[i] = new double[y];
}

Matrix::Matrix(const Matrix &ref)
{
    if (mtrx)
    {
        for (u_int i = 0; i < x; i  )
            delete[] mtrx[i];
        delete[] mtrx;
    }
    x = ref.x;
    y = ref.y;
    *mtrx = new double[x];
    for (u_int i = 0; i < x; i  )
        mtrx[i] = new double[y];
}

Matrix::~Matrix()
{
    if (mtrx)
    {
        for (u_int i = 0; i < x; i  )
            delete[] mtrx[i];
        delete[] mtrx;
    }
}

bool Matrix::IsEqual(const Matrix &a) const     // If sizes of matrixes are equal
{                                               // matrixes are equal
    return (a.GetX() == x) && (a.GetY() == y);
}

Matrix Matrix::operator (const Matrix &a) const
{
    if (!IsEqual(a))                            // Check on equality matrixes
        return Matrix(1,1);                     // I have not any idea yet what          
    Matrix matrix(x, y);                        // should it give is sizes 
    for (u_int i = 0; i < x; i  )               // of Matrix are not equal
        for (u_int j = 0; j < y; j  )
            matrix[i][j] = a.mtrx[i][j]   mtrx[i][j];
    return matrix;
}

main.cpp

#include <stdio.h>
#include "Matrix.hpp"

int main()
{
    Matrix a(2, 5);
    Matrix b(2, 5);
    for (u_int i = 0; i < a.GetX(); i  )
    {
        for (u_int j = 0; j < a.GetY(); j  )
        {
            a[i][j] = i   j;
        }
    }
    for (u_int i = 0; i < b.GetX(); i  )
    {
        for (u_int j = 0; j < b.GetY(); j  )
        {
            b[i][j] = i   j;
        }
    }
    Matrix c = a   b;
    return 0;
}

When I turn on my program, it throws me a segmentation fault after adding two matrixes. The more interesting thing for me in that situation is first 2 lines in Matrix.cpp, Matrix Matrix::operation ....

When I delete these 2 lines (check on equality 2 matrixes), when I turn on my program, it doesn't throw me a segmentation fault, but when I add these 2 lines, the program throws me a s.f. Can you tell me why does it work like that?

CodePudding user response：

You might consider using a 1D double[x*y] array instead of a 2D double*[x] of double[y] arrays. It will make memory management a bit easier, since you will have only 1 array to deal with, instead of multiple arrays.

In any case, your Matrix(const Matrix &) copy constructor should not be delete[]'ing anything in mtrx yet, because mtrx has not been initialized yet (it is pointing at random memory). And the rest of that constructor is not allocating the arrays correctly, either. You allocate the arrays correctly in the Matrix(u_int, u_int) constructor, so just copy that logic into the copy constructor. And then finish off the copy constructor by, you know, actually copying the double values from the Matrix being copied from.

As for operator , if you don't know what you should return for different sized matrices, then I would suggest throw'ing an exception instead.

Also, even though your current code doesn't use this yet, you should add a copy assignment operator= to finish off the Rule of 3 (you already have a copy constructor and a destructor).

Try this:

#ifndef MATRIX_HEADER
#define MATRIX_HEADER

typedef unsigned int u_int;

class Matrix
{
    double **mtrx;
    //or: double *mtrx;
    u_int x, y;

public:
    Matrix(u_int a, u_int b);
    Matrix(const Matrix &);
    ~Matrix();

    double det();

    Matrix& operator=(const Matrix &);

    Matrix operator (const Matrix &) const;
    Matrix operator-(const Matrix &) const;                           
    Matrix operator*(const Matrix &) const;
    friend Matrix operator*(const Matrix &, const double &);
    Matrix operator/(const Matrix &) const;

    double* operator[](const u_int idx) const {
        return mtrx[idx];
        //or: return &mtrx[idx*x];
    }

    bool IsEqualSize(const Matrix &) const;
    u_int GetX() const { return x; }
    u_int GetY() const { return y; }
};

#endif

#include "Matrix.hpp"
#include <utility>
#include <stdexcept>

Matrix::Matrix(u_int a, u_int b)
    : x(a), y(b)
{
    mtrx = new double*[x];
    for (u_int i = 0; i < x;   i) {
        mtrx[i] = new double[y];
    }

    // or: mtrx = new double[x*y];
}

Matrix::Matrix(const Matrix &ref)
    : x(ref.x), y(ref.y)
{
    mtrx = new double*[x];
    for (u_int i = 0; i < x;   i) {
        mtrx[i] = new double[y];
        for (u_int j = 0; j < y;   j) {
            mtrx[i][j] = ref.mtrx[i][j];
        }
    }

    /* or:
    u_int size = x*y;
    mtrx = new double[size];
    for (u_int i = 0; i < size;   i) {
        mtrx[i] = ref.mtrx[i];
    }
    */
}

Matrix::~Matrix()
{
    // this loop is not needed for a 1D array...
    for (u_int i = 0; i < x;   i) {
        delete[] mtrx[i];
    }

    delete[] mtrx;
}

bool Matrix::IsEqualSize(const Matrix &a) const
{
    return (a.GetX() == x) && (a.GetY() == y);
}

Matrix& Matrix::operator=(const Matrix &a)
{
    if (&a != this)
    {
        Matrix tmp(a);
        std::swap(mtrx, tmp.mtrx);
    }
    return *this;
}

Matrix Matrix::operator (const Matrix &a) const
{
    if (!IsEqualSize(a))
        throw std::logic_error("Cannot add matrices of different sizes");

    Matrix matrix(x, y);

    for (u_int i = 0; i < x;   i) {
        for (u_int j = 0; j < y;   j)
            matrix.mtrx[i][j] = mtrx[i][j]   a.mtrx[i][j];
    }

    /* or:
    u_int size = x*y;
    for (u_int i = 0; i < size;   i) {
        matrix.mtrx[i] = mtrx[i]   a.mtrx[i];
    }
    */

    return matrix;
}

...

#include <stdio.h>
#include "Matrix.hpp"

int main()
{
    Matrix a(2, 5);
    Matrix b(2, 5);

    for (u_int i = 0; i < a.GetX();   i)
    {
        for (u_int j = 0; j < a.GetY();   j)
        {
            a[i][j] = i   j;
        }
    }

    for (u_int i = 0; i < b.GetX();   i)
    {
        for (u_int j = 0; j < b.GetY();   j)
        {
            b[i][j] = i   j;
        }
    }

    Matrix c = a   b;

    return 0;
}