0

Ive created class mat (matrix) with consructor functions and operator + overloaded to add two matrix' but when i compile and run my code, Everything works fine till i initialize my m1, m2..and then result is "segmentation fault (core dumped)". I cant figure out why is it showing it? i've not posted here the ostream& operator << function to shorten the code length.

class mat{
    int r,c;
    float **p;
    public:
    mat(){}
    mat(int,int);
    mat(int,int,float);
    void initialize();
    mat operator+(mat); //defined
    friend ostream& operator<<(ostream&,mat&);  
};
void mat :: initialize(void){
    int i,j;
    cout<<"\nEnter the elements : ";
    for(i=0;i<r;++i){
        for(j=0;j<c;++j){
            cin>>p[i][j];
        }
    }
    return;
}
mat mat :: operator+(mat x){
    mat tmp;
    int i,j;
    for(i=0;i<r;++i){
        for(j=0;j<c;++j){
            tmp.p[i][j]=(p[i][j])+(x.p[i][j]);
        }
    }
    return tmp;
}
mat :: mat (int a, int b){ 
    r=a;
    c=b;
    p=new float*[r];
    for(int i=0; i<r; ++i){
        p[i]=new float[c];
    }
}
mat :: mat (int a, int b, float t){ 
    r=a;
    c=b;
    p=new float*[r];
    for(int i=0; i<r; ++i){
        p[i]=new float[c];
    }
    
      for(int i=0;i<r;++i){
        for(int j=0;j<c;++j){
            p[i][j]=t;
        }
    }
}
int main(){
    mat m1(3,3),m2(3,3),m3(3,3,0);
    cout<<"\nInitialize M1";
    m1.initialize();
    cout<<"\nInitialize M2";
    m2.initialize();
    m3=m1+m2;
    cout<<m3;
    return(0);
}

i'm new to coding.Help me out in easy language please.

shubham737
  • 37
  • 5
  • 1
    Your `mat` class is not safely copyable or assignable, yet you are returing it in `operator +`. This is a [rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) violation (see the **Managing Resources** section at that link). Either write the requisite user-defined copy constructor, assignment operator, and destructor, or use `std::vector> p;`. If you did the latter, your code may just magically start to work. – PaulMcKenzie Dec 19 '20 at 10:09
  • 1
    e.g. `mat tmp;` you copy to tmp although you havent allocated any space tmp.p. use a std::vector instead then many of your problems will disappear. – AndersK Dec 19 '20 at 10:14
  • Read [this C++ reference](https://en.cppreference.com/w/cpp) then the documentation of your C++ compiler, perhaps [GCC](https://gcc.gnu.org/). If you use GCC compile your code with all warnings and debug info, so with `g++ -Wall -Wextra -g`. Then read documentation of your debugger (perhaps [GDB](https://www.gnu.org/software/gdb/)...). At last use `gdb` to understand the behavior of your program. Take inspiration from existing open source C++ software, e.g. [fish](https://fishshell.com/), [FLTK](https://fltk.org/), [RefPerSys](http://refpersys.org/) – Basile Starynkevitch Dec 19 '20 at 10:16
  • 1
    You also failed to initialize your member variables in the `mat` default constructor. Basically your `mat` class is broken in several different ways, so broken that even thinking about writing an `operator +` for it is premature. – PaulMcKenzie Dec 19 '20 at 10:17
  • @AndersK thanks a lot.it worked. ...i realised what mistake i made in the process. – shubham737 Dec 19 '20 at 10:20
  • @PaulMcKenzie i started c++ like 4 days ago brother. i'm working on it. – shubham737 Dec 19 '20 at 10:21
  • @shubham737 Then forget about `new[]` and use `std::vector`. There is no need to get into the weeds of dynamically allocated memory with `new[]` (which is now not recommended or desired in modern C++ programs at the user-level). – PaulMcKenzie Dec 19 '20 at 10:23
  • @PaulMcKenzie will right away study about `std::vector`. – shubham737 Dec 19 '20 at 10:27

1 Answers1

3

The mat tmp; in the operator+ is not initialized. It uses default argumentless constructor which does nothing, and, in particular, does not set the p pointer to anything, which causes the segmentation fault when you try to assign to its contents. A good operator+ should check if both matrices have the same dimensions and the create the result matrix using the two-argument constructor.

This should answer the question but I will also elaborate on PaulMcKenzie's comment because it's important.

A class like this has no defined special behaviour when it is assigned, moved or destroyed, which is unacceptable when dealing with raw pointers. For example, when you assign one object of this class to another, the fields are simply copied, which means that two objects share the same p pointer which is probably not what you want. Your object also does not clean up its memory upon destruction which means that you have several memory leaks in your program.

In order to have a functioning class that uses raw pointers, you'll need

mat(const& mat other); - copy constructor

mat& mat::operator=(const& mat other); - copy assignment operator

mat(mat&& other) noexcept; - move constructor (C++11 and above)

mat& mat::operator=(mat&& other) noexcept; - move assignment operator (C++11 and above)

~mat(); - destructor

If you don't provide destructor, you have a memory leak each time object of this class is destroyed. If you provide destructor but copy/move stays at default, it will crash because after you copy or move an object, two objects will hold the same p pointer and after the two objects are destroyed, the pointer will be freed twice which causes a crash.

An easy fix would be using std::vector, as PaulMcKenzie suggested, it has all the memory management done for you.

If you want to write them yourself, please look up "rule of 0"/"rule of 3"/"rule of 5".

On a semi-related note, mat mat :: operator+(mat x) is declared wrong, as it creates an unnecessary copy of the argument and is not const. A proper way to declare it would be mat mat :: operator+(const mat& x) const, which passes the argument as a const reference, and is also const itself, meaning it can be used on a const variable.

Acvaxoort
  • 66
  • 2