By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
446,413 Members | 1,023 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 446,413 IT Pros & Developers. It's quick & easy.

what's wrong with this code?

P: n/a
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?

Nov 13 '05 #1
Share this Question
Share on Google+
5 Replies


P: n/a
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
"cc0064263" <ca*************@yahoo.com> wrote in message
news:DM********************@comcast.com...
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?

Nov 13 '05 #2

P: n/a
In article <DM********************@comcast.com>, cc0064263 wrote:
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
ja***********@lists.sourceforge.net http://jail-ust.sourceforge.net
(send mail to ja*********************@lists.sourceforge.net)
Nov 13 '05 #3

P: n/a
TDR
( I made a typo on the first reply)
"TDR" <td*******@hotmail.com> wrote in message
news:3f********@news.tm.net.my...
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
"cc0064263" <ca*************@yahoo.com> wrote in message
news:DM********************@comcast.com...
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?


Nov 13 '05 #4

P: n/a
"cc0064263" <ca*************@yahoo.com> wrote:
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);
}

--
Simon.
Nov 13 '05 #5

P: n/a
cc0064263 wrote:
thanks all for pointing it out.
the while loop does make things easier to see.
maybe i just needed my coffee.
for (p=head; p!= NULL; p->next)
free(p);

what's wrong with it?

Nov 13 '05 #6

This discussion thread is closed

Replies have been disabled for this discussion.