Deleting node from linked list in C

Question

Okay this is my code for deleting a node from a linked list.

vec_store holds seq and size. Variable seq holds the vectors and a pointer.

For some reason, the else if(i<s->size-1) doesn't work which is the last condition.

Any1 can solve the problem? By the way this is C code.

void delete_vec(vec_store s, int i)
{
    if (i<0 || s->size-1<i)
    {
        printf("Cannot delete vector because index %d is out of bounds\n",i);
    }
    else if (i==0)
    {
        node temp;
        temp = s->seq;
        s->seq = s->seq->next;
        s->size--;
        free(temp);
    }
    else if(i==s->size-1)
    {
        node temp1, temp2;
        //temp1 = malloc(sizeof (node));
        temp2 = malloc(sizeof (node));
        temp1=s->seq;
        if(temp1->next==NULL) 
        {
            free(temp1);
            s->seq=NULL;
            s->size--;
            printf("s->size-1\n");
        }
        else
        {
            while (temp1->next!=NULL)
            {
                temp2 = temp1;
                temp1 = temp1->next;    
            }
            free(temp1);
            temp2->next=NULL;
            s->size--;
            printf("s->size-1 2\n");
        }
    }
    else if(i<s->size-1)
    {
        node temp1,temp2;
        int j;
        temp1=s->seq;
        temp2 = malloc(sizeof (struct node_record));

        for(j=0;j<=i-1;j++)
        {
            temp2=temp1;
            temp1 = temp1->next;
        }

        free(temp1);
        temp2->next=NULL;
        s->size--;
    }
}

Answer 1

Now that your indenting has been fixed, I'll point out the problem. When you remove a node in the last condition, you're setting the next field for the previous node to NULL, rather than to the node following.

Answer 2

This code is unreachable:

if(temp1->next==NULL){
free(temp1);
s->seq=NULL;
s->size--;
printf("s->size-1\n");
}

...because temp1 is the first element in the list, and so it would only happen if the first element was also the last element - which implies s->size is 1, so this would have been caught by the earlier if (i==0) case.

This allocation to temp2 (which occurs in two places) is bogus - temp2's value gets overwritten anyway, leaking the memory you allocated:

temp2 = malloc(sizeof (node));

Finally, what is probably the problem you're asking about lies here (in the if(i<s->size-1) case):

free(temp1);
temp2->next=NULL;

This chops the entire end off the list. You want to keep the tail of the list around - like this:

temp2->next = temp1->next;
free(temp1);

By the way, temp2 and temp1 are pretty uninspiring variable names - what about previous and current or something? Also, the special case for if(i==s->size-1) is completely unnecessary - it should be handled fine by the code you have for the if(i<s->size-1) case.