CODEWITHSUNDEEP
clinked-list
user3447260
user3447260

Reputation: 1

Setting up a linked list in c

I'm trying to set up a linked list but just get the same element in every location -

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

#define LENGTH 45

typedef struct node
{
    char* word;
    struct node* next;
}
node;


int main(void)
{

    node* head = NULL;  //start of list
    // open input file 
    FILE* inptr = fopen("smalllocal", "r");
    if (inptr == NULL)
    {
        printf("Could not open %s.\n", "smalllocal");
        return 2;
    }
    printf("Opened file\n");

    //Get a word from dictionary
    char str1[LENGTH +1];
    while (fscanf(inptr, "%s", str1) != EOF)
    {      
        node* new_node = malloc(sizeof(node));  //malloc space for a new node
        if (new_node == NULL)
        {
            return 3;
        }
        new_node->word = str1;
        // is it the first insertion at this index?
        if (head == NULL)
        {
              new_node->next = head;
              head = new_node;
        }
        else
        // collision so insert at front of list
        {
            new_node->next = head;
            head = new_node;
        }
    }
    fclose(inptr);
    printf("Closed file\n");
    node* pointer = head;
    while (pointer != NULL)
    {
        printf("%s\n", pointer->word);
        pointer = pointer->next;
    }

    return 0;
}

The file 'smalllocal' contains about 15 different words but the print routine at the end just prints out the last element in the file for every location. Can anybody help please??

Upvotes: 0

Views: 110

Answers (4)

ssm
ssm

Reputation: 5383

You cannot simply do 

new_node->word = str1;

You need to allocate memory first and copy the string into memory ...

new_node -> word = (char *) malloc( sizeof(char)*(LENGTH +1) );
strcpy(new_node -> word, str1);

That should do it. Otherwise, all the pointers in the linked list are pointing to the same memory location.

Upvotes: 0

Aean
Aean

Reputation: 779

Your new_node->word is a pointer, it doesn't contain any characters. All nodes points to the same block of memory. When you insert a new node, you changed the content of str1 so it just prints out the last string.

Use new_node->word = strdup(str1); instead. You need to include string.h header.

Upvotes: 1

Natan Streppel
Natan Streppel

Reputation: 5866

You are allocating memory for your strcut, but you also need to allocate memory for your string. Change your 

new_node->word = str1;

for

new_node->word = malloc(strlen(str1)+1);
strcpy(new_node->word, str1);

so that you allocate the necessary memory to hold the string and then copy it to this allocated memory. Otherwise, all of your nodes word pointer will be pointing to the same string, str1.

Upvotes: 1

TypeIA
TypeIA

Reputation: 17258

This isn't the correct way to copy strings in C (you can't assign them using =).

Instead, you need to allocate a character array long enough to hold the string, and use strcpy().

new_node->word = malloc(strlen(str1) + 1);
strcpy(new_node->word, str1);

Don't forget to free() them later to avoid a memory leak.

The reason your program prints the same value over and over is that every node's word pointer points to the str1 array, but that array is reused for each word. So at any given time, no matter how many nodes you have, you only really have one string: inside str1, and naturally its value will be the last string read in your loop.

Upvotes: 1

Related Questions