9 replies to this topic

[onus]

I am writing a program which can create a link list

#include <stdio.h>


struct node {

struct  node *next,*prev;

  int data;

} n,*start;

void add_element (int e);

static int j=0;

int main ()

{

	int i,element,l,choice;

        choice=1;

       struct node *temp;

	start = NULL;

     while (choice == 1)

    {

        printf("What do you want 0 exit 1 to add\n");

	 scanf("%d",&choice);

	 if(choice==0)

	  break;

	 printf("Enter data to be entered\n");

	scanf("%d",&element);

	 add_element(element);

    }


}


void add_element(int e)

{

 printf("\n printing from function %d\n",e);	

  struct node *temp;

  if(j==0)

  {

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

       start->next=NULL;

      start->data=e;

    printf("the data entered is %d \n",start->data);

   }

  j++;

}

When ever I am compiling the above code
the error I am getting
In function
add_element is

linklist.c: In function ‘add_element’:

linklist.c:34: warning: incompatible implicit declaration of built-in function ‘malloc’

am I doing any mistake by declaring malloc as below?

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

[dbug]

You should include stdlib.h.

[onus]

Oh thanks :) it worked.

[onus]

I now have added some more lines of code to above program

#include <stdio.h>
#include <stdlib.h>
struct node {
	struct node *next, *prev;
	int data;
} *start, *prv;
void add_element(int e);
void read_list ();
static int j = 0;
int main()
{
	int i, element, l, choice;
	choice = 1;
	struct node *temp;
	start = NULL;
	prv = start;
	while (choice == 1) {
		printf("What do you want 0 exit 1 to add\n");
		scanf("%d", &choice);
		if (choice == 0)
			break;
		printf("Enter data to be entered\n");
		scanf("%d", &element);
		add_element(element);
               read_list();
	}

}

void add_element(int e)
{
//      printf("\n printing from function %d\n", e);
	struct node *temp;
	if (j == 0) {
		start = (struct node *)malloc(sizeof(struct node));
		start->next = NULL;
		start->data = e;
		//printf("the data entered is %d \n", start->data);
		j++;
	}


		temp = (struct node *)malloc(sizeof(struct node));
		temp->data = e;
		prv->next = temp;
	          temp->next=NULL;
		prv=temp;//since prv is  global pointer if I do not do this step then next time when new memory is allocated prv is still pointing to start or previous pointer what ever the value was so new temp declaration will not have much effect
		j++;

	
}

void read_list()
{
	struct node *temp;
	temp = start;
	while (temp) {
		printf(" %d --> ", temp->data);
		temp = temp->next;
	}
}

but this program gave me a segmentation fault at line

		prv->next = temp;

why is that so any clue?

[dbug]

You initialize prv with a NULL pointer when there aren't any elements. If you access prv->next, it will point to an invalid memory address, generating a segmentation fault.

The correct way to do that is:

temp->next = prv;

prv = temp;

This inserts each new element at the begining of the list.

Edited by dbug, 23 September 2010 - 02:04 AM.
bad code

[onus]

You mean to say that I am doing some thing like

NULL=temp;

But start and prv I have initialized both to point to same memory address.So how come this is happening.
In the if loop I have made a check for j==0 then start will be given some memory
and in the main I have done

prv=start;

[dbug]

prv = start does not make that prv points to start. It assigns the value of start (that initially is NULL) to prv. If you later modify start, this won't change anything in prv.

[onus]

Ohhh thanks for clearing that but now my concern is I declared prv outside main.
So to be able to make sure that prv is same as what start is (start is a pointer to starting of list when created )
prv should also contain the address of first node which in start when I am in add_list function
what should I do.
I declared prv to be a global pointer.That does not help?

[dbug]

I think you have made it very complex. It's more simple to create a linked list of elements:

#include <stdio.h>

#include <stdlib.h>


struct node {

    struct node *next;

    int data;

} *start = NULL;


void add_element(int e)

{

    struct node *temp;


    temp = (struct node *)malloc(sizeof(struct node));

    temp->data = e;

    temp->next = start;

    start = temp;

}


void read_list()

{

    struct node *temp;


    for (temp = start; temp; temp = temp->next)

    {

        printf(" %d --> ", temp->data);

    }

    printf("\n");

}


int main(void)

{

    int element, choice;


    do

    {

        printf("What do you want 0 exit 1 to add\n");

        scanf("%d", &choice);

        if (choice == 0)

        {

            break;

        }

        printf("Enter data to be entered\n");

        scanf("%d", &element);

        add_element(element);

        read_list();

    } while (1);


    return 0;

}

I have used a single linked list, adding new elements at the begining (this reverses the order of the elements but it's very simple). If you need to mantain the order or use double linked lists, it can be easily adapted.

[onus]

Thanks I got your point I was able to make my program work correctly as I wanted.
I got your point prv = start. I pointed both to null and in my add_list in wrong program
so when I did start =(struct node *) malloc (sizeof(struct node))
would assign start some memory location
where as prv was still pointing to NULL so I was getting error of segmentation fault.

I am posting the code here

#include <stdio.h>

#include <stdlib.h>

struct node {

	struct node *next, *prev;

	int data;

} *start, *prv;

void add_element(int e);

void read_list ();

static int j = 0;

int main()

{

	int i, element, l, choice;

	choice = 1;

	struct node *temp;

	start = NULL;

	while (choice == 1) {

		printf("What do you want 0 exit 1 to add\n");

		scanf("%d", &choice);

		if (choice == 0)

			break;

		printf("Enter data to be entered\n");

		scanf("%d", &element);

		add_element(element);

	}


               read_list();

}


void add_element(int e)

{

//      printf("\n printing from function %d\n", e);

	struct node *temp;

	if (j == 0) {

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

		

		start->data = e;

		//printf("the data entered is %d \n", start->data);

		prv=start;

		

	}


          if(j!=0){

		temp = (struct node *)malloc(sizeof(struct node));

		temp->data = e;

		prv->next = temp;

	          temp->next=NULL;

		prv=temp;//since prv is  global pointer if I do not do this step then next time when new memory is allocated prv is still pointing to start or previous pointer what ever the value was so new temp declaration will not have much effect

         }

	

	j++;


	

}


void read_list()

{

	struct node *temp;

	temp = start;

	while (temp) {

		printf(" %d --> ", temp->data);

		temp = temp->next;

	}

}

I have checked it.It is working.