Object Deallocated before it can be used.

JonReed (2)
Hello. I am currently working on a matrix class for my CIS class. I have however run into a bit of a snag with an object being deallocated before it can be used.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
 
// Matrix.cpp : Defines the entry point for the console application.
//
#include "MatrixType.cpp"

using namespace std;

int main()
{
	MatrixType<int> a(2,2);
	MatrixType<int> b(2,2);
	MatrixType<int> t(2,2);
	MatrixType<int> *res;

	a(1,1) = 1;
	a(1,2) = 2;
	a(2,1) = 3;
	a(2,2) = 4;

	b(1,1) = 1;
	b(1,2) = 2;
	b(2,1) = 4;
	b(2,2) = 5;

	cout << "Test" << endl;
	t = a+b; //This is the point of crash becuase the matrix returned by
                 // A+B is deallocated before t = can occur.
	cout << "Test" << endl;

	res = new MatrixType<int>(a+b);

	system("pause");
	return 0;
}


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
 
#ifndef matrixType_h
#define matrixType_h
#include <iostream>

template<class elemType>
class MatrixType
{
	int rowS;
	int colS;
	elemType *matrix;
public:
	//operator overloads
	elemType& operator() (int row, int col);
	elemType  operator() (int row, int col) const;

	MatrixType<elemType>& operator+= (const MatrixType& other);
	MatrixType<elemType>& operator-= (const MatrixType& other);
	MatrixType<elemType>& operator*= (const MatrixType& other);

	const MatrixType<elemType>& operator+ (const MatrixType& other) const;
	const MatrixType<elemType>& operator- (const MatrixType& other) const;
	const MatrixType<elemType>& operator* (const MatrixType& other) const;
	const MatrixType<elemType>& operator/ (const MatrixType& other) const;
	MatrixType<elemType>& operator= (const MatrixType& other);
	//util functions
	void print() const;

	//constructors
	MatrixType(int rows = 1, int collumns = 1);
	MatrixType(const MatrixType& other);
	~MatrixType();
};

#endif 


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
 
#include "matrixType.h"
#include <assert.h>

template<class elemType>
MatrixType<elemType>& MatrixType<elemType>::operator=(const MatrixType& other)
{
	if(this == &other)
		return *this;
	else
	{
		delete [] matrix;
		rowS = other.rowS;
		colS = other.colS;
		matrix = new elemType[rowS*colS]; //This is where the program
                         //crashes when it tries to make an array of 0
                         // elements, due to A+B deallocating before it can be 
                         // used.
		assert(matrix != NULL);
		for(int k = 0; k < rowS; k++)
			for(int i = 0; i < colS; i++)
				matrix[k*colS+i] = other.matrix[k*colS+i];
	}
	return *this;
}

template<class elemType>
MatrixType<elemType>& MatrixType<elemType>::operator+= (const MatrixType& other) 
{
	if(this->rowS == other.rowS & this->colS == other.colS)
	{
		for(int i = 0; i < this->rowS; i++)
			for(int k = 0; k < this->colS; k++)
				this->matrix[i*colS+k] = this->matrix[i*colS+k]+other(i+1,k+1);
	}
	else
	{
		cerr << "Different Sizes." << endl; 
	}
	return *this;
}

template<class elemType>
MatrixType<elemType>& MatrixType<elemType>::operator-= (const MatrixType& other) 
{
	if(this->rowS == other.rowS & this->colS == other.colS)
	{
		for(int i = 0; i < this->rowS; i++)
			for(int k = 0; k < this->colS; k++)
				this->matrix[i*colS+k] = this->matrix[i*colS+k]-other(i+1,k+1);
	}
	else
	{
		cerr << "Different Sizes." << endl; 
	}
	return *this;
}

template<class elemType>
MatrixType<elemType>& MatrixType<elemType>::operator*= (const MatrixType& other)
{
	if(this->colS == other.rowS)
	{
		MatrixType<elemType> temp(this->rowS, other.colS);
		for(int i = 1 ; i <= temp.rowS ; i++)
			 for(int j = 1 ; j <= temp.colS ; j++)
			 {
				temp(i,j) = 0;
				for(int k = 1 ;k <= temp.colS ; k++)
				   temp(i,j) += (*this)(i,k)*temp(k,j);
			 }
		*this = temp;
	}
	else
		cerr << "Number of Columns in A and Rows in B are not the same." << endl;
	return *this;
}

template<class elemType>
const MatrixType<elemType>& MatrixType<elemType>::operator+ (const MatrixType& other) const
{
	return MatrixType<elemType>(*this) += other;
}

template<class elemType>
const MatrixType<elemType>& MatrixType<elemType>::operator- (const MatrixType& other) const
{
	return MatrixType<elemType>(*this) -= other;
}

template<class elemType>
const MatrixType<elemType>& MatrixType<elemType>::operator* (const MatrixType& other) const
{
	return MatrixType<elemType>(*this) *= other;
}

template<class elemType>
const MatrixType<elemType>& MatrixType<elemType>::operator/ (const MatrixType& other) const
{
	if(this->colS == other.rowS)
	{
		MatrixType<elemType> result(this->rowS, other.colS);
		for(int i = 1 ; i <= result.rowS ; i++)
			 for(int j = 1 ; j <= result.colS ; j++)
			 {
				result(i,j) = 0;
				for(int k = 1 ;k <= result.colS ; k++)
				   result(i,j) += (*this)(i,k)/other(k,j);
			 }
	}
	return result;
}

template<class elemType>
elemType& MatrixType<elemType>::operator() (const int row, const int col)
{
	if(row < 1 || row > rowS || col < 1 || col > colS)
		cerr << "Selection Out of Bounds: " << endl;
	else
		return matrix[(row-1)*colS+col-1];
}

template<class elemType>
elemType MatrixType<elemType>::operator() (const int row, const int col) const
{
	if(row < 1 || row > rowS || col < 1 || col > colS)
		cerr << "Selection Out of Bounds: " << endl;
	else
		return matrix[(row-1)*colS+col-1];
}

template<class elemType>
void MatrixType<elemType>::print() const
{
	for(int i = 0; i < rowS+1; i++)
		for(int k = 0; k < colS+1; k++)
			cout << "[" << (i+1) << "," << (k+1) << "] " << matrix[i*colS+k] << endl;
}

//constructors
template<class elemType>
MatrixType<elemType>::MatrixType(int rows, int cols)
{
	if(rows < 1 || cols < 1)
	{
		cerr << "The matrix must be atleast 1x1. Creating a 1x1 matrix."<<endl;
		rowS = 1;
		colS = 1;
	}
	else
	{
		rowS = rows;
		colS = cols;
	}

	matrix = new elemType[rowS*colS];
	assert(matrix != NULL);
}

template<class elemType>
MatrixType<elemType>::MatrixType(const MatrixType& other)
{
	rowS = other.rowS;
	colS = other.colS;
	matrix = new elemType[rowS*colS];
	assert(matrix != NULL);
	for(int k = 0; k < rowS; k++)
		for(int i = 0; i < colS; i++)
			matrix[k*colS+i] = other.matrix[k*colS+i];

}

template<class elemType>
MatrixType<elemType>::~MatrixType()
{
	delete [] matrix;
}

TheDestroyer (441)
That's just not true. You're claiming that a is getting deallocated. That can't be possible because a is still in scope. Something else is wrong. Debug your operator+ and operator+=.
andywestken (4094)
operator*, operator+, etc should return by value rather than by reference.

As it stands, you're returning the address of a temporary matrix; when operator* (etc) returns, the temporary's destructor is triggered, so you're left with a dodgy reference.

operator*=, etc are the ones where you need a reference.

Andy

PS I see you are using evil 1-based indexing. Is this a requirement of some kind?
Last edited on
TheDestroyer (441)
operator*, operator+, etc should return by value rather than by reference.


This is the case only if you're dealing with friend operator*,... . Returning the reference is 100% correct here, because (*this) has to be returned, which doesn't have to be copied.
andywestken (4094)
This code (from above)

1
2
3
4
5
 
template<class elemType>
const MatrixType<elemType>& MatrixType<elemType>::operator* (const MatrixType& other) const
{
	return MatrixType<elemType>(*this) *= other;
}


is not returning *this.

It's returning an anonymous, new matrix instance which has been created using the copy constructor, which has then had operator*= applied to it (rather then the original instance).

The address of this anonymous variable is then being returned just before it's destroyed.

In less terse form, this method can be reexpressed as

1
2
3
4
5
6
7
 
template<class elemType>
const MatrixType<elemType>& MatrixType<elemType>::operator* (const MatrixType& other) const
{
	MatrixType<elemType> newValue(*this);
	newValue *= other;
	return newValue;
}


which is almost a pretty standard form of operator* (pretty much cannonical, save for the return type).

operator*= does, of course, return *this
Last edited on
JonReed (2)
Thank you Andy Westken. I understand were I went wrong now. Just to make sure I understand.

Instead of passing the actual object back from + - * /, I was passing back the address to the temporary object. When it returned this address,the temporary object would go out of scope causing the address returned to point to nothing. Then when I would attempt to copy the object the pointer was pointing to, it would grab incorrect values leading to the new object trying to make an array of 0 or less length.

That about right?

The 1-based indexing is because matrices are labeled (1,1) (1,2)...and so on. Even though it is only classwork, I decided that it should resemble how matrices are labeled.
Last edited on
andywestken (4094)
Yep, that's about right.

But rather than pointing to nothing, the reference will be pointing to memory which has been popped off the call stack and is ready to be reused when making the next call. Depending on the circumstance, the memory could be cleaned up, reused, or maybe just left alone.

This kind of bug can lead to erratic crashes and the like. If you use the reference immediately after the function returns you may well get away with using it. But then you add a call to another function and things get very messy.

Here I think it's your destructor helping to make things behave consistently.
Topic archived. No new replies allowed.