0

I have a member function called Get as part of a matrix class structure. This matrix holds the greyscale code for each pixel of an image. The class declares a pointer to private variable

double* matData new double[N*M];

Whenever I call the Get function I get the error:

ACCESS VIOLATION READING LOCATION

Here are the two files that I am working with:

Matrix.h

#ifndef MATRIX
#define MATRIX


class Matrix {
public:
    Matrix(int sizeR, int sizeC, double* input_data, string n);
    double Get(int i, int j) const;
    void Set(int i, int j, double val);
    ~Matrix();
private:
    int M; int N;
    string name;
    double* matData;
};


#endif

Matrix.cpp

#include <iostream>
#include <string>

using namespace std;

#include "matrix.h"

Matrix::Matrix(int sizeR, int sizeC, double* input_data, string n)
{
    M = sizeR;
    N = sizeC;
    name = n;
    double* matData = new double[N*M];

    cout << "New Matrix '" << name << "' created" << endl;

    for (int ii = 0; ii<M*N; ii++)
    {
        *(matData + ii) = *(input_data + ii);
    }
}
Matrix::Get(int i, int j) const 
{
    return matData[i * N + j];
}
Matrix::Set(int i, int j, double val)
{
    matData[i*N + j] = val;
}
Matrix::~Matrix()
{
    delete matData;
}

Can someone explain what causes this error and why it's being thrown when I'm returning what is the value at a certain point in an array at the memory location that matData is pointing to. Thank you!

jb167897
  • 37
  • 5
  • it should be `delete[] matData`. – Gene Nov 19 '16 at 16:12
  • Please post a [Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve). What you should try first may be obeying [The Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) and trying again. – MikeCAT Nov 19 '16 at 16:13
  • Maybe... In your constructor, you are using double * matData =... , which creates a new variable, not the class member. Try writing just matData = new double[N*M] – Honza Dejdar Nov 19 '16 at 16:13

3 Answers3

3

double* matData = new double[N*M]; creates a new pointer inside the constructor. The memory is leaked right away, and the member pointer is still uninitialized.

Remove the double* to refer to the member variable.

Bo Persson
  • 90,663
  • 31
  • 146
  • 203
2

The member variable matData isn't initialized, so dereferencing it has a big hance to cause segmentation fault.

Stop declareing local variable matData, which shadows the member variable, in the constructor and initialize the member variable before dereferencing it.

Other notes are:

  • delete[] has to be used instead of delete for deleting what is allocated via new[].
  • Obey The Rule of Three: define copy constructor and assignment operator properly in order to avoid problem caused by copying pointer (not data in array).
Community
  • 1
  • 1
MikeCAT
  • 73,922
  • 11
  • 45
  • 70
0

I do not see the reason why you are not just using std::vector<> which allocated and removes unused memory as you need it and it is also safe. Also it works just like any other array for example std::vector<int> myArray; after populating it you can use it as myArray.at(0); or myArray[0];. Also std::vector<> knows its size which can come in handy.

matt2405
  • 171
  • 2
  • 13