0

This is just a snippet of my code. This function should create a linked list from a given file "dictionary.txt" which contains one word on each line, but I simply can't seem to get the links to work properly. When I run the code I get the first 2 entries properly (a,aa) but then it skips straight to the last entry (zyzzyvas) and a seg fault and it skips all other 80000 or so entries between.

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

typedef struct word {
    char entry[64];
    struct word *next;
}   WORD;

WORD *Dstart, *Dend, *curr;

WORD* makeDictionary() {

    FILE *fp;
    fp=fopen("dictionary.txt","r");

    fgets(Dstart->entry,63,fp);
    fgets(Dend->entry,63,fp);
    Dstart->next=Dend;
    int count=1;
    while(!feof(fp))
    {
        curr = (WORD *) malloc(sizeof(WORD));
        fgets(curr->entry,63,fp);
        Dend->next=curr;
        Dend=Dend->next;
        Dend->next=NULL;
        count++;
    }
    int i;
    curr=Dstart;
    for (i=1;i<=20;i++)    //Just to test if it's linking
    {
        printf("%s",curr->entry);
        curr=curr->next;
    }
}

int main {

    // curr = (WORD *) malloc(sizeof(WORD)); moved to the while loop
    Dstart = (WORD *) malloc(sizeof(WORD));
    Dend = (WORD *) malloc(sizeof(WORD));
    Dstart->next=NULL;
    Dend->next=NULL;

    makeDictionary();
    return(0);
}

Edit: Thank you Matt and interjay, moving the curr malloc into the while body fixed the problem I was having. Fixed my code.

  • oh `fgets` - we meet again. – ryrich Dec 11 '13 at 15:07
  • You really need to add some detail here: does the compiler emit any messages? How is this not behaving as expected? Don't ask us to read your code, find the problems, *and* debug it for you. You need to ask a specific question. – Ben Collins Dec 11 '13 at 15:09
  • 1
    `Dstart->next=Dend;` when this statement is reached `Dstart` is not pointing to anything. – wildplasser Dec 11 '13 at 15:09
  • Were you able to open and read the contents of the file? Maybe file doesn't exists or may be its empty. – chuthan20 Dec 11 '13 at 15:09
  • 1
    You've never allocated `Dstart` and `Dend`. – AAA Dec 11 '13 at 15:10
  • You're not allocating memory anywhere. You need to read about malloc. – egur Dec 11 '13 at 15:11
  • 1
    @ryrich: At least it's not `gets` – Matt Dec 11 '13 at 15:14
  • "fp = fopen();" need mandatory a "if (fp)" – Peter Miehle Dec 11 '13 at 15:14
  • Sorry, I'm trying to not to put up all the unnecessary code from the rest of the project. Malloc is now included. – Nathan.C.Jones Dec 11 '13 at 15:14
  • @PeterMiehle: That type of tidying up is a *looooooooong* way down the list of priorities for code like this. – Matt Dec 11 '13 at 15:16
  • Why not use getline()? If each word occupies a line, getline will get the work with trailing '\0'. Beats fget() IMO. – Xephon Dec 11 '13 at 15:18
  • Trace through the problem with a pencil and some paper. You'll see that you only ever allocate three nodes. What happens on the fourth iteration? Does that explain why your program is "skipping" words? – Matt Dec 11 '13 at 15:18
  • 1
    Please note that [`while (!feof(file))` is always wrong](http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong)! – Jonathan Leffler Dec 11 '13 at 15:21

1 Answers1

1

You are only ever allocating three nodes (at program initialization), and overwriting one of them repeatedly. You need to allocate a new node for each line in the file. For example, you can move the line curr = malloc(sizeof(WORD)); to the beginning of the while loop body.

interjay
  • 107,303
  • 21
  • 270
  • 254