double linked list

[peo_leo]

I have a simple implementation of double linked list
this code is crashing if i enter values not present in the linked
list(during insertion) or if i try to delete values not present in the
list
can u suggest how can i avoid it?

#include<stdio.h>
#include<stdlib.h>
#include<ctype.h>

void main()
{
int i,n,c,x,m,flag=0;

struct node{
struct node* next;
int data;
struct node* prev;
}*h,*t,*start,*last,*z,*p,*q;

printf("enter the number of nodes");
scanf_s("%d",&n);
if(n<=0)
{
printf("no of nodes cannot be %d\n",n);
exit(0);
}
start=(node*)malloc(sizeof(node));
start->data=NULL;
start->prev=NULL;
start->next=NULL;

last=(node*)malloc(sizeof(node));
last->data=NULL;
last->next=NULL;
last->prev=NULL;

z=(node*)malloc(sizeof(node));
printf("enter data");
scanf_s("%d",&z->data);
start->next=h=z;
z->prev=NULL;
i=1;

while(i<n)
{
q=(node*)malloc(sizeof(node));
printf("enter data");
scanf_s("%d",&q->data);
z->next=q;
q->prev=z;
z=q;
++i;
}

z->next=NULL;
last->prev=t=z;

for(;;)
{
printf("\n");
printf("1.Insertion\n2.Deletion\n3.Display \n4.exit\n");
printf("\tenter your choice");
scanf_s("%d",&c);
switch(c)
{

case 1:
printf("enter the element after which node has to be inserted:\t");
scanf_s("%d",&x);

p=h;
if(x==-999)
{
z=(node*)malloc(sizeof(node));
printf("enter the data");
scanf_s("%d",&z->data);

z->next=p;
p->prev=z;
z->prev=NULL;

start->next=h=z;
}
else
{
while((p->data!=x)&&(p!=NULL))
{
p=p->next;
if(p==NULL)
{
printf("node cannot be inserted\n");
//exit(0);
}
}

if (p->next==NULL)
{
z=(node*)malloc(sizeof(node));
printf("enter data");
scanf_s("%d",&z->data);
p->next=z;
z->next=NULL;
z->prev=p;
last->prev=t=z;
}
else
{
q=p->next;
z=(node*)malloc(sizeof(node));
printf("enter the data");
scanf_s("%d",&z->data);
z->next=q;
q->prev=z;
p->next=z;
z->prev=p;
}
}
break;

case 2:
printf("enter the node to be deleted");
scanf_s("%d",&m);

p=h;
if(p->data==m)
{
start->next=h=p->next;
free(p);
}

else
{
//p=h;
q=p->next;
while((q->data!=m)&&(q!=NULL))
{
p=q;
q=q->next;
}
if(q==NULL)
{
printf("node cannot be deleted\n");
//exit(0);
}

else
{
p->next=q->next;
free(q);
}
}
break;

case 3:
p=start->next;
printf("data are");
while(p!=NULL)
{
printf("%5d",p->data);
p=p->next;
}
break;

case 4:
exit(0);
default:
printf("\t\tWrong Choice");
}
}
}

[T.M. Sommers]

peo_leo wrote:

I have a simple implementation of double linked list
this code is crashing if i enter values not present in the linked
list(during insertion) or if i try to delete values not present in the
list
can u suggest how can i avoid it?

#include<stdio.h>
#include<stdlib.h>
#include<ctype.h>

void main()

int main (void)

start=(node*)malloc(sizeof(node));

You should not cast the return value of malloc, and it can be
helpful to use the sizeof the variable being assigned to as the
argument to malloc, in case its type ever changes:

start = malloc(sizeof(*start));

You should always check the return value of malloc for NULL.

while((p->data!=x)&&(p!=NULL))

It would be a real good idea to check p for NULL *before*
dereferencing it.

--
Thomas M. Sommers -- tm*@nj.net -- AB2SB

[Ben Bacarisse]

"peo_leo" <pe*****@gmail.comwrites:

I have a simple implementation of double linked list
this code is crashing if i enter values not present in the linked
list(during insertion) or if i try to delete values not present in the
list
can u suggest how can i avoid it?

#include<stdio.h>
#include<stdlib.h>
#include<ctype.h>

void main()
{
int i,n,c,x,m,flag=0;

struct node{
struct node* next;
int data;
struct node* prev;
}*h,*t,*start,*last,*z,*p,*q;

I am not going to solve the specific problem for you, but I do want to
help you get more help. By the time I got to this point in your
program I had lost heart and could not go on to see what detail was
actually causing your problem.

You need to break this program up into functions, each of which does
one simple to verify task, based on the parameters it is passed. If
this is a student exercise, you would loose out big time by not having
done this. If the program is just for your own study and you have
avoided function because you are unsure of them, you need to sort that
out before you get to dynamic data structures like linked lists. The
correct use of functions is the single most important element in
program design.

A big chuck of code like this puts people off. With well defined
subunits, the reader can check each one: "yes, that will append a new
node to list p", "yes, that will delete a node", and so on.

Finally, by avoiding functions you have made all your variables
global. OK, so they are not *technically* global, but they are all in
scope for the entire program and so the value for any of z, p, q, c, m,
x... (all unhelpful names, by the way) could affect any part of the
program.

--
Ben.

[Barry Schwarz]

On 8 Nov 2006 22:40:48 -0800, "peo_leo" <pe*****@gmail.comwrote:

>I have a simple implementation of double linked list
this code is crashing if i enter values not present in the linked
list(during insertion) or if i try to delete values not present in the
list
can u suggest how can i avoid it?

#include<stdio.h>
#include<stdlib.h>
#include<ctype.h>

void main()

int main(void)

>{
int i,n,c,x,m,flag=0;

struct node{
struct node* next;
int data;
struct node* prev;
}*h,*t,*start,*last,*z,*p,*q;

printf("enter the number of nodes");
scanf_s("%d",&n);

Is there some reason you cannot use standard functions? Anyone who
wants to compile your code has to fiddle with these.

> if(n<=0)
{
printf("no of nodes cannot be %d\n",n);
exit(0);
}
start=(node*)malloc(sizeof(node));

Don't cast the return from malloc. You never check to see if malloc
succeeds.

> start->data=NULL;
start->prev=NULL;
start->next=NULL;

last=(node*)malloc(sizeof(node));
last->data=NULL;
last->next=NULL;
last->prev=NULL;

z=(node*)malloc(sizeof(node));
printf("enter data");
scanf_s("%d",&z->data);
start->next=h=z;
z->prev=NULL;
i=1;

while(i<n)
{
q=(node*)malloc(sizeof(node));
printf("enter data");
scanf_s("%d",&q->data);
z->next=q;
q->prev=z;
z=q;
++i;
}

z->next=NULL;
last->prev=t=z;

BZZT! You no longer have a double linked list. last->prev points to
z but z->next obviously does not point to last.

>
for(;;)
{
printf("\n");
printf("1.Insertion\n2.Deletion\n3.Display \n4.exit\n");
printf("\tenter your choice");
scanf_s("%d",&c);
switch(c)
{

case 1:
printf("enter the element after which node has to be inserted:\t");
scanf_s("%d",&x);

p=h;
if(x==-999)
{
z=(node*)malloc(sizeof(node));
printf("enter the data");
scanf_s("%d",&z->data);

z->next=p;
p->prev=z;
z->prev=NULL;

start->next=h=z;
}
else
{
while((p->data!=x)&&(p!=NULL))
{
p=p->next;
if(p==NULL)
{
printf("node cannot be inserted\n");

If x is not in the list, you get here.

> //exit(0);
}

And then fall through to here. But at this point, p is NULL and any
attempt to evaluate p->data (in the above while statement) invokes
undefined behavior. A seg fault is good in this case.

> }

if (p->next==NULL)
{
z=(node*)malloc(sizeof(node));
printf("enter data");
scanf_s("%d",&z->data);
p->next=z;
z->next=NULL;
z->prev=p;
last->prev=t=z;

Again, the list is broken.

> }
else
{
q=p->next;
z=(node*)malloc(sizeof(node));
printf("enter the data");
scanf_s("%d",&z->data);
z->next=q;
q->prev=z;
p->next=z;
z->prev=p;
}
}
break;

case 2:
printf("enter the node to be deleted");
scanf_s("%d",&m);

p=h;
if(p->data==m)
{
start->next=h=p->next;
free(p);
}

else
{
//p=h;
q=p->next;
while((q->data!=m)&&(q!=NULL))

These tests are in the wrong order.

> {
p=q;

If x is not in the list, q will eventually equal z.

> q=q->next;

Now q is NULL. You loop back to the while statement and attempt to
evaluate q->data. A seg fault is one of the better results you can
hope for. If the tests were reversed, you would exit the while
cleanly.

> }
if(q==NULL)
{
printf("node cannot be deleted\n");
//exit(0);
}

else
{
p->next=q->next;

You broke the list again. p->next now points somewhere valid but you
forgot to set the ->prev in that somewhere to point to q. You need
something like
q->next->prev = p;

> free(q);
}
}
break;

case 3:
p=start->next;
printf("data are");
while(p!=NULL)
{
printf("%5d",p->data);
p=p->next;
}
break;

case 4:
exit(0);
default:
printf("\t\tWrong Choice");
}
}
}

Remove del for email

[peo_leo]

thx to all of u all for ur responses!!
gave me a good insight!!
On Nov 19, 11:12 pm, Barry Schwarz <[e-mail]:

On 8 Nov 2006 22:40:48 -0800, "peo_leo" <[e-mail]:

I have a simple implementation of double linked list
this code is crashing if i enter values not present in the linked
list(during insertion) or if i try to delete values not present in the
list
can u suggest how can i avoid it?

#include<stdio.h>
#include<stdlib.h>
#include<ctype.h>

void main()int main(void)

{
int i,n,c,x,m,flag=0;

struct node{
struct node* next;
int data;
struct node* prev;
}*h,*t,*start,*last,*z,*p,*q;

printf("enter the number of nodes");
scanf_s("%d",&n);Is there some reason you cannot use standard functions? Anyone who

wants to compile your code has to fiddle with these.

if(n<=0)
{
printf("no of nodes cannot be %d\n",n);
exit(0);
}

start=(node*)malloc(sizeof(node));Don't cast the return from malloc. You never check to see if malloc

succeeds.

start->data=NULL;
start->prev=NULL;
start->next=NULL;

last=(node*)malloc(sizeof(node));
last->data=NULL;
last->next=NULL;
last->prev=NULL;

z=(node*)malloc(sizeof(node));
printf("enter data");
scanf_s("%d",&z->data);
start->next=h=z;
z->prev=NULL;
i=1;

while(i<n)
{
q=(node*)malloc(sizeof(node));
printf("enter data");
scanf_s("%d",&q->data);
z->next=q;
q->prev=z;
z=q;
++i;
}

z->next=NULL;
last->prev=t=z;BZZT! You no longer have a double linked list. last->prev points to

z but z->next obviously does not point to last.

for(;;)
{
printf("\n");
printf("1.Insertion\n2.Deletion\n3.Display \n4.exit\n");
printf("\tenter your choice");
scanf_s("%d",&c);

switch(c)
{

case 1:
printf("enter the element after which node has to be inserted:\t");
scanf_s("%d",&x);

p=h;
if(x==-999)
{
z=(node*)malloc(sizeof(node));
printf("enter the data");
scanf_s("%d",&z->data);

z->next=p;
p->prev=z;
z->prev=NULL;

start->next=h=z;
}
else
{
while((p->data!=x)&&(p!=NULL))
{
p=p->next;
if(p==NULL)
{
printf("node cannot be inserted\n");If x is not in the list, you get here.

//exit(0);
}And then fall through to here. But at this point, p is NULL and any

attempt to evaluate p->data (in the above while statement) invokes
undefined behavior. A seg fault is good in this case.

}

if (p->next==NULL)
{
z=(node*)malloc(sizeof(node));
printf("enter data");
scanf_s("%d",&z->data);
p->next=z;
z->next=NULL;
z->prev=p;
last->prev=t=z;Again, the list is broken.

}
else
{
q=p->next;
z=(node*)malloc(sizeof(node));
printf("enter the data");
scanf_s("%d",&z->data);
z->next=q;
q->prev=z;
p->next=z;
z->prev=p;
}
}
break;

case 2:
printf("enter the node to be deleted");
scanf_s("%d",&m);

p=h;
if(p->data==m)
{
start->next=h=p->next;
free(p);
}

else
{
//p=h;
q=p->next;
while((q->data!=m)&&(q!=NULL))These tests are in the wrong order.

{
p=q;If x is not in the list, q will eventually equal z.

q=q->next;Now q is NULL. You loop back to the while statement and attempt to

evaluate q->data. A seg fault is one of the better results you can
hope for. If the tests were reversed, you would exit the while
cleanly.

}
if(q==NULL)
{
printf("node cannot be deleted\n");
//exit(0);
}

else
{
p->next=q->next;You broke the list again. p->next now points somewhere valid but you

forgot to set the ->prev in that somewhere to point to q. You need
something like
q->next->prev = p;

free(q);
}
}
break;

case 3:
p=start->next;
printf("data are");
while(p!=NULL)
{
printf("%5d",p->data);
p=p->next;
}
break;

case 4:
exit(0);
default:
printf("\t\tWrong Choice");
}
}