Thread: Linked list program dumping core when executed.

Hi,

I wrote the below core for creating a linked list and retrieving entries from the list. But when I execute the code, although it compiles successfully, eventually it dumps core.

Code:

typedef struct test{
	char name[10];
	char city[10];
	int marks;
	struct test *node;
}RECORD;

RECORD *head;
void reading_values (RECORD *);
 
#include <stdio.h>
void main()
{
int t,i,v,j,g;
RECORD *p;


p= (RECORD *)(malloc(sizeof(RECORD)));
printf("\n how many entries do you want to make?");
scanf("%d",&v);
for (j=0;j<v;j++)
{
	if (j==0)
	{
		p->node=head;
		printf("\n enter #### %s ####name ",v);
		scanf("%s",&p->name);

		printf("\n enter #### %s ####city ",v);
		scanf("%s",&p->city);

		printf("\n enter #### %d ####city ",v);
		scanf("%d",&p->marks);
	}
	else
	{
		p++;
		printf("\n enter #### %s ####name ",v);
		scanf("%s",&p->name);

		printf("\n enter #### %s ####city ",v);
		scanf("%s",&p->city);

		printf("\n enter #### %d ####marks ",v);
		scanf("%d",&p->marks);

	}
		p->node = NULL;

	
	printf("To read the list : please press 1");
	scanf("%d",&g);
	if(g==1)
		reading_values(head);


}

}

/* Function to read values from the list*/

 void reading_values(RECORD *head)
 {
	 RECORD *t;
	 t=head;

	while(t!=NULL)
	{
		printf("\n enter #### %s ####name ",t->name);
		printf("\n enter #### %s ####city ",t->city);
		printf("\n enter #### %d ####marks ",t->marks);
		printf("\n pointer location ###### %u ######",t);

		t++;
	}
 }

And the output of the code:

$ ./link_list.exe

how many entries do you want to make?2
2 [main] link_list 6156 _cygtls::handle_exceptions: Error while dumping state (probably corrupted stack)
Segmentation fault (core dumped)

p++ does not magically give you another object. You will need to malloc space for that object.

You also don't do any linking (i.e. the next pointer in each struct should be set to the address of the next struct in line).

Your code is very broken. I'm not sure how "fixable" it is until you have a better understanding of pointers, memory allocation and linked lists.

• It's convention to put your #includes at the top of the file, above your typedefs and prototypes.
• Your variable and function names are horrible. Only use single-letter variables for loop indexes (j). Things like g and v are confusing. Try num_entries and do_print. Your reading_values function should probably be called print_list. "next" is a better name than "node" for the pointer in your struct.
• Read this: Cprogramming.com FAQ > main() / void main() / int main() / int main(void) / int main(int argc, char *argv[]). It's int main(void), and you need to return an int at the end, usually 0.
• There is no reason to make head a global variable. Move it inside main.
• head is a pointer, but you never allocate memory for it or make it point to anything useful. Instead, you keep making p->node point to it.
• You only allocate p once, and keep changing the data it contains.
• You never free your allocated memory when you're done.
• Your "if (j==0)/else" blocks only differ in 1 line of code. Move all common code outside that.
• Don't put an & in front of a char array if you're reading into it with scanf. Just do "scanf("%s", p->name);".
• The t++ line in your print function is broken. It should be t = t->node.

You need to do some serious reading on how pointers, memory allocation and linked lists work. We have some tutorials here: Cprogramming.com - Programming Tutorials: C++ Made Easy and C Made Easy. Google will turn up dozens more. I also suggest you read your textbooks and work through all the example programs until you really understand this stuff.

Hi Folks,

Thanks a lot folks. Actually I have been mostly a test team guy and have worked on C very little professionally(although I have always wanted to). Now the now company that I have moved to requires me to do some coding and understand the code. After looking at your suggestions, I made some changes and looks like the code works fine now. I would really appreciate if you could review this and let me know if this is ok or should I make some changes. Looking forward for your responses.

Code:

Code:

typedef struct test{
	char name[10];
	char city[10];
	int marks;
	struct test *node;
}RECORD;

RECORD *temp1;
void reading_values (RECORD *);
 
#include <stdio.h>
//#include <stdlib.h>
void main()
{
int t,i,v,j,g;
RECORD *p, *temp2;


p=(RECORD *)(malloc(sizeof(RECORD)));
printf("value of p is %u", p);
printf("\n how many entries do you want to make?");
scanf("%d",&v);
for (j=0;j<v;j++)
{
	if (j==0)
	{
		
		temp1=p;
		printf("\n enter #### %d ####name ",j+1);
		scanf("%s",&temp1->name);

		printf("\n enter #### %d ####city ",j+1);
		scanf("%s",&temp1->city);

		printf("\n enter #### %d ####marks ",j+1);
		scanf("%d",&temp1->marks);
		
		//p++;
		printf("\nvalue of original pointer %u",p);

		p= (RECORD *)(malloc(sizeof(RECORD)));
		temp1->node=p;

		printf("\nvalue of pointer %u",temp1->node);
		printf("\nvalue of next pointer %u",p);
		printf("\n##########################");
		
	}

	else
	{
		temp2=p;
		printf("\n enter #### %d ####name ",j+1);
		scanf("%s",&temp2->name);

		printf("\n enter #### %d ####city ",j+1);
		scanf("%s",&temp2->city);

		printf("\n enter #### %d ####marks ",j+1);
		scanf("%d",&temp2->marks);

		printf("\nvalue of original pointer %u",p);
		p=(RECORD *)(malloc(sizeof(RECORD)));
		temp2->node=p;
		printf("\nvalue of pointer %u",temp2->node);
		printf("\nvalue of next pointer %u",p);
		printf("\n##########################");

	}


}
		temp2->node = NULL;
		printf("\nvalue of pointer %u",temp2->node);
		free(p);
printf("To read the list : please press 1");
	scanf("%d",&g);
	if(g==1)
		reading_values(temp1);

}

/* Function to read values from the list*/

 void reading_values(RECORD *temp1)
 {
	 RECORD *t;
	 int count=1;
	 t=temp1;

	while(t!=NULL)
	{
		printf("\n @@@@@@@@@@@@@@@@@@@ HERE IS YOUR DATA @@@@@@@@@@@@@@@@@@@@@@@@@@@@");
		printf("\n %d name  :: %s #### ",count,t->name);
		printf("\n %d city  :: %s #### ",count,t->city);
		printf("\n %d marks :: %d ####marks ",count,t->marks);
		printf("\n pointer location ###### %u ######",t);
		printf("\n next pointer location ###### %u ######",t->node);
		printf("\n @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
		t=t->node;
		
	}
 
 }

Hi,

Apologies !! . I have made changes to the code again and added function to free memory.

Code:

#include <stdio.h>

typedef struct test{
	char name[10];
	char city[10];
	int marks;
	struct test *node;
}RECORD;

void reading_values (RECORD *);
void free_allocated_memory (RECORD *);
 
int main()
{
int i,v,j,g;
RECORD *allocation_for_struct, *next,*head;



allocation_for_struct=(RECORD *)(malloc(sizeof(RECORD)));
printf("value of allocation_for_struct is %u", allocation_for_struct);
printf("\n how many entries do you want to make?");
scanf("%d",&v);
for (j=0;j<v;j++)
{
	if (j==0)
	{
		
		head=allocation_for_struct;
		printf("\n enter #### %d ####name ",j+1);
		scanf("%s",head->name);

		printf("\n enter #### %d ####city ",j+1);
		scanf("%s",head->city);

		printf("\n enter #### %d ####marks ",j+1);
		scanf("%d",&head->marks);
		
		printf("\nvalue of original pointer %u",allocation_for_struct);

		allocation_for_struct= (RECORD *)(malloc(sizeof(RECORD)));
		head->node=allocation_for_struct;

		printf("\nvalue of pointer %u",head->node);
		printf("\nvalue of next pointer %u",allocation_for_struct);
		printf("\n##########################");
		
	}

	else
	{
		next=allocation_for_struct;
		printf("\n enter #### %d ####name ",j+1);
		scanf("%s",next->name);

		printf("\n enter #### %d ####city ",j+1);
		scanf("%s",next->city);

		printf("\n enter #### %d ####marks ",j+1);
		scanf("%d",&next->marks);

		printf("\nvalue of original pointer %u",allocation_for_struct);
		allocation_for_struct=(RECORD *)(malloc(sizeof(RECORD)));
		next->node=allocation_for_struct;
		printf("\nvalue of pointer %u",next->node);
		printf("\nvalue of next pointer %u",allocation_for_struct);
		printf("\n##########################");

	}


}
		next->node = NULL;
		printf("\nvalue of pointer %u",next->node);
		free(allocation_for_struct);
		printf("To read the list : please press 1 :: ");
	scanf("%d",&g);
	if(g==1)
	{
		reading_values(head);
		free_allocated_memory(head);
	}
return 0;
}

/* Function to read values from the list*/

 void reading_values(RECORD *head)
 {
	 RECORD *pointer_list_retreaval;
	 int count=1;
	 pointer_list_retreaval=head;

	while(pointer_list_retreaval!=NULL)
	{
		printf("\n @@@@@@@@@@@@@@@@@@@HERE IS YOUR DATA @@@@@@@@@@@@@@@@@@@@@@@@@@@@");
		printf("\n %d name  :: %s #### ",count,pointer_list_retreaval->name);
		printf("\n %d city  :: %s #### ",count,pointer_list_retreaval->city);
		printf("\n %d marks :: %d ####marks ",count,pointer_list_retreaval->marks);
		printf("\n pointer location ###### %u ######",pointer_list_retreaval);
		printf("\n next pointer location ###### %u ######",pointer_list_retreaval->node);
		printf("\n @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
		pointer_list_retreaval=pointer_list_retreaval->node;
		count++;
	}
 
 }


 /*Freeing memory*/
 void free_allocated_memory(RECORD *head)
 {
	 RECORD *pointer_list_retreaval,*temp;
	 pointer_list_retreaval=head;
	 printf("\n#########################################");
	 printf("\n####### This is head %u #######",head); 

	 	while(pointer_list_retreaval!=NULL)
	{
		temp=pointer_list_retreaval;
		printf("\n #######this is pointer to the list %u #####",pointer_list_retreaval);
		pointer_list_retreaval=temp->node;
		printf("\n #######this is pointer temp %u #####",temp);
		free(temp);
		
		
		}

 
 
 }