+0100 in comp.lang.c.
newbie question: removal of node in linked list's a cool scene! Dig
it!
Quote:
>Please, why does this code work: <snippet from switch case>
Why *does* it work? I'm surprised it does, if it does. I can't see
how it is supposed to work. You have not shown enough of the program.
We need to know what the variables are, what values they hold, etc..
But I do know that this code has at least one error that could crash
the system. Who knows what else may be wrong with it?
I'm assuming this is a single linked list, not a double linked list.
BTW, the following code is very horribly indented! YUCK!
Quote:
>case 3: if(tail!=NULL) {
> tmp=head;
> if(tail==head) {
> free(tail);
> free(head);
Uh oh! This is likely to crash. If tail == head, you can't free both
of them, because you're freeing the same thing twice. The two pointers
point at the same thing. You're officially in Undefined Behaviour
Land.
Quote:
> tail=NULL;
> head=NULL;
> tail=head;}
That is utterly pointless. You set tail to NULL, then, two lines
later, set it to the value of head, which you also set to NULL.
Quote:
> else {
> prev=NULL;
> while(tmp->next!=NULL) {
> prev=tmp;
> tmp=tmp->next;
> }
What is the point of that? I don't understand the logic here. What
is the purpose of the above lines? It appears to assign to prev
(whatever that is) the address of the last node in the list, or NULL
if tmp is the last node. Why is this done? It doesn't look quite
right, but without knowing how it is supposed to work, I can't really
tell.
Quote:
> free(tail);
> tail=prev;
> tail->next=NULL;
> ;}
> }
> else {
> printf("No nodes to remove from list\n");}
> break;
>And why this does not work, instead of traversing the list first from
>head, just do this:
>tmp=tail->next; /* tail->next should be NULL anyway */
>free(tail);
>tail=tmp;
If tail->next == NULL, then tail = tmp sets tail to NULL. This
doesn't sound right. tail should only == NULL when there are no nodes
in the list.
What you need to do is set the previous node's next pointer to the
value of the current (the one you want to delete) node's next pointer,
then, if the current node is the tail node, set tail to the previous
node, and lastly free the current node. But that assumes there *is* a
previous node. You must first check whether the current node is the
head node, and take appropriate action. Something like this (current
points at the node you want to delete):
/* if current node is head there is no previous node to
deal with and we need to point head at the next node */
if(head == current)
{
/* next node becomes the new head node - if current is
the only node in the list then current->next == NULL
so head will be set to NULL to indicate an empty list */
head = current->next;
/* if current node is tail node it must be the only node in
the list - set tail to NULL to indicate an empty list */
if(tail == current)
tail = NULL;
Quote:
}
else
{
YOUR_LINKED_LIST_NODE_TYPE *prev;
/* find previous node */
prev = head;
while(prev->next != current)
prev = prev->next;
/* effectively remove current node from the chain - if
current node is tail then current->next == NULL so
prev->next will be set to NULL to indicate that it
is the last node in the linked list (tail node) */
prev->next = current->next;
/* if current node is tail set tail to previous node */
if(tail == current)
tail = prev;
Quote:
}
/* now that we have effectively removed the
current node from the chain we can free it */
free(current);
Of course, all this is a little simpler and much more efficient if
it is a double linked list, because finding the previous node is
trivial.
Quote:
>I know Richard Bos explained this to me once, (and numerous others) but
>i have seem to forgotten how or why this does not work. I mean, yes i am
>freeing tail, but i stored the address of where tail->next points to in
>tmp, so that's saved right ? Or not?
tail->next shouldn't point at anithing. It should be NULL. You said
it yourself in a comment.
--
Dig the even newer still, yet more improved, sig!
http://alphalink.com.au/~phaywood/
"Ain't I'm a dog?" - Ronny Self, Ain't I'm a Dog, written by G. Sherry & W. Walker.
I know it's not "technically correct" English; but since when was rock & roll "technically correct"?