Hi all,

I am new in C programming, currently learning about linked list. I know that there is a lot of examples about linked list, but I decided to write my own code, to make me more understand. I managed to build a linked list, insert a new node after one cell, but I have a segmentation fault after deleting one node.

Can anyone help me with this, why I got segmentation fault ?

The code snippet (just for the delete block)

``````void cell_delete(int a)
{
cell *p, *q;
printf("you are before while block\n");

while((p!=NULL))
{
q=p;
p=p->next;
if(p->value = a)
{
printf("correct value\n");
p=q->next;
q->next = p->next;
print_list();
free(p);
free(q);
}
else
{
printf("value of p is wrong");
}
}

printf("you are OUT OF while block\n");
}``````

>if(p->value = a)
Two things:

• Comparison is done with the == operator. The = operator is for assignment. You're doing the equivalent of this:
``````p->value = a;
if (p->value != 0)``````
• You've already advanced p at this point, so how do you know it's not NULL?

>if(p->value = a)
[*]You've already advanced p at this point, so how do you know it's not NULL?
[/list]

Thank you for your answer and thank you for the == operator.
I am kinda confused with what you ask, AFAIK, the p will traverse from one node till the last node (which is NULL), that is why I write do the code below while (p!=NULL) .. Am I understanding you well?

``````while (p != NULL)
{
/* p is definitely not NULL, so this is safe */
p = p->next;

/* If p->next was NULL, p is now NULL, so this is *not* safe */
if (p->value == a)
{
...
}
else
{
...
}
}``````

See the problem?

The problem is that you are changing the meaning of p before you test p->value, which means that you don't know yet if p (and hence p->value) is valid. You want to test q->next, not p->next:

``````while((p!=NULL))
{
q=p;
p = p->next;
if(q->value = a)
{
printf("correct value\n");
p=q->next;
q->next = p->next;
print_list();
free(p);
free(q);
}
else
{
printf("value of p is wrong");
}
}``````

PS: You're indent style is unusual; is that by intention?

The problem is that you are changing the meaning of p before you test p->value, which means that you don't know yet if p (and hence p->value) is valid. You want to test q->next, not p->next:

PS: You're indent style is unusual; is that by intention?

Yes thank you everyone about pointing out
q=p;
p = p->next; //if p = NULL, it cause segmentation fault. I did not aware of it ..

but I am sorry again, I don't understand what do you mean with "you want to test q->next, not p->next". I still do not get the idea to fix the segmentation fault, as I tried to change the algorithm, but the delete part will not work.. :(
Yes, about indent style, I thought it will be easier to read.