newbie question: removal of node in linked list 
Author Message
 newbie question: removal of node in linked list

Please, why does this code work: <snippet from switch case>

case 3: if(tail!=NULL) {
                         tmp=head;
                         if(tail==head) {
                                 free(tail);
                                 free(head);
                                 tail=NULL;
                                 head=NULL;
                                 tail=head;}
                         else {
                         prev=NULL;
                         while(tmp->next!=NULL) {
                                 prev=tmp;
                                 tmp=tmp->next;
                         }
                         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;

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?

Much obliged,
atv



Wed, 31 Aug 2005 20:57:04 GMT  
 newbie question: removal of node in linked list

Quote:

> Please, why does this code work: <snippet from switch case>

> case 3: if(tail!=NULL) {
>                         tmp=head;
>                         if(tail==head) {
>                                 free(tail);
>                                 free(head);

If this worked you should consider it fortunate.
Since pointer tail  is equal to pointer head, they
are both pointing to the same allocated memory.
Deallocating that memory twice, as was done here
by calling free(tail) and free(head) results in
undefined behavior as explicited stated in the C
Standard. To correct this a call to either free(head)
or free(tail) would do, but not both.

Quote:
>                                 tail=NULL;
>                                 head=NULL;
>                                 tail=head;}
>                         else {
>                         prev=NULL;
>                         while(tmp->next!=NULL) {
>                                 prev=tmp;
>                                 tmp=tmp->next;
>                         }
>                         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;

> 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?

This appears to be a singly-linked list. tail should point to the last
node in the list. If you assign to tail the value of tmp, you
make tail == NULL. What want is for tail to be assigned to point to
the node previous to tail.

----------
Al Bowers
Tampa, FL. USA

http://www.geocities.com/abowers822
comp.lang.c



Wed, 31 Aug 2005 23:01:37 GMT  
 newbie question: removal of node in linked list

+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"?



Mon, 05 Sep 2005 08:10:37 GMT  
 
 [ 3 post ] 

 Relevant Pages 

1. Question about swapping nodes in a doubly linked list

2. Problems with binary tree node removal.

3. Node removal from a tree

4. nodes linked listed and such

5. 2 nodes end up in a cycle in a single linked list

6. How to find a node in link list

7. Realloc()ing data of a node from a linked list

8. why malloc for a new node in linked list

9. Destroying All Nodes in a Linked List

10. Destroying All Nodes is a Linked List

11. Removing node from linked list

12. Removing a NODE for good (double linked lists)

 

 
Powered by phpBB® Forum Software