CODEWITHSUNDEEP
c++arraysoopmultidimensional-arraysegmentation-fault
Mazzone
Mazzone

Reputation: 308

Segmentation fault but array object is not going out of bounds (C++)

There are other posts about common causes of segmentation faults, but I don't think the built-in array object I've created here (result) doesn't go out of bounds when I assign values to it.
I think this could be helpful to people in the future who have arrays not out of bounds, and I also haven't seen a lot of stuff about making 2D built-in array objects - examples I've seen are almost entirely vectors or std:array objects.

Here is runnable, relevant code:

matrix.h 

#ifndef MATRIX_H
#define MATRIX_H

#include <initializer_list>
using std::initializer_list;

typedef unsigned int uint;

class Matrix {

  public:

    Matrix(uint rows, uint cols);  
    ~Matrix();  
    Matrix add(double s) const;  
    const uint numRows() const;  
    const uint numCols() const;  
    double & at(uint row, uint col);  
    const double & at(uint row, uint col) const;  

  private:

    uint rows, cols;
    double ** matrix;

    void makeArray() {
      matrix = new double * [rows];
      for(uint i = 0; i < rows; ++i) {
        matrix[i] = new double [cols];
      }
    }

};

#endif

matrix.cpp 

#include "matrix.h"
Matrix::Matrix(uint rows, uint cols) {
  //Make matrix of desired size
  this->rows = rows;
  this->cols = cols;

  makeArray();

  //Initialize all elements to 0
  for(uint i = 0; i < rows; ++i) {
    for(uint j = 0; j < cols; ++j) {
      this->matrix[i][j] = 0.0;
    }
  }
}
Matrix::~Matrix() {
  for(uint i = 0; i < numRows(); ++i) {
    delete[] matrix[i];
  }
  delete[] matrix;
}
const uint Matrix::numRows() const {
  return this->rows;
}

const uint Matrix::numCols() const {
  return this->cols;
}

double & Matrix::at(uint row, uint col) {
  return matrix[row][col];
}

const double & Matrix::at(uint row, uint col) const {
  return matrix[row][col];
}

Matrix Matrix::add(double s) const {
  uint r = this->numRows();
  uint c = this->numCols();

  Matrix * result;
  result = new Matrix(r, c);

  for(uint i = 0; i < r; ++i) {
    for(uint j = 0; j < c; ++j) {
      result->at(i,j) = (this->at(i,j)) + s;
    }
  }

  return * result;
}

main.cpp 

#include <iostream>
#include <cstdlib>
#include "matrix.h"

using namespace std;

typedef unsigned int uint;

int main() {

  Matrix * matrix;
  matrix = new Matrix(3, 2); //Works fine

  double scaler = 5;

  matrix->at(2,1) = 5.0; //Works fine

  Matrix r = matrix->add(scaler); //DOESN'T WORK

  return EXIT_SUCCESS;
}

Any ideas why the add function is causing a segmentation fault error? The for-loop I used to fill the result Matrix object doesn't go out of bounds, and I'm not familiar enough with C++ to know what else could be causing it.
Thanks in advance.

Upvotes: 0

Views: 377

Answers (1)

Peter
Peter

Reputation: 36617

The problem is lack of a manually defined copy constructor or assignment operator, given that the class manages a resource (memory).

If an instance of the class is assigned, or used to create a copy, the result will be two distinct objects that reference the same memory. When those two objects which refer to the same memory are destroyed, the memory is released twice. The result of that is undefined behaviour.

Look up the "rule of three" for a solution. In C++11, that often becomes a "rule of five" or a "rule of zero" (which involves using techniques to avoid the problem in the first place).

There is also a pretty significant problem in the add() function, since it dynamically creates a Matrix, then returns a copy of it. That causes a memory leak, even if the problem with copying the object is resolved. That function actually looks like something which would be written in a garbage collected language - the problem being that C++ is not garbage collected.

Upvotes: 1

Related Questions