Trouble with reading from file to linked list

Question

I'm trying to make function which reads child's names from text files and writes them in linked list. I've got a stucture with writing it in to the list because whole list is filled with the last name from the file.

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

typedef struct Child child;

struct Child {
    char *name;
    child *next;
};

void readFromFile(char fileName[], child **head) {
    FILE *file;

    if (!(file = fopen(fileName, "rt"))) {
        printf("Can't open file\n");
        abort();
    } else {
        static char buffer[1024];
        while (fgets(buffer, 1024, file)) {
            child *new = (child *)malloc(sizeof(child));
            new->name = buffer;
            new->next = (*head);
            (*head) = new;
        }
    }
    fclose(file);
}

void printList(child *head) {
    child *tmp = head;
    while (tmp) {
        printf("%s", tmp->name);
        tmp = tmp->next;
    }
}

int main() {
    child *head = NULL;

    readFromFile("file.txt", &head);
    printList(head);

    return 0;
}

File contains data in this style:

John
Ann
Adam
Arthur

Note that `"rt"` is not standards-compliant mode, it must be `"r"`. `"rt"` is some microsoft garbage. — Antti Haapala -- Слава Україні, May 04 '19 at 20:05
[`new->name = strdup(buffer);`](https://stackoverflow.com/a/252802/918959) (and you then need to `free` each `name` later of course. — Antti Haapala -- Слава Україні, May 04 '19 at 20:07
`printf("Can't open file\n");` is the canonical example of a useless error message. `perror(fileName)`. Prints an error which includes the path and the reason the open failed, and it prints those things to the correct stream. — William Pursell, May 04 '19 at 20:23

score 1 · Accepted Answer · answered May 04 '19 at 20:07

Your reading loop makes all the nodes point to the same static array:

    static char buffer[1024];
    while (fgets(buffer, 1024, file)) {
        child *new = (child *)malloc(sizeof(child));
        new->name = buffer;
        new->next = (*head);
        (*head) = new;
    }

You should instead allocate a copy of the string for each node:

    char buffer[1024];
    while (fgets(buffer, sizeof buffer, file)) {
        child *new_node = (child *)malloc(sizeof(child));
        new_node->name = strdup(buffer);
        new_node->next = *head;
        *head = new_node;
    }

It is also advisable to check for memory allocation failure and to avoid using c++ keywords. You might also want to strip the trailing newline from the buffer and any leading or trailing spaces.

Trouble with reading from file to linked list

1 Answers1