what's wrong with this code?

[cc0064263]

hello,

i am very new to C and i am reading K&R and i came
accross this code in the book. K&R says that it is
incorrect to free something after it has been freed
and gives this as an example.

for (p=head; p!= NULL; p->next)
free(p);

what's wrong with it?

 

[TDR]

Well looks of things the "p->next" part doesn't really advance the pointer p
as a result on the first run of the loop, the pointer p is freed but on the
second (since it was not advance and was already freed) it gets freed
second.
Now I don't really remember what would happen then (usually it just works or
just plain crashes as I rarely met such cases).

hope this helps

TDR

 

[TDR]

( I made a typo on the first reply)
Well looks of things the "p->next" part doesn't really advance the pointer
p
as a result on the first run of the loop, the pointer p is freed but on the
second run of the loop (since it was not advance and was already freed) it
gets freed a
second time.
Now I don't really remember what would happen then (usually it just works
or
just plain crashes as I rarely met such cases).

hope this helps

TDR

 

[Ronald Landheer-Cieslak]

hello,

i am very new to C and i am reading K&R and i came
accross this code in the book. K&R says that it is
incorrect to free something after it has been freed
and gives this as an example.

for (p=head; p!= NULL; p->next)
free(p);

what's wrong with it?

There are at least two problems with this code: p->next doesn't do anything
and p is accessed after it's been freed.

Even if the first problem weren't there (i.e. the code said `p = p->next')
the second would still (possibly) kill your pprogram.

The for loop in while form looks like this:

p = head;
while (p != NULL)
{
free(p);
p = p->next;
}

perhaps it is a bit clearer now why p is accessed after the free?

To do it right, you can do it like this:

p_type p = head;
p_type last = NULL;
while (p != NULL)
{
last = p;
p = p->next;
free(last);
}

(in which p_type is some pointer type). Here, you save the old value of p to
last, get the value you want for p, and *then* free the saved, old value.

HTH

rlc

--
Jail: Just Another Interpreted Language
Just: Jail Uses Silly Terms

Join the discussion on the definition of this language at
(e-mail address removed) http://jail-ust.sourceforge.net
(send mail to (e-mail address removed))

 

[Simon Biber]

hello,

i am very new to C and i am reading K&R and i came
accross this code in the book. K&R says that it is
incorrect to free something after it has been freed
and gives this as an example.

for (p=head; p!= NULL; p->next)

^^^^^^^ should be p = p->next

free(p);

what's wrong with it?

Look at the equivalent with a while loop:

p = head;
while(p != NULL)
{
free(p);
p = p->next;
}

Now you see the problem is that after p has been freed (now the
pointer is invalid and its contents lost), you still try to use
the struct's 'next' member.

One possible fix is to change it to:
for(p = head; p != NULL; p = tmp)
{
tmp = p->next;
free(p);
}

 

[cc0064263]

cc0064263 wrote:


thanks all for pointing it out.
the while loop does make things easier to see.
maybe i just needed my coffee.