Sig Error: Testing out a matrix class

[sonic1015]

*sigh* I'm stumped. I'm attempting some practise problems to get familiar with classes, but I keep getting this annoying Sig error, and then my console freezes up. I'm sure that there's something stupid I'm doing wrong so any insight would be great. If I had to guess, it's probably my return statement in the overloaded +-operator but I'm really not very certain at all.

Also, any added advice about whether my code is properly laid out would be welcome as well, and whether there is a better way (that is within the scope of my newbie knowledge). Here are the files:

this is matrix.cpp:

#include "matrix.h"

int main () {
	matrix m1(2, 2); //this works
	m1.input(); //this works
	m1.display(); //this displays
	m1 = m1 + m1; //debug statements (commented out) all iniate, but...
	m1.display(); //...never displays this.
}

this is matrix.h:

#include <iostream>
using namespace std;

class matrix {
	
	int ** n;
	
	protected:
	//array, width, & height
	int x;
	int y;
	
	public:
	
	//const & destr
	matrix(int, int);
	~matrix();
	
	//matrix functions
	void input();
	void display();
	friend matrix operator+(matrix, matrix);
	void operator=(matrix);
};

matrix::matrix(int a, int b) {
	n = new int * [a];
	
	for(int i = 0; i < a; i++) {
		n[i] = new int [b];
	}
	
	x = a;
	y = b;
	
}//contructor

matrix::~matrix() {
	for(int i = 0; i < x; i ++)
	{
		delete[] n[i];
	}
	
	delete[] n;
}//destructor

void matrix::input() {
	for(int j = 0; j < y; j++) {
		for(int i = 0; i < x; i++)
			cin >> n[i][j];
	}
}//input

void matrix::display() {

	for(int j = 0; j < y; j++) {
		for(int i = 0; i < x; i++) {
			cout << n[i][j] << " ";
		}
		cout << endl;
	}
}//display

void matrix::operator=(matrix m) {
	//cerr << "op= start" << endl;
	
	if((m.x != x) || (m.y != y)) {
		cerr << "non-compatible matrix" << endl;
	}
	else {
		for(int i = 0; i < x; i++) {
			for(int j = 0; j < y; j++)  {
				n[i][j] = m.n[i][j];
			}		
		}
	}
	
	//cerr << "op= end" << endl;
}//= operator

matrix operator+(matrix m1, matrix m2) {
	//cerr << "op+ start" << endl;
	
	if((m1.x != m2.x) || (m1.y != m2.y)) {
		cerr << "non-compatible addition" << endl;
		matrix temp(0,0);
		return temp;
	}
	
	matrix temp(m1.x, m2.y);
	
	for(int i = 0; i < m1.x; i++) {
		for(int j = 0; j < m2.y; j++)  {
			temp.n[i][j] = m1.n[i][j] + m2.n[i][j];
			//cout << temp.n[i][j] << " "; //note: this prints it inverted.
		}		
		//cout << endl;
	}
	
	//cerr << "op+ end" << endl;
	return temp;
	
}// + operator 

compiled with g++. Here is the matrix.exe.stackdump incase it proves useful.

Stack trace:
Frame Function Args
0028C934 756A1184 (00000134, 0000EA60, 00000000, 0028CA58)
0028C948 756A1138 (00000134, 0000EA60, 000000A4, 0028CA3C)
0028CA58 610BADB3 (00000000, 00000134, 0028CA78, 00000000)
0028CB38 610B7A37 (00000000, 00000000, 00000000, 00000000)
0028CB88 610B7E4B (00000C08, 0028CBB0, 6C4F5920, 6C4B65D4)
0028CC48 610B7F71 (00000C08, 00000006, 0028CC78, 610B8015)
0028CC58 610B7FAC (00000006, 0028CE88, 00000007, 0028CCC0)
0028CC78 610B8015 (00CF0568, 00000001, 6C4F5920, 00CD0340)
0028CCE8 610E4ECD (0028CD0C, 0028CD0C, 0028CD18, 000000C0)
0028CD38 004019A3 (0028D008, 00000002, 611A0CCE, 61006DDA)
0028CD78 61006DDA (00000000, 0028CDB0, 610066E0, 7EFDE000)
End of stack trace

[jsmith]

Start by writing a proper copy constructor. If that doesn't fix the problem, then post again.

[binarybob350]

Proper copy constructor

matrix::matrix( const matrix& m )
{
	x = m.x;
	y = m.y;

	n = new int * [x];
	
	for(int i = 0; i < x; i++)
	{
		n[i] = new int [y];

		for( int j=0; j < y; j++ )
			n[i][j] = m.n[i][j];
	}
}

[sonic1015]

Thanks, I forgot about the copy constructor. Ok, now for some reason, it has decided it will abort the program after it's done with the display function. I have no idea what prompted this, so any insight would be greatful as well as any feedback as to how well the code is laid-out.

matrix.h:

#include <iostream>
using namespace std;

class matrix {
	
	int ** n;
	
	protected:
	//array, width, & height
	int x;
	int y;
	
	public:
	
	//const & destr
	matrix(int, int);
	matrix(const matrix &);
	~matrix();
	
	//matrix functions
	void input();
	void display();
	void setlh(int, int);
	void clean();
	
	//overloaded ops
	friend matrix operator+(matrix, matrix);
	matrix operator=(matrix);
};
//----------------------------------------------------
matrix::matrix(int a, int b) {
	this->setlh(a, b);
}//contructor

matrix::matrix(const matrix & m) {
	*this = m;
}//copy constructor

matrix::~matrix() {
	this->clean();
}//destructor

//--------------------------------------------------
void matrix::input() {
	for(int j = 0; j < y; j++) {
		for(int i = 0; i < x; i++)
			cin >> n[i][j];
	}
}//input

void matrix::display() {
	for(int j = 0; j < y; j++) {
		for(int i = 0; i < x; i++) {
			cout << n[i][j] << " ";
		}
		cout << endl;
	}
}//display

void matrix::setlh(int a, int b) {
	//cerr << "setlh start" << endl;
	n = new int * [a];
	
	for(int i = 0; i < a; i++) {
		n[i] = new int [b];
	}
	
	x = a;
	y = b;
	
	//cerr << "setlh end" << endl;
}//set length & height

void matrix::clean() {
	//cerr << "clean start" << endl;
	
	for(int i = 0; i < x; i ++)
	{
		delete[] n[i];
	}
	
	delete[] n;
	
	//cerr << "clean end" << endl;
}//clean/free mem

//-------------------------------------------------
matrix matrix::operator=(matrix m) {
	//cerr << "op= start" << endl;
	
	this->clean();
	this->setlh(m.x, m.y);
	
	for(int i = 0; i < x; i++) {
		for(int j = 0; j < y; j++)  {
			n[i][j] = m.n[i][j];
		}		
	}
	
	//cerr << "op= end" << endl;
	return *this;
}//= operator

matrix operator+(matrix m1, matrix m2) {
	//cerr << "op+ start" << endl;
	
	if((m1.x != m2.x) || (m1.y != m2.y)) {
		cerr << "non-compatible addition" << endl;
		matrix temp(0,0);
		return temp;
	}
	
	matrix temp(m1.x, m1.y);
	
	for(int i = 0; i < m1.x; i++) {
		for(int j = 0; j < m2.y; j++)  {
			temp.n[i][j] = m1.n[i][j] + m2.n[i][j];
			//cout << temp.n[i][j] << " "; //note: this prints it inverted.
		}		
		//cout << endl;
	}
	
	//cerr << "op+ end" << endl;
	return temp;
	
}// + operator  

[binarybob350]

Your copy constructor is wrong, look at the one I posted above. The way yours is written you end up with 2 objects pointing to the same dynamic memory, then both destructors will try and free the same memory.

[jsmith]

Also since operator= is declared to return by value and not by (const) reference, invoking
operator= may call the copy constructor, but the copy constructor calls operator=.

Never write the copy constructor in terms of operator=; the reverse should be done for
proper exception safety (which this class is far from).

[sonic1015]

Oh Ok, I think I get it. The copy constructor is only used to initiate new values so requires no clean-up (which is where I got my error) and no return value (which would call the copy constructor), whereas the assignment operator does require clean-up and return, but should return a reference and not a value because we'd be calling the copy constructor again?

I did get it to work, sorry binary, I didn't realize that teh copy constructor was only used on initiation, and thought I could be lazy and just call the assignment operator :|.

Anyways, it is working perfectly now, I just want to make sure this is all right, and there isn't something that would have made this better.

#include <iostream>
using namespace std;

class matrix {
	
	int ** n;
	
	protected:
	//array, width, & height
	int x;
	int y;
	
	public:
	
	//const & destr
	matrix(int, int);
	matrix(const matrix &);
	~matrix();
	
	//matrix functions
	void input();
	void display();
	void setlh(int, int);
	void clean();
	
	//overloaded ops
	matrix & operator=(const matrix &);
	friend matrix operator+(const matrix &, const matrix &);
};
//----------------------------------------------------
matrix::matrix(int a, int b) {
	this->setlh(a, b);
}//contructor

matrix::matrix(const matrix & m) {
	this->setlh(m.x, m.y);
	
	for(int i = 0; i < x; i++) {
		for(int j = 0; j < y; j++)  {
			n[i][j] = m.n[i][j];
		}		
	}
}//copy constructor

matrix::~matrix() {
	this->clean();
}//destructor

//--------------------------------------------------
void matrix::input() {
	for(int j = 0; j < y; j++) {
		for(int i = 0; i < x; i++)
			cin >> n[i][j];
	}
}//input

void matrix::display() {
	for(int j = 0; j < y; j++) {
		for(int i = 0; i < x; i++) {
			cout << n[i][j] << " ";
		}
		cout << endl;
	}
}//display

void matrix::setlh(int a, int b) {
	//cerr << "setlh start" << endl;
	n = new int * [a];
	
	for(int i = 0; i < a; i++) {
		n[i] = new int [b];
	}
	
	x = a;
	y = b;
	
	//cerr << "setlh end" << endl;
}//set length & height

void matrix::clean() {
	//cerr << "clean start" << endl;
	
	for(int i = 0; i < x; i ++)
	{
		delete[] n[i];
	}
	
	delete[] n;
	
	//cerr << "clean end" << endl;
}//clean/free mem

//-------------------------------------------------
matrix & matrix::operator=(const matrix & m) {
	//cerr << "op= start" << endl;
	this->clean();
	this->setlh(m.x, m.y);
	
	for(int i = 0; i < x; i++) {
		for(int j = 0; j < y; j++)  {
			n[i][j] = m.n[i][j];
		}		
	}
	//cerr << "op= end" << endl;
	return *this;
}//= operator

matrix operator+(const matrix & m1, const matrix & m2) {
	//cerr << "op+ start" << endl;
	
	if((m1.x != m2.x) || (m1.y != m2.y)) {
		cerr << "non-compatible addition" << endl;
		matrix temp(0,0);
		return temp;
	}
	
	matrix temp(m1.x, m1.y);
	
	for(int i = 0; i < m1.x; i++) {
		for(int j = 0; j < m2.y; j++)  {
			temp.n[i][j] = m1.n[i][j] + m2.n[i][j];
			//cout << temp.n[i][j] << " "; //note: this prints it inverted.
		}		
		//cout << endl;
	}
	
	//cerr << "op+ end" << endl;
	return temp;
	
}// + operator