Reading a text file into a LinkedList

Question

I am trying to read in a text file and add strings from it word by word into a Linked List. I'm fairly new at C and don't quite understand pointers. I've had a few different errors just messing around with it, but now I'm getting a segmentation fault in my insert method. It's actually quite frustrating. Can someone please explain what I am doing wrong here?

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

struct listNode {  /* self-referential structure */
   char data[50];
   struct listNode *nextPtr;
};

typedef struct listNode LISTNODE;
typedef LISTNODE *LISTNODEPTR;

void insert(LISTNODEPTR *, char[]);
void printList(LISTNODEPTR);
char fpeek(FILE *);

main() {

    FILE *fptr;
    char file_name[20];
    int nrchar = 0;
    LISTNODEPTR startPtr = (struct listNode *) malloc(sizeof(struct listNode));
    char word[50];
    char c;
    int i;

    printf("What is the name of the file in which the text is stored?\n");
    scanf("%s",file_name);
    //  printf("Type the number of characters per line");
    //scanf("%d", &nrchar);
    fptr = fopen(file_name,"r");
        while(fpeek(fptr) != EOF) {
      i = 0;
      while(fpeek(fptr) != ' '){
        word[i] = fgetc(fptr);
        i++;
        printf("%d", i);
      }
      word[strlen(word)] = '\0';
      insert(&startPtr, word);
      word[0] = '\0';
    }
    fclose(fptr);
    printList(startPtr);


return 0;
}

    /* Insert a new value into the list in sorted order */
    void insert(LISTNODEPTR *sPtr, char value[])
    {
      LISTNODEPTR newPtr, currentPtr;

      newPtr = malloc(sizeof(LISTNODE));
      strcpy(newPtr->data, value);
      newPtr->nextPtr = NULL;
      currentPtr = *sPtr;

      while(currentPtr != NULL){
        currentPtr = currentPtr->nextPtr;
      }
      currentPtr->nextPtr = newPtr;

    }


    /* Return 1 if the list is empty, 0 otherwise */
    int isEmpty(LISTNODEPTR sPtr)
    {
       return sPtr == NULL;
    }

    /* Print the list */
    void printList(LISTNODEPTR currentPtr)
    {
       if (currentPtr == NULL)
          printf("List is empty.\n\n");
       else {
          printf("The list is:\n");

          while (currentPtr != NULL) {
             printf("%s --> ", currentPtr->data);
             currentPtr = currentPtr->nextPtr;
          }

          printf("EOF\n\n");
       }
    }

    char fpeek(FILE *stream) {
        char c;
        c = fgetc(stream);
        ungetc(c, stream);
        return c;
    }

Charlie Burns · Answer 1 · 2013-10-14T22:42:26.287

3

First, check your return values from library functions like fopen() etc.

Second, see simonc's answer.

Third, after this loop:

  while(currentPtr != NULL){
    currentPtr = currentPtr->nextPtr;
  }
  currentPtr->nextPtr = newPtr;

currentPtr is null, so currentPtr->nextPtr = newPtr; will dereference a null pointer. Perhaps something like

  while(currentPtr && currentPtr->nextPtr) {
    currentPtr = currentPtr->nextPtr;
  }
  currentPtr->nextPtr = newPtr;

is more what you are looking for.

Finally,

char fpeek(FILE *stream) {
    char c;
    c = fgetc(stream);
    ungetc(c, stream);
    return c;
}

should be

int fpeek(FILE *stream) {
    int c;
    c = fgetc(stream);
    ungetc(c, stream);
    return c;
}

and in main

char fpeek(FILE *);

should be

int fpeek(FILE *);

edited Oct 14 '13 at 22:42

answered Oct 14 '13 at 21:26

Charlie Burns

6,994
20
29

I made the changes and the segfault is gone, but now I think I'm getting an infinite loop somewhere. Also, what do you mean by check the return values from library functions? – user2880410 Oct 14 '13 at 21:49
You should check the return value from fopen() to see if it is null. In this case it doesn't look like it is, but it's good coding style to check. As for the loop, I'm not sure, your nested fpeek() loops are hard to follow. – Charlie Burns Oct 14 '13 at 21:55
This 'word[strlen(word)] = '\0';' should be 'word[i] = '\0'; – Charlie Burns Oct 14 '13 at 21:55
Using printf I found it to be in my outer fpeek() loop. This would mean that the loop never finds the end of the file? I'm not sure. – user2880410 Oct 14 '13 at 22:19
The issue may be fseek() returning a char and not an int. See this answer http://stackoverflow.com/questions/9378073/getcfp-causing-trouble/9378109#9378109 – Charlie Burns Oct 14 '13 at 22:40
It didn't fix it but why should it be int? Aren't we dealing with characters? – user2880410 Oct 14 '13 at 23:20
See the answer I linked to above. getchar() returns an int, not a char. EOF is defined as -1. – Charlie Burns Oct 14 '13 at 23:22

score 0 · Answer 2 · answered Oct 14 '13 at 21:28

I took a quick look at your code and I'm pretty sure the segfault problem is here:

while(currentPtr != NULL){
   currentPtr = currentPtr->nextPtr;
}
currentPtr->nextPtr = newPtr;

What this does is that it loops through the list until currentPtr equals null. Then you are trying to assign the struct field through a null pointer (currentPtr->nextPtr), which causes the segmentation fault.

score 0 · Answer 3 · answered Oct 15 '13 at 17:21

Ok, this right here:

while(fpeek(fptr) != EOF) {
      i = 0;
      while(fpeek(fptr) != ' '){
        word[i] = fgetc(fptr);
        i++;
        printf("%d",i);
      }
      word[i] = '\0';
      insert(&startPtr, word);
      printf("%c", word[4]);
      word[0] = '\0';
    }

When I run my full code, it prints 12345ooooooooooooooooooooooo...etc. In my test file the first word is "Hello" so that's where the infinite 'o's come from. If the outer loop is the one that is infinite, then wouldn't the second while loop also execute more than once? What I mean is, why is the second print statement the only one that repeats itself?

Reading a text file into a LinkedList

3 Answers3