Thread: linked list newbie

Hi all!

I'm new to linked lists. I hope the beginning of the code is OK. In any case the end of the code is wrong according to the compiler.

Code:

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

char string[10];
struct node
{
	char *word;
	struct node *next;
};

int main(void)
{

	struct node *Head=NULL;		
	Head = malloc( sizeof(struct node));  
	struct node *root=NULL;
	Head->next = root;

	while(string[0] !='q')
	{
		printf("\nInput string:\n");
		scanf("%s", string);
		root = malloc( sizeof(struct node));  
		root->word = string;
		root->next = root;
	}
	printf("\nHere is the list:\n");
	struct node* current = Head;
	while (current != NULL)
	{ 
		printf(" %s\n"),word;
		current = current->next;
	}
	struct node* current = Head;
	while (current != NULL)
	{ 
		free(node*);
		current = current->next;
	}

	return 0;
}

Thanks in advance for help.

Thanks for the answers!

Unfortunately I don't know how to fix the problem with printing the strings and how to free memory. I just copied the "current" loop idea from Linked List Basics

Well those are pretty fundamental things you don't know how to do. If you keep reading that resource and practice along side it you will get what to do in the general case for linked lists.

Anyway, as long as you store a copy of the word and use printf you can print the strings, so there is one problem solved. Like I said earlier, assigning pointers will not do what you want. You want to call strcpy(word , string); after you've allocated space for the word entry.

If you're going to insert a lot of consecutive values I typically implement it with at least two types of pointers. I use a pointer to know where I am in the list, and to connect the list together, and another for the allocation. Other than that I think you have the idea down. Just do what I'm neglecting to do right now: debug your code.

When you delete the linked list, make sure you remember your place in the list until you're finished.

Code:

struct node *p = head;
struct node *n = NULL;
while ( p ) {
   n = p->next;
   free(p->word);
   free(p);
   p = n;
}

Thanks a lot!

I fixed it

hmmm... I thought I had fixed it... but now I get:
glibc detected *** test5: double free or corruption
...after the scanf-loop.

any ideas?


here is the code

Code:

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

char string[10];
struct node
{
	char *word;
	struct node *next;
};

int main(void)
{

	struct node* head = NULL;
	struct node* p = NULL;
	struct node* n = NULL;
	
	
	head = malloc(sizeof(struct node));
	head->word = malloc(10);
	head->word = "List:";
	n = head->next;
	
	while ( string[0] !='q' ) 
	{
		p = n;
		printf("\nInput string:\n");
		scanf("%s", string);
		p = malloc(sizeof(struct node));
		p->word = malloc(10);
		strcpy(p->word, string);
		n = p->next;
	}
	
	printf("\n%s",head->word);
	p = head->next;
	while(p!=NULL)
	{
		printf("\n%s",p->word);
		p = p->next;
	}

	n = head->next;
	free(head->word);
	free(head);
	p = n;
	while (p != NULL)
	{
		n = p->next;
		free(p->word);
		free(p);
		p = NULL;
		p = n;
	}
	printf("\n");
	return 0;
}

> head->word = "List:";

While this is not wrong, you free this string which is probably where the "corruption" problem originates. You cannot free strings pointed-to like this. Decide what you want to do here: either you eschew allocating head->word and treat the head differently, or you treat the head like every other node, copying into head->word, etc.

Thanks whiteflags!

The error is solved, but it still doesn't want to print the list. To see what was wrong I added some lines. Now I get a segmentation fault when I have finished the list input. The computer doesn't like "printf("\n%s",p->word);". But I can't understand why...

Code:

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

char string[10];
struct node
{
	char *word;
	struct node *next;
};

int main(void)
{

	struct node* head = NULL;
	struct node* p = NULL;
	struct node* n = NULL;
	
	
	head = malloc(sizeof(struct node));
	head->word = malloc(10);
	head->word = NULL;
	n = head->next;
	
	while ( string[0] !='q' ) 
	{
		p = n;
		printf("\nInput string:\n");
		scanf("%s", string);
		p = malloc(sizeof(struct node));
		p->word = malloc(10);
		strcpy(p->word, string);
		n = p->next;
	}
	
	
	p = head->next;
	printf("\n%s",p->word);
	p = p->next;
	while(p!=NULL)
	{
		printf("\n%s",p->word);
		p = p->next;
	}

	n = head->next;
	free(head->word);
	free(head);
	p = n;
	while (p != NULL)
	{
		n = p->next;
		free(p->word);
		free(p);
		p = NULL;
		p = n;
	}
	printf("\n");
	return 0;
}

That basically means we haven't made any progress.

struct node* p = NULL;
struct node* n = NULL;

These need real names.

Also you can't build a list like you're going to delete it. There's a lot of similarity between the delete and insertion parts of the program. It's more like this:

Code:

p = head; /* start at the head */

while (string[0] != 'q') 
{
  printf("enter string: \n");
  scanf("%9s" , string);

   n = malloc(sizeof *n);

   /* fill in your node's fields */
   n->word = malloc(sizeof string);
   strcpy(n->word , string);
   n->next = NULL;

  /* connect your node */
  p->next = n;
  p = p->next; /* p == n here, so that the next node is after n */
}

See how one pointer holds the new node, and the other just keeps track of the place? When the loop quits you should have a good list starting from head and ending at NULL.

You might also want to look at the head again. Right now you've leaked head->word's memory.

Thanks a lot!

Now I know how to deal with linked lists.

Here is the final code. Perhaps it can be helpful for other newbies.

Code:

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

char string[10];
struct node
{
	char *word;
	struct node *next;
};

int main(void)
{

	struct node* head = NULL;	
	struct node* p = NULL;
	struct node* n = NULL;

	head = malloc(sizeof *head);
	head->next = malloc(sizeof(struct node));
	p = head->next;
	p->word = malloc(sizeof string);
			
	while (string[0] != 'q') 
	{
		printf("Enter string:\n");
		scanf("%s" , string);

		strcpy(p->word, string);
		p->next = malloc(sizeof(struct node));
		n = p->next;
		n->word = malloc(sizeof string);
		p=n;
	}
	
	printf("The string list:\n");
	p = head->next;
	while(p!=NULL)
	{
		printf("%s\n",p->word);
		p = p->next;
	}

	p = head->next;
	free(head->word);
	free(head);
	head = NULL;
	while (p != NULL)
	{
		n = p->next;
		free(p->word);
		free(p);
		p = NULL;
		p = n;
	}
	printf("\n");
	return 0;
}

string[10] doesnt have to be global. I mean, you only have one function, main(). The global scope of string[10] wouldn't serve anything.
Not that it is wrong, but global variables should be avoided. So when there is no reason to make a variable global, don't

I'm learning liked lists in C too, I see you got it how linked lists work so here is how I would go about solving that problem, I used 3 pointers because that is how I'm used to code linked lists in C.
Hope that helps.

Code:

/* 
 * File: liked.c
 * -------------
 * Creates a linked list and prints the strings
 * entered by the user.
 */

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

typedef struct node{
        char word[10];
        struct node *next;
        }NODE;
        
int main(void)
{
    NODE *head = NULL;
    NODE *prev,*curr;
    char str[10];
    
    printf("[ENTER TO QUIT] String: ");
    
    while( gets(str) != NULL && str[0] != '\0' )
    {
           curr = (NODE*) malloc(sizeof(NODE));
           
           //if head == NULL so it is the first position
           if ( head == NULL )
           head = curr;
           else
           //prev->next = current position
           prev->next = curr;
           
           //set the curr pointer to be the end
           //of the linked list
           curr->next = NULL;
           prev = curr;
           
           strcpy(curr->word,str);
           printf("String: ");
    }
    
    curr = head;
    
    // there is a null at the end of the linked list
    // so we can do while != NULL
    
    while ( curr != NULL )
    {
          printf("%s\n",curr->word);
    // set the current position to be the next position
          curr = curr->next;
    }
    
    return 0;
}