Linked list nodes added to the same address

Question

I'm reading a text file in UTF-8 using fgetc and separating words. I made an append function to add each word to a linked list, but when I print out the address of the words they are all the same, indicating they are just being overwritten. How do I correctly add data to my list?

I also made a print ruction to traverse the list and although the data prints out correctly in my append function, the print function just gives a garbage value.

struct node
{
    void *data;
    struct node *next;
};

I type def this to linked_list

I call the append function in my main each time I get a new word.

void append(linked_list *list, void *word)
{

    if(list->data == NULL)
    {
        list->data = word;
        list->next = NULL;
                //printf("WORD: %s\n", (char *)list->data);
        //printf("ADDRESS %p\n", list->data);
    }
    else
    {
        linked_list *new_node;
        new_node = malloc(sizeof(linked_list));
        new_node->data = word;
        new_node->next = NULL;

        while(list->next != NULL)
        {
            if(list->next == NULL)
            {
                list->next = new_node;
            }

        }
                //printf("WORD: %s\n", (char *)list->data);
        //printf("ADDRESS %p\n", list->data);

    }

}

And here's my print function

void print_list(linked_list *list) {

    if(list == NULL)
    {
        printf("Print: the list is empty!\n");
    }

    while (list != NULL) {
        printf("DATA %s\n", (char *)list->data);
        list = list->next;
    } 

}

I expect the print function to print

'DATA the_word' for all the words but I get ' DATA ï¿½ '

The print in the append function gives:

WORD: The
ADDRESS 0x55b6fa2314b0

WORD: Project
ADDRESS 0x55b6fa2314b0

WORD: Gutenberg
ADDRESS 0x55b6fa2314b0

WORD: EBook
ADDRESS 0x55b6fa2314b0

WORD: of
ADDRESS 0x55b6fa2314b0

WORD: Pride
ADDRESS 0x55b6fa2314b0

WORD: and
ADDRESS 0x55b6fa2314b0

WORD: Prejudice,
ADDRESS 0x55b6fa2314b0

WORD: by
ADDRESS 0x55b6fa2314b0

WORD: Jane
ADDRESS 0x55b6fa2314b0

WORD: Austen
ADDRESS 0x55b6fa2314b0

"they are all the same" -- Yes, because you probably scan them into the same buffer and then just store the address of that buffer. You should make a copy of the string when you store it. (`strdup` might be useful, but is not a standard function.) — M Oehm, Oct 19 '19 at 14:10
It isn't clear how this program can do anything at all, because `while(list->next != NULL)` loop will never terminate. — n. m. could be an AI, Oct 19 '19 at 14:13

score 0 · Answer 1 · answered Oct 19 '19 at 14:04

0

The problem is in this part:

while(list->next != NULL)
{
    if(list->next == NULL)
    {
        list->next = new_node;
    }

}

You do not go to the next node and always are in the first node. Make copy of list (to not change list itself) and iterate through it:

linked_list *copy = list;

while(copy->next != NULL)
{
    copy = copy->next;
}

copy->next = new_node;

answered Oct 19 '19 at 14:04

Miradil Zeynalli

423
4
18

This still gives me the same output! – js_hammer08 Oct 19 '19 at 14:14
The print function? I should mention, when I run the program under valgrind it does give me the right data, and different addresses. And when I just run it I get garbage values for all the words. `void print_list(linked_list *list) { if(list == NULL) { printf("Print: the list is empty!\n"); } while (list != NULL) { printf("DATA %s\n", (char *)list->data); list = list->next; } }` – js_hammer08 Oct 19 '19 at 14:31
But where do you print ADDRESS? – Miradil Zeynalli Oct 19 '19 at 14:32
1

@js_hammer08 You need to post a [mcve] rather than a collection of haphazardly edited fragments. – n. m. could be an AI Oct 19 '19 at 14:33

score 0 · Accepted Answer · answered Oct 19 '19 at 14:31

There are many problems with this program.

The representation of an empty list. It looks like you have chosen to represent an empty list with a dummy node with no data. While this is not technically wrong per se, this is not the most intuitive and straightforward representation. Usually an empty list is represented simply with a null pointer. See this SO question for more info.

The addition loop is wrong. It isn't clear how your function can ever add more than one node to a list.

while(list->next != NULL)
{
    // at this point, list->next != NULL, by definition (see loop condition)
    if(list->next == NULL) // this is always false
    {
        list->next = new_node; // this is never executed
    }
    // the loop body doesn't modify anything
}

The other answer has a way to correct this, I won't repeat it here.

Last but not least, the error you are complaining about is the result of a very common pitfall.

The solution to the question you linked fixed the issue. I get the data and it is stored at the right address. But can you explain how copying the data into another pointer first would actually make a difference? — js_hammer08, Oct 19 '19 at 14:45
@js_hammer08 You need to understand how arrays and pointers work in C, what is the difference between them, and what is the connection. This is way outside of the scope of this answer. — n. m. could be an AI, Oct 19 '19 at 14:48

Linked list nodes added to the same address

2 Answers2