C linked list - inserting node at the end

Question

I am trying to read from a list that has 1 word per line and 20 lines, and then add each word to the end of a linked list. My problem is that when I print out the linked list at the end it is printing that last word in the file 20 times instead of each word once. I have been working on this for hours and can't figure out what i'm doing wrong.

In my main I have

while (!feof(input)) {
        fscanf(input, "%s", currentName);
        head = insert(head, currentName);
}
print(head);
delete(head);

Insert Function

node* insert(node* head, char* name) {
        node *temp = NULL;
        if (head == NULL) {
                head = create_node(name);
        }
        else {
                temp = head;
                while(temp->next != NULL){
                        temp = temp->next;
                }
                temp->next = create_node(name);
            }
        return head;
}

Create Function

node* create_node(char* name) {
        node *newNode;
        newNode = malloc(sizeof(node));
        if(newNode == NULL) {
                printf("Failed to create node");
        }
        newNode->name = name;
        newNode->next = NULL;
        return newNode;
}

Print and Delete

void print(node* head){
        while (head != NULL) {
                printf("%s -> ", head->name);
                head = head->next;
        }
        printf("NULL\n");
}
void delete(node* head) {
        node *temp = NULL;
        while(head != NULL) {
                temp = head;
                head = head->next;
                free(temp);
        }
}

score 2 · Accepted Answer · edited May 23 '17 at 12:15

You're saving the address of the same buffer back in main() for each and every insertion. Each node is simply holding the base address of currentName, the content of which is changed with each input processed. Therefore you have a linked list of structures containing name pointers, where each points to the same buffer (currentName). Thus the last one will be the only one you see.

You need to dynamically allocate space for the name in create_node. The following uses the POSIX function strdup() to do this, though you're perfectly free to use a strlen/malloc combination if you desire.

node* create_node(char* name) 
{
    node *newNode;
    newNode = malloc(sizeof(node));
    if(newNode == NULL)
        printf("Failed to create node");

    newNode->name = strdup(name);
    newNode->next = NULL;
    return newNode;
}

Don't forget when you're cleanup up your linked list to free() each node name to avoid a memory leak.

void delete(node* head) 
{
    node *temp = NULL;
    while(head != NULL) 
    {
        temp = head;
        head = head->next;
        free(temp->name);
        free(temp);
    }
}

Unrelated: Your while-loop condition for loading your content is wrong. Read this answer to see why

C linked list - inserting node at the end

1 Answers1