File data to a linked list in C

Question

I am really close here.

This reads the file in and using gdb I can see the words from my text file going to the linked list.

However when I print my linked list (bottom part of code) the whole linked list seems to just contain the last word from the file duplicated for however many entries are in the file.

bool load(const char* dictionary) {
    // open dictionary
    FILE* file = fopen(dictionary, "r");
    if (file == NULL)
        return false;

    char buf[LENGTH];

    //read the contents of the file and write to linked list
    while (!feof(file)) {
        fscanf(file,"%s", buf);

        // try to instantiate node
        node* newptr = malloc(sizeof(node));
        if (newptr == NULL) {
            return false;
        }

        // initialize node
        newptr->next = NULL;

        // add new word to linked list
        newptr->dictword = buf;

        //move node to start of linked list
        newptr->next = first;
        first = newptr;
    }

    fclose(file); 

    // traverse and print list.
    node* ptr = first;
    while (ptr != NULL) {
        printf("%s\n", ptr->dictword);
        ptr = ptr->next;
    }

    return true;
}

You are using `feof` incorrectly: http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong/5432517#5432517 — William Pursell, Feb 27 '13 at 23:16
bool is a terrible return type for this function. Why don't you just return a pointer to the start of the list (or NULL) ? — wildplasser, Feb 27 '13 at 23:18
fscanf(file,"%s",buf), might easily lead to a bufferoverflow — hetepeperfan, Feb 27 '13 at 23:20
Why might this lead to a buffer overflow? I think it has when my dictionary gets bigger. — Dan Heaford, Mar 02 '13 at 22:16

score 1 · Answer 1 · answered Feb 27 '13 at 23:16

You only have one char buf[] into which you read every word. You save a pointer to that buffer in every linked list element, but you immediately read over the data. You need to copy it when you put it into the allocated node. The easiest way is to use newptr->dictword = strdup(buf) which does the alloc and copy in one step.

Tuxdude · Answer 2 · 2013-02-27T23:24:53.863

Assuming dictword is of size LENGTH just as your buf, i.e:

struct node {
...
...
    char dictword[LENGTH];
...
...
};

you need to make the following changes in your code:

strcpy(newptr->dictword, buf);

The problem was that you were just setting the dictword to point to the same memory as buf in all the nodes of the linked list.

A better approach would be to dynamically allocate the memory for the string:

struct node {
...
...
    char* dictword;
...
...
};

newptr->dictword = (char*)malloc(strlen(buf) + 1);
strcpy(newptr->dictword, buf);

score 0 · Answer 3 · answered Feb 27 '13 at 23:19

0

I recommend strdup solution as BenJackson's. But it does not exist in the standard C library.

Here is my workaround for it:

char *strdup(const char *data)
{
   char *retval = malloc(strlen(data) + 1);
   strcpy(retval, data);
   return retval;
}

answered Feb 27 '13 at 23:19

Aniket Inge

25,375
5
50
78

score 0 · Answer 4 · answered Feb 27 '13 at 23:28

There is a problem with the line:

// add new word to linked list
newptr->dictword = buf;

In this way you are allways passing the reference to the beginning of the buf to all your elements, so as you update the "buf" all elements are updated. And that's why you have the referred behaviour.

Try replace it by:

newptr->dictword = (char*) malloc((strlen(buf)+1)*sizeof(char));
strcpy(newptr->dictword, buf);

The "strlen(buf)+1" is the length of the current string stored in buf plus the space for the character "\0", which means end of string.

PS. assuming you have declared the elements "dictword" with char*.

File data to a linked list in C

4 Answers4