0

I have a class called Matrix with 2 dimensional pointer bool** matrix.

look at this code :

void Matrix::operator=(const Matrix& A){ 

    cout << "address in A : " << A.matrix << endl ;
    cout << "address in y : " << matrix << endl ;

    //return *this;
}

I called my = operator in my main function like this :

Matrix x(3,5);
Matrix y(3,5);

x.set(1,4,1);
cout << "address out X: " << x.matrix << endl;
cout << "address out Y: " << y.matrix << endl;
y = x;
cout << "address out X: " << x.matrix << endl;
cout << "address out Y: " << y.matrix << endl;

The desctructor is like this :

Matrix::~Matrix(){
    cout << "address de : " << matrix <<endl;
    for (int i=0;i<m;i++)
        delete[] matrix[i];
    delete[] matrix;
    cout << "address de finish : " << matrix <<endl;
}

When I run my program in xcode i get :

address out X: 0x100103ab0
address out Y: 0x100103af0
address in A : 0x100103ab0
address in y : 0x100103af0
address out X: 0x100103ab0
address out Y: 0x100103af0
address de : 0x100103af0
address de finish : 0x100103af0
address de : 0x100103ab0
address de finish : 0x100103ab0

and it's look fine but when i change the = operator function like this :

Matrix Matrix::operator=(const Matrix& A){

    cout << "address in A : " << A.matrix << endl ;
    cout << "address in y : " << matrix << endl ;

    return *this;
}

I get this as result :

address out X: 0x100103ab0
address out Y: 0x100103af0
address in A : 0x100103ab0
address in y : 0x100103af0
address de : 0x100103af0
address de finish : 0x100103af0
address out X: 0x100103ab0
address out Y: 0x100103af0
address de : 0x100103af0
Thesis(26190) malloc: *** error for object 0x100103b10: pointer being freed was not allocated

can anyone explain to me why the destructor triggers sooner in the latter code ?! and how can i prevent it

thank you in advance

Shnd
  • 1,846
  • 19
  • 35
  • Can you post your declaration for your class and also the code that populates (initializes) your object? – Paul Dardeau Jul 24 '13 at 15:17
  • 1
    Quick advice: throw all this away, and write a simple wrapper around `std::vector`. Writing the whole thing will probably be less work than fixing what you have now. – Jerry Coffin Jul 24 '13 at 15:18
  • Mark B's answer is correct, but if you're getting that error, you have other problems, most likely in your copy constructor, which I'm guessing you failed to write. – Benjamin Lindley Jul 24 '13 at 15:21

2 Answers2

8

The reason is that your modified copy assignment operator returns by value which creates a copy of the matrix, returns it, and then destroys it.

The canonical signature for your copy assignment operator would be Matrix& Matrix::operator=(const Matrix& A) (note that I'm returning by non-const reference).

EDIT: Keep in mind that if you just use vector all this memory management just goes away and all these functions can use the compiler defaults. Although if you are in fact storing bools in the matrix, vector<bool> is specialized and you should just make sure you understand how that specialization interacts with your code.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
Mark B
  • 95,107
  • 10
  • 109
  • 188
  • It also sounds like he's using the default copy constructor, which copies the value of the pointer, rather than making a copy of the data. – Drew McGowen Jul 24 '13 at 15:19
  • Thanks for your quick reply, and you are right, I change the = operator function to return reference, and it works as a charm. one last question: i'm going to use and compile this code on other OS es like windows or linux. is the vector class standard in other compilers ? – Shnd Jul 24 '13 at 15:29
4

Your assignment operator is returning *this by value, making a copy of it. Normally, you would return a reference:

Matrix& Matrix::operator=(const Matrix& A)
//    ^

The copy is made using the implicit copy constructor, which simply copies the member pointer. You now have two objects which both think they "own" the allocated memory. The temporary value returned by operator= is soon destroyed, deleting the memory, and leaving y with a dangling pointer; you get the error when y tries to delete the same memory a second time.

This is why classes that manage resources should follow the Rule of Three, to ensure that they are safe to copy. Standard containers such as std::vector do this; it's usually a good idea to use these rather than managing memory yourself.

Community
  • 1
  • 1
Mike Seymour
  • 249,747
  • 28
  • 448
  • 644