1

I've problem with easy C++ program which finds the largest submatrix in given matrix:

int *A = new int[n*m];
... setting fields for matrix, finding the largest one and etc
... r := size of square-submatrix, max_i := row, max_j := column of the largest
for (i = max_i; i < max_i + r; i++)
{
    for (j = max_j; j < max_j + r; j++)
        cout << A[i * n + j] << "\t";

        cout << "\n";
} 
<memory free>
end of program

Everything works great (so it's not problem with logic) - finding correct submatrix, prinitng etc.

Unexpectedly (or due to late night) when I put in memory free line delete[] A or delete everything crushes down with errors (but it's still printing result correctly - so the error must be in this line). I've tried set it to NULL and every combination. What goes wrong?

Thanks in advance

EDIT:

#include <iostream>
#include <cstdlib>

using namespace std;

int main()
{
int i,j,k; 

int m,n; 
int r; 

cout << "Size of matrix: \nRows: ";
cin >> n;
cout << "Columns: ";
cin >> m;

int *A = new int[n*m];

for (i = 0; i < n; i++)
{
    for (j = 0; j < m; j++)
    {
        A[n*i + j] = rand() % 19 - 9;
        cout << A[n*i + j] << "\t";
    }

    cout << "\n";
}


cout << "Size of submatrix: ";
cin >> r;

int liczba_kwadratow = (m + 1 -r) * (n+1 -r); 

int max = -10000;
int max_i = 0;
int max_j = 0;

int row_iter = 0;
int col_iter = 0;   

for (k = 0; k <liczba_kwadratow; k++)
{
    int sum = 0;
    for ( i = row_iter; i < row_iter + r; i++)
        for ( j = col_iter; j < col_iter + r; j++)
            sum += A[i * n + j];    

    if ( sum > max )
    {
        max = sum;
        max_i = row_iter;
        max_j = col_iter;
    }

    col_iter++;
    if ( row_iter + r > m )
    {
        row_iter++;
        col_iter = 0;   
    }

}

cout << "Field of the largest submatrix  " << r << " of " << r << " equals " << max << "\n";

for (i = max_i; i < max_i + r; i++)
{
    for (j = max_j; j < max_j + r; j++)
        cout << A[i * n + j] << "\t";

    cout << "\n";
}

...works great without delete[] A or delete A

}
6
  • Any chance you are writing to A[-1]? Commented Jan 14, 2015 at 0:36
  • I don't think so. I've posted code. Thanks! Commented Jan 14, 2015 at 0:41
  • 1
    use some asserts to make sure you are not perform any out-of-bound access, or better yet, use a std::vector and member function at() to perform the access. This way, if you attempt to access out-of-bounds, an exception is being thrown (this is not the case if you simply use std::vector::operator[]). If you use such a vector, can easily test what's going on. And as a general rule, you should try avoiding using raw pointers in modern C++. Commented Jan 14, 2015 at 0:43
  • Yes - it's a way I used to do that kind of things - but I'm helping friend of mine and it must be done in this way (dynamically allocated arrays). Commented Jan 14, 2015 at 0:45
  • 1
    Unless I'm too tired to read anymore, your multipliers are wrong. It should be i multipled by the stride of a row. you're multiplying by the height of a column. i*m+j should be your index value. n should only appear in those loops in one place: the top < limit for i in the outer-for-loop. Commented Jan 14, 2015 at 0:52

2 Answers 2

1

The problem can at least be identified via the method(s) I've told you in the comments (or by using a debugger). Easiest way is changing the dynamical array to a std::vector then use member function at. You will then realize you get an out of bound exception thrown here:

for (i = 0; i < n; i++)
{
    for (j = 0; j < m; j++)
    {
        A.at(n*i + j) = rand() % 19 - 9; // throws here!!!!, replace n by m
        cout << A.at(n*i + j) << "\t";
    }

    cout << "\n";
}

When input is 6 5 3, then you get an exception when you first try to access A[30], which gives you a headache when delete[] - ing. Now you can figure out what's wrong I guess ...

Sign up to request clarification or add additional context in comments.

3 Comments

Of course it was the problem. Thanks for your time and help - it seems I was too tired to distinguish n and m. Btw - What's the point in teaching C++ in that old fashioned C-like way?
@lemoid absolutely no point in my opinion, one should start learning from the very beginning with modern C++, i.e. learn the Standard Library and best practices of its usage.
Yeah - it is sad, but some tutors or even PhD teach C++ in a C-way + <iostream>, classes (without basic distinguish between classes and structs) and new/delete - sometimes they're still using mallocs! Undoubtedly it's good to know such a low-level memory management, but I agree - I cannot see a point in using C++ to that kind of stuff. Cheers!
1

You inverted i and j in your matrix access code.

basically the formula could be

current row + current column * number of rows

or

current row * number of columns + current column

Two remarks on your code and debugging:

  1. using one-letter variables minimizes the wear and tear on your keyboard and fingers and reduces disk and memory usage. It tends to increase debugging time dramatically, though.

  2. The assumption that the code is OK just because it does not crash immediately is absolutely wrong. C/C++ is the realm of undefined behaviours (in your case, probably the most common one: writing outside allocated memory). An UB is a nasty piece of work precisely because it can produce any outcome, including (frequently) an apparent absence of consequences, only to cause a perfectly OK bit of code to crash 10.000 lines later.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.