Thread: Deleting a node from a list

Hi my delete method for my linked list works for all nodes apart from the last node before the pointer the last item in the list e.g
1->3->4->5->Null

deleting my 5 is a problem

Code:

struct listset{
    int length;
    struct node *head;
};
struct listset * litset_new(){
    struct listset *p = malloc(sizeof(*p));
    p->head=NULL;
    return p;

}
void listset_add(struct listset * t, int item){
    struct listset *a=t;
    struct node *temp= malloc(sizeof(*temp));
    temp->key=item;
    temp->next=a->head;
    a->head=temp;
    a->length++;


}
void listset_remove(struct listset * p, int item){
   struct listset *a=p;
  // struct node *next;
   struct node *previous;
   struct node *current ;
   struct node * temp;
    current=a->head;//first node in list
    previous=current;
    
    //if only one item on d list
    if(a->length==1){
        a->head=current->next;
        free(current);
    }

    //beginning of the list
  else  if(current->key==item){
        a->head=current->next;
        free(current);
    }
   else if(listset_lookup(a,item)){

        while(current!=NULL)
        {
            if(current->key==item)
            {
               // printf("%d\n",current->next->key);
               // printf("%d\n",previous->key);
                previous->next=current->next; //relinking nodes
               // a->head=previous;
                free(current);
              //  printf("%d\n",current->next->key);


            }

           //  printf("i am in");
             previous=current;
             current=current->next;
        }


    }
}

It looks basically right -- I'm surprised you're managing to call listset_remove() without getting a segfault though.

Consider this part:

Code:

   else if(listset_lookup(a,item)){
 
        while(current!=NULL)
        {
            if(current->key==item)
            {
               // printf("%d\n",current->next->key);
               // printf("%d\n",previous->key);
                previous->next=current->next; //relinking nodes
               // a->head=previous;
                free(current);
              //  printf("%d\n",current->next->key);
 
 
            }
 
           //  printf("i am in");
             previous=current;
             current=current->next;
        }

Suppose current->key == item... then you correctly join previous->next to current->next, then correctly free current.
What is not correct is that you try to dereference current with current=current->next, when current might have just been free'd.

If you want to delete the first item that matches, I suggest you return after the free().
If you want to delete all the items that match, I suggest setting current to the next element after the free.


Also, you should initialise listset length to 0, otherwise it'll contain garbage!