Click here to Skip to main content
14,976,809 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
hi everybody.
i'm sorry if my doubt is trivial, but i can't figure out how to fix it.
i have a class that in constructor I allocate a 2d array like this:

C++
int i;
this->matrix = new double*[5];
for(i = 0 ; i < 5; i++)
    this->matrix[i]=new double[5];


and in the destructor i delete the 2d array like this.

C++
int i;
for(i = 0; i < 5 ; i++)
{
    if(this->matrix[i] != NULL)
    {
	delete[] this->matrix[i];
	this->matrix[i] = NULL;
    }
}
if(this->matrix != NULL)
{
    delete []this->matrix;
    this->matrix = NULL;
}


some times i have a heap corruption and I discovery that this happens in the destructor. I don't know if the 2d array allocation is wrong or is the destruction that is wrong.

PS: It's possible that in my program I allocate an array with 1000 elements and the program works fine, and sometimes give me out of memory error? if so, what causes this error? the memory management by OS?

Can anyone help me?
thanks in advance, Filipe
Posted
Comments
Albert Holguin 13-Sep-11 13:08pm
   
what do you have matrix defined as? is this all within the same class or are you passing the pointers around? one thing you can do is right down all the addresses allocated in the array, then see which one kicks an error, then figure out why, maybe you overwrote one of the pointers.

   
Comments
CPallini 13-Sep-11 13:01pm
   
From my point of view the OP code is correct. I don't think he really needs memory allocation tutorials.
In your destructor, the
if(this->matrix != NULL)
{


should go before and around the for loop - otherwise you are iterating over the contents of the outer array deleting its members, and then checking if it exists before you delete the outer array.
   
Comments
CPallini 13-Sep-11 15:51pm
   
Good point.
Filipe Marques 14-Sep-11 4:54am
   
thanks, make sense.
1. How many constructors does your class have? Your post implies there is only one. If that's correct your constructor should be fine; otherwise check your other constructors to make sure they use the same code (or better, call the same function for the allocation)

2. Do you pass out a pointer to your matrix or matrix rows outside your class? If so, make sure that the callers don't try and delete[] these pointers!

3. What happens when you copy an object of the class type that contains this matrix? If you don't have a copy constructor - and according to 1. you probably don't - then the compiler creates one for you, and that copy constructor will only copy the pointer of your matrix, not create a new copy of the structure on the heap as it should! As a result, when the copy is destructed, the matrix will be released, but the pointer in the original object will not be affected and upon destruction you will again try to delete it which causes a runtime error.

You should provide a copy constructor of your own instead. It needs to do a deep copy of your data so that destructing the object will not affect the matrix object of the original.

Note that a copy constructor may be invoked implicitely without you being aware of it, e. g. when passing an object by value to a function or passing one as a return value. Also, if you store your objects in any of the STL containers they may invoke the copy constructor.

4. See solution 3 regarding your destructor code.

My bets are on 3. ;)
   
v2
Comments
Filipe Marques 14-Sep-11 5:07am
   
Thanks for help.
I have a copy constructor in may class.
Your allocation/deallocation schema looks fine. Possibly the heap corruption depends on something else.
[update]
I made a test, no problems at all with the posted code.
[/update]
   
v2
I don't know if this, but I figure out a problem in my code.
In some part of it, I have the next:

C++
this->matrix = new double*[this->row];
this->matrix = NULL;
for(i = 0 ; i < row ; i++)
{
    this->matrix[i]=new double[this->collumn];
    this->matrix[i] = NULL;
}


and at the begin I put the first address equal to NULL, to initialize the array, but inside of for loop, it couldn't create the second dimension, because the address was NULL.
So what i meant was:

C++
this->matrix = new double*[this->row];
*this->matrix = NULL;
for(i = 0 ; i < row ; i++)
{
    this->matrix[i]=new double[this->collumn];
    *this->matrix[i] = NULL;
}
   
Comments
Legor 14-Sep-11 5:49am
   
Yes, your first variant would lead to access violations due to the reason you stated correctly.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)




CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900