Linked list of strings has the same strings for each node

Question

I'm trying to make a linked list, where each node stores a string, but I'm having a problem where each node ends up storing the same exact string in every single node. At the end of main(), I print out the word stored in each node, and it always just repeats the last string entered for the entire list.

I don't have any clue what is happening, because if I make it a string of characters it works perfectly fine, where each character is stored in the correct node.

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

struct wordnode {
    char *word;
    struct wordnode *next;
};

struct wordnode *link = NULL;

void addword(char *aword);

int main(void) {


    char *aword;
    int i;

    for(i = 0; i < 10; i++) {
        scanf(" %s", aword);
        addword(aword);
    }
    printf("\n");
    for(; link != NULL; link = link->next) {
        printf("|%s ", link->word);
    }

    printf("|\n");
    return 0;
}

void addword(char *aword) {
    struct wordnode *cur, *prev, *new_node;

    new_node = malloc(sizeof(struct wordnode));

    new_node->word = aword;

    for(cur = link, prev = NULL; cur != NULL; prev = cur, cur = cur->next) {
        ;
    }

    new_node->next = cur;

    if(prev == NULL) {
        link = new_node;
    } else {
        prev->next = new_node;
    }
}

Answer 1

You have not allocated memory for the string. As a result aword would contain garbage value and passing this to scanf is a undefined behavior. Lets say aword has 0x7fffe4e0cdf0 and your scanf stores the string at the address 0x7fffe4e0cdf0 and pass this address to the addword function and your structure member word is also updated with the same value. The next scanf also stores the new value in the same memory pointed by aword and passes to the function. As a result word in all the linked lists are pointing to the same memory location. The ideal solution is to allocate memory for each string being scanned and pass it to the '`addword' function.

Answer 2

There are many problems in the code. Some of them are already mentioned. The code will be something like this. Explanation at the end of the code.

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

#define STR2(x) #x
#define STR(X) STR2(X)
#define MAXWORD 10
#define MAXWORDLEN 20

struct wordnode {
    char *word;
    struct wordnode *next;
};


struct wordnode* addword(char *aword, struct wordnode *link);
void printList(struct wordnode*link);
void freeList(struct wordnode *link);

int main(void) {
    char aword[MAXWORDLEN+1];

    struct wordnode *link = NULL;
    for(size_t i = 0; i < MAXWORD; i++) {
        if( scanf("%" STR(MAXWORDLEN) "s", aword[i]) == 1 ){
           link = addword(aword, link);
        }
        else{
            fprintf(stderr, "%s\n","Error in input" );
            exit(1);
        }
    }

    printList(link);  

    freeList(link);
    return 0;
}
void printList(struct wordnode*link){
    while(link){
        printf("%s \n", link->word);
        link  = link->next;
    }
}
void freeList(struct wordnode *link){
    struct wordnode *temp;
    while(link){
        temp = link;
        link = link->next;
        free(temp);
    }
}

struct wordnode* addword(char *aword, struct wordnode *link) {

    struct wordnode *new_node = malloc(sizeof(struct wordnode));

    if( new_node == NULL){
        fprintf(stderr, "%s\n", "Error in malloc");
        exit(1);
    }
    new_node->word = strdup( aword );
    if( new_node->word == NULL){
        fprintf(stderr, "%s\n", "Error in strdup" );
        exit(1);
    }
    new_node->next = NULL;

    if( link == NULL){
        return new_node;
    }
    struct wordnode *cur = link;
    while( cur->next != NULL ){
        cur = cur -> next;
    }
    cur->next = new_node;
    return link;
}

You wanted to store some string (nul terminated char array) and then you wanted to add them in the list. Also from your example implementation you were trying to add it in the list at the tail.

To sum up -

• scanf demands a pointer to some memory where it can store inputted data. But yours were uninitialized.

• secondly the way you were copying strings, it was just a shallow copy (you were making it point to some already existing memory). You need copy it either using strdup or malloc - memcpy or malloc-strcpy.

• In case POSIX strdup() is not available, you can use what Jonathan Leffler mentioned.

• Here you can see that we have freed the allocated memory using freeList() function. When you are done working with the memory you allocated- free the memory.

• Don't cast the return value of malloc.

• Also check whether the malloc is successful checking it's return value.

• You have used the list head as the global variable. It is not needed here.

Answer 3

char * aword is not initialized and located. It should be:

char aword[100];

(100 is just a number, size of array. You can replace it with any number what you want)