List double free problem

[hal22]

Hello, I have a problem deleting lists. The function delete_all will
delete a complete list, starting with the entry given as parameter.
delete_one deletes one element of a list and returns the next. I want
to make these functions bullet proof, but it seems that I don't get
how to set the parameter to NULL after freeing the contents.

void delete_all( Monom* monom ){
while( monom != NULL ){
monom = delete_monom( monom );
}
}

static Monom*
delete_one( Monom* monom){
Monom *temp = monom->next;
free( monom );
monom = NULL;
return temp;
}

Lets say I have a list "test", the following gives an error:
delete_all( test );
delete_all( test );

This is fine:
delete_all( test );
test = NULL;
delete_all( test );

Thanks in advance.

Sorry for the double post, hit the wrong keys.

 

[maverik]

Hello, I have a problem deleting lists. The function delete_all will
delete a complete list, starting with the entry given as parameter.
delete_one deletes one element of a list and returns the next. I want
to make these functions bullet proof, but it seems that I don't get
how to set the parameter to NULL after freeing the contents.

void delete_all( Monom* monom ){
  while( monom != NULL ){
    monom = delete_monom( monom );

Mistyped here? -^^^ Do you mean delete_one() ?

}
}

static Monom*
delete_one( Monom* monom){
Monom *temp = monom->next;
free( monom );
monom = NULL;
return temp;

}

Without knowledge of inner structure of your list we can't help. Are
you sure that during the list construction you assign NULL pointer to
last entry:

monom->next = NULL;

Can your list has cyclic references?

monom1 -[next]---> monom2 -|
^ |
|-----[next]--------------

Oh, to change the pointer you should use pointer to pointer as
function argument:

static Monom*
delete_one( Monom **monom){
Monom *temp = (*monom)->next;
free( *monom );
*monom = NULL;
return temp;
}

 

[Richard Tobin]

Hello, I have a problem deleting lists. The function delete_all will
delete a complete list, starting with the entry given as parameter.
delete_one deletes one element of a list and returns the next. I want
to make these functions bullet proof, but it seems that I don't get
how to set the parameter to NULL after freeing the contents.

C function arguments are passed by value, so you can't modify the
caller's argument. You could achieve the same effect by passing in a
pointer to the list (that is, a Monom **), or by using a macro, but
it's not really worth the trouble. It doesn't work reliably - what if
the caller has another variable with the same value?

Lets say I have a list "test", the following gives an error:
delete_all( test );
delete_all( test );

This code is a mistake. You're deleting something you've already
deleted.

delete_all( test );
test = NULL;
delete_all( test );

This code still has a mistake. You haven't corrected it, just hidden
it.

-- Richard

 

[Lew Pitcher]

Hello, I have a problem deleting lists. The function delete_all will
delete a complete list, starting with the entry given as parameter.
delete_one deletes one element of a list and returns the next. I want
to make these functions bullet proof, but it seems that I don't get
how to set the parameter to NULL after freeing the contents.

void delete_all( Monom* monom ){
while( monom != NULL ){
monom = delete_monom( monom );
}
}

static Monom*
delete_one( Monom* monom){
Monom *temp = monom->next;
free( monom );
monom = NULL;
return temp;
}

Lets say I have a list "test", the following gives an error:
delete_all( test );
delete_all( test );

Your variable "test" is declared /outside/ of the scope of the delete_all()
function. Nothing /in/ your delete_all() will alter the value of test; your
code side of delete_all() which invokes delete_all() will have to alter the
value of test appropriately.

The simplest way I can think of to "fix" your problem is to modify your
delete_all() function so that it returns a NULL pointer on completion,

Monom * delete_all( Monom* monom ){
while( monom != NULL ){
monom = delete_monom( monom );
}
return monom /* will return NULL */
}

and change how you call delete_all() to capture this return pointer

test = delete_all( test );
test = delete_all( test );



HTH
--
Lew Pitcher

Master Codewright & JOAT-in-training | Registered Linux User #112576
http://pitcher.digitalfreehold.ca/ | GPG public key available by request
---------- Slackware - Because I know what I'm doing. ------

 

[Chris Dollin]

Hello, I have a problem deleting lists. The function delete_all will
delete a complete list, starting with the entry given as parameter.
delete_one deletes one element of a list and returns the next.

I think that's a perfectly horrid design.

I want
to make these functions bullet proof, but it seems that I don't get
how to set the parameter to NULL after freeing the contents.

Even if you could set the parameter to null, all that will do is
give you a false sense of security.

void delete_all( Monom* monom ){
while( monom != NULL ){
monom = delete_monom( monom );
}
}

static Monom*
delete_one( Monom* monom){
Monom *temp = monom->next;
free( monom );
monom = NULL;
return temp;
}

I don't think the `delete_one` method is giving you anything here.
The assignment `monom = NULL;` does /nothing/ useful; you're assigning
to a variable that will vanish as soon as the function returns.
If you just do:

void delete_all( Monom *m )
{
while (m != NULL)
{
Monom next = m->next;
free( m );
m = next;
}
}

you avoid the distraction that `delete_one` caused you.

You seem to be expecting that after `delete_all` the actual
argument in the call will be set to null. It won't. C doesn't
have "pass by reference": if you want to sing that song, you
have to sing it yourself:

void delete_all( Monom **rm )
{
Monom *m = *rm;
<<body of delete_all above>>
*rm = NULL;
}

and then the call will be like:

delete_all( &test );

However, I advise /against/ this. Strongly.

Why? You might think that by setting the variable referring to
the list to null, you can avoid falling into the trap of
walking a list you've deleted, because the reference has been
nulled. But if there are multiple pointers into the list,
/those other pointers will be non-nulled/, and using them
will do bad things; and if this was the last pointer to the
list, you will nullify it pointlessly if the pointer itself
is about to go away.

You may have reason to nullify the list reference when you delete
the list, but it's not an /automatic/ thing to do. So don't do
it without thought; do it only if it proves both needful and
harmless.

(If it's needful and harmful, then you have a bigger design
problem.)

Lets say I have a list "test", the following gives an error:
delete_all( test );
delete_all( test );

Well, yes, lucky you: you freed something twice. Bad Things
Happen. Don't do that.

 

[hal22]

Thanks, it was the pointer to pointer thing. But I guess I better
leave it out. I should let it fail if it tries to free the same thing
twice; instead of just handling such an error prematurely.

 

[Chris Dollin]

Thanks, it was the pointer to pointer thing. But I guess I better
leave it out. I should let it fail if it tries to free the same thing
twice; instead of just handling such an error prematurely.

You have no guarantee that it will fail if you free the same thing
twice, only the guarantee that you have no guarantees once you've
done it.

Just ensure you don't free the same thing twice. It's non-trivial -- that's
why garbage-collection exists in several other languages, because many
programmers have better things to do than keep track of storage if the
implementation can do a better job -- but achievable, especially if you
/start/ knowing that you'll need a storage-management policy.

(Because C lacks garbage-collection, it lacks features whose implementation
needs garbage-collection and has features that make garbage-collection
... interesting. Which makes a whole raft of techniques unavailable to
C programmers. Still, there's got to be /something/ at the foundations.)