Deleting node in linked list

Question

Most of code works but when I try to delete last element of my list and I print it I see in its place some garbage data, what I'm doing wrong here? Could somebody point my mistake?

void DeleteClient2(struct client *temp,struct client **head)
{   struct client *prev=*head;
    struct client *current = *head;
    struct item *currentitem = (*head)->item_data,*save;
    if(temp== *head)
    {
        while(currentitem != NULL)
        {
            save = currentitem;
            currentitem = currentitem ->next;
            free(save);
        }
        free(temp);
        temp->item_data = NULL;
        (*head) = (*head)->next;
    }
    else
    if(temp->next == NULL)
    {
        while(currentitem != NULL)
        {
            save = currentitem;
            currentitem = currentitem ->next;
            free(save);
        }
        temp->item_data = NULL;
        free(temp);
    }
    else
    if(temp != *head && temp->next != NULL)
    {
        while(prev->next != temp)
        {
            prev=prev->next;
        }
        prev->next = temp->next;
        while(currentitem != NULL)
        {
            save = currentitem;
            currentitem = currentitem ->next;
            free(save);
        }
        temp->item_data = NULL;
        free(temp);
        temp=temp->next;
    }
}

first indent your code correctly.

Grijesh Chauhan
– Grijesh Chauhan

2014-01-23 06:14:01 +00:00
Commented Jan 23, 2014 at 6:14 — Grijesh Chauhan
– Grijesh Chauhan, Commented Jan 23, 2014 at 6:14

Grijesh Chauhan · Accepted Answer · 2014-01-23 06:11:42Z

2

Code:

    free(temp);
    temp=temp->next;  // temp->next is invalid

Is undefined behaviour, Once you free a node you can't access it, doing so is illegal.

answered Jan 23, 2014 at 6:11

Grijesh Chauhan

58.6k20 gold badges145 silver badges214 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

user3209183 Over a year ago

Than how should I link previous node and next node ?

Grijesh Chauhan Over a year ago

@user3209183 keep Save previous address

Ashish · Accepted Answer · 2014-01-23 06:28:09Z

0

void DeleteClient2(struct client *temp,struct client **head)
{   struct client *prev=*head;
    struct client *current = *head;
    struct item *currentitem = (*head)->item_data,*save;
    if(temp== *head) \\ if head is the node to be deleted
    {
        while(currentitem != NULL)
        {
            save = currentitem;
           *head = currentitem->next;
          \\  currentitem = currentitem ->next;
            free(save);
        }

    }
    else
    if(temp->next == NULL) \\ if the node to be deleted is last node then
    {
    while(currentitem->next != NULL)
        {
           *prev = currentitem;
            currentitem = currentitem ->next;

        }
        prev->next = NULL;
        free(currentitem);
}
else
if(temp != *head && temp->next != NULL)\\ node to be deleted is between head and last node
{while(prev->next != temp)
{   currentitem = *prev;
    prev=prev->next;
}
currentitem->next = temp->next;
free(temp);
\\while(currentitem != NULL)
    \\    {
    \\        save = currentitem;
    \\        currentitem = currentitem ->next;
    \\\        free(save);
    \\    }
      \\  temp->item_data = NULL;
     \\   free(temp);
     \\   temp=temp->next;
}
}

answered Jan 23, 2014 at 6:28

Ashish

1,9412 gold badges14 silver badges17 bronze badges

1 Comment

Ashish Over a year ago

there may some mistake in my code but you can try this idea(the changes i have made

Dabo · Accepted Answer · 2014-01-23 06:16:33Z

0

Here you check that *head == temp and if yes, you free temp i.e *head cause they equal, you just verified it, and than you say (*head) = (*head)->next; which set *head to garbage most probably

if(temp== *head)
{
    while(currentitem != NULL)
    {
        save = currentitem;
        currentitem = currentitem ->next;
        free(save);
    }
    free(temp);
    temp->item_data = NULL;
    (*head) = (*head)->next;
}

Same mess is here

    temp->item_data = NULL;
    free(temp);
    temp=temp->next;

answered Jan 23, 2014 at 6:16

Dabo

2,3832 gold badges21 silver badges30 bronze badges

Collectives™ on Stack Overflow

Deleting node in linked list

3 Answers 3

2 Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related