De-allocating memory from a 2d pointer in C++

[PolteRGeisT]

Can anyone explain why this segfaults??


matrix_t::~matrix_t( void )
{
// free matrix memory
for( register int i = 0; i < row_size; ++i ) // FIXME : CRASHES!!!!!!!!!
delete [] matrix[i];
delete [] matrix;
}


Which was allocated like so...


matrix_t::matrix_t( unsigned int r_size, unsigned int c_size, unsigned int series_size )
:row_size(r_size),col_size(c_size)
{
connect_series_size = series_size;

// allocate matrix memory
matrix = new chip_slot_t*[row_size];
for( register int i = 0; i < row_size; ++i )
matrix[i] = new chip_slot_t[col_size];
}

[mnsharif]

if u allocate memory through malloc or new function thyen delete will work otherwise not

[Energon]

Are you sure using ++i gives you the exact same results as i++? Also, have you run the code through gdb to see exactly which line is faulting and what the values of all your variables are when it faults?

[Stuka]

Call me crazy, but this section:// allocate matrix memory
matrix = new chip_slot_t*[row_size];
for( register int i = 0; i < row_size; ++i )
matrix[i] = new chip_slot_t[col_size]; jumps out at me. You're new'ing an array of chip_slot_t pointers, and then trying to create each of them as an array of new chip_slot_t objects. IIRC, you need to, in the end, call new for each individual object. I THINK what you ought to do should look like this:matrix = new chip_slot_t**[row_size];
for (int i = 0; i < row_size; ++i){
matrix[i] = new chip_slot_t*[col_size];
for (int j = 0; j j< col_size; ++j){
matrix[i][j] = new chip_slot_t;
}
}Note that I've just fired this off, and haven't tested it, but I'm fairly sure that the idea, if not all the code, is right. ;)

[bwkaz]

Stuka -- not quite. You have to do something like that if you don't want to use the new[] operator, but just the new operator. Your code should then look something like this:

matrix = <some allocated memory space, enough for row_size chip_slot_t * objects>;

for (int i = 0; i < row_size; ++i){
matrix[i] = <more allocated memory, enough for col_size chip_slot_t objects>;
for (int j = 0; j j< col_size; ++j){
matrix[i][j] = new chip_slot_t;
}
} However, just using new[] works much better, and you do it just like PolteRGeisT posted:

matrix = new chip_slot_t *[row_size]; // Creates consecutive space for row_size
// chip_slot_t pointers, and sets matrix (a chip_slot_t **) to the first address

for(i=0; i<row_size; i++) {
matrix[i] = new chip_slot_t[col_size];

// You could have another for loop here to initialize each chip_slot_t, but it
// isn't strictly necessary if chip_slot_t has a default constructor
} However, it would be nice to know which line (exactly) is segfaulting, so compiling with -g and then running it through gdb would be a good idea.

Originally posted by mnsharif
if u allocate memory through malloc or new function thyen delete will work otherwise not No, not really. If you use new, then you can (have to) use delete. If you use new[], you have to use delete[] (the reason is that the compiler doesn't know the difference between a pointer to X and an array of 10 X objects). If you use malloc, you have to use free. Mixing malloc-type memory management with new/delete-type memory management will corrupt your heap, eventually...

[Stuka]

ah - I've never used new[], but I suppose I should've known you could, since delete[] is available. Add one more bit of knowledge to the ol' pile.

[PolteRGeisT]

matrix_t::~matrix_t( void )
{
// free matrix memory
for( register int i = 0; i < row_size; ++i )
delete [] matrix[i]; // This line crashes the program upon the first itteration. I have no clue why.
delete [] matrix;
}

[Energon]

Again, just out of curiosity, what is the value of i and row_size? I really don't like the ++i as I'm not sure if it's going to increment before the first iteration, and if what I'm thinking is right, that would be the problem.

The next thing to look at is the value of each array upon allocation and right before de-allocation (EDIT - ie, what address does the pointer to the array point to?).

Last, what is the type of the matrix member? If it's not a basic type, then that too could be a potential problem.

As just an aside (this thread got me thinking)... what would be the ultimate result of a class that has a member of it's own type that is allocated via new in the constructor. I think recursive stack overflow, but I'm not entirely sure if that's the case (and possibly the compiler will pick it out as an error?). EDIT - and that's what happened... interesting though that the compiler can't see it as a problem.

[PolteRGeisT]

You raise some interresting questions. I'll check into what you have mentioned once I can get back to where my code is. Thanks.

[bwkaz]

AFAIK, in a for loop, i++ and ++i will do the same thing (assuming i is an int, which it is). The only difference between them is that ++i increments i, then returns the new value, while i++ returns the current value, then increments i. In this for loop, since you're not using the value returned by ++i, it shouldn't matter.

But printing out the value of row_size and i before each delete would probably be a good idea...

[truls]

The code compiles and runs fine (I've only got Visual C++ here, shame on me. Anyways, compiled and ran it - no bluescreen).
Can you post the declaration of matrix(chip_slot_t** matrix, right?),
and also - is there any way that row_size can change?
But put some cout statements in there and run the thing,
at least you'll find out what isn't wrong and that's half the struggle.

Weather(sp?) it's i++ or ++i doesn't matter, since the last statement in a for loop is executed AFTER the loop is finished.
Really people, you ough to know at least this much :p

And who messed up their tags, it's all over the place now...:eek:

[PolteRGeisT]

It's alright. The problem was the chip_slot_t destructor (which I haven't posted), I made a change and neglected to adjust that function accordingly. Thanks for all the advice. :)