C++: ostream infinite recursion

Question

I am somehow causing infinite recursion (and an eventual stack overflow) to occur inside this destructor:

MyMatrix::~MyMatrix() {
    if (this != NULL) {
        cout << "Destructor called for " << this->matrix << ":" << endl << *this << endl;
        /*for (int i = 0; i < m; i++) {
            for (int j = 0; j < n; j++) {
                cout << matrix[i][j] << " ";
            }
            cout << endl;
        }*/
        delete[] *matrix;
        delete[] matrix;
    }
}

If I uncomment the for() loops and remove the end of the initial cout, the function works fine. Therefore, I think that it is caused by the overloaded << operator:

ostream &operator<<(ostream &output, MyMatrix a)
{
    if (&a != NULL) {
        for (int i = 0; i < a.m; i++) {
            for (int j = 0; j < a.n; j++) {
                output << a.matrix[i][j] << " ";
            }
            output << endl;
        }
    }
    return output;
}

EDIT: here is the constructor

MyMatrix::MyMatrix(int i_m, int i_n) {
    m = i_m;
    n = i_n;
    if (n < 1 || m < 1)
        throw string("Dimensions cannot be negative");
    matrix = new float*[m];
    for (int i = 0; i < m; i++) {
       matrix[i] = new float[n];
       for (int j = 0; j < n; j++)
           matrix[i][j] = 0;
    }
}

Your `<<` takes the matrix by value. Your destructor calls `<<`, passing it a copy of the matrix. Then that copy needs to be destroyed, which calls the destructor, which calls `<<` and passes it a copy of the matrix, which then needs to be destroyed, which calls the destructor... I think you can see where this is going. — T.C., Sep 12 '17 at 23:43
As @T.C. says, you should be calling by const-reference in the `<<` operator, but also, `this != NULL` and `&a != NULL` can never be true. You should remove them. — Andrew Lazarus, Sep 12 '17 at 23:48
By the way, it's not possible for `this` or `&a` to be a null pointer. — aschepler, Sep 12 '17 at 23:49
Good odds that reading this question and answer will help stave off your next bug: [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — user4581301, Sep 12 '17 at 23:54

score 2 · Accepted Answer · edited Sep 12 '17 at 23:57

The problem is in your declaration of operator<<:

ostream &operator<<(ostream &output, MyMatrix a);

You are passing a by value. This causes a temporary copy to be made of the passed Matrix, and that copy is destructed when operator<< exits. When you call operator<< inside the Matrix destructor, you cause a recursive loop.

You should avoid passing function arguments by value whenever possible. Avoid making unnecessary copies, this slows down your program, as it generates extra code (in this case, a copy constructor and a destructor).

Change your definition of operator<< to what it should have been from the start:

ostream &operator<<(ostream &output, const MyMatrix &a);

A side note: from the code you showed, it seems that you have a Matrix containing a Matrix*. This is a recursive structure. I doubt this is really necessary for a matrix.

C++: ostream infinite recursion

1 Answers1