0

While Trying to count the number of lines in a text file, I noticed that fgetc is always returning EOF. This code was working on Freebsd 10 but now it's not working on Mac OSX. I checked the file to see if it was empty, it's not, it's about 1 KB in size and contains 16 lines. I added a line to seek to the beginning of the file thinking that's the problem, but it's still returning EOF. So why is fgetc always returning EOF?

 int getLines(int listFd, int *lines)
 {

     /* Declarations */
     *lines = 0;
     int ch;
     FILE *list;

     /* Get File Stream */
     list = fdopen(listFd, "r");
     if(list == NULL)
     {
         printf("Can't Open File stream\n");
         return -1;
     }

     /* Seek To beginning Of file */
     fseek(list, 0, SEEK_SET);

     /* Get Number of Lines */
     while(!feof(list))
     {
         ch = fgetc(list);
         if(ch == '\n')
         {
             lines++;
         }

         else if(ch == EOF)
         {
             break;
         }
     }

     printf("lines: %d\n", *lines);

     /* Clean up and Exit */
     fclose(list);

    return 0;
}
2trill2spill
  • 1,333
  • 2
  • 20
  • 41
  • 4
    What's the `feof` loop for? – Carl Norum Sep 18 '14 at 17:31
  • http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong – William Pursell Sep 18 '14 at 17:33
  • Is fd a stream, or a regular file? Check the return of fseek. – William Pursell Sep 18 '14 at 17:33
  • Documentation for fgetc says it can also return EOf if a reading error occurs(It will set error indicator ferror). What is the state of that indicator? – Grice Sep 18 '14 at 17:33
  • To loop through each char until EOF, but it was looping forever so I added the else if(ch == EOF) during debugging so it would stop looping. I though the feof in the loop would achieve that but it didin't. – 2trill2spill Sep 18 '14 at 17:36
  • the error it specified was Device not configured – 2trill2spill Sep 18 '14 at 17:37
  • thanks William That looks useful and i didn't see it earlier when researching. listFd is a file descriptor and list is a file stream. – 2trill2spill Sep 18 '14 at 17:40
  • 3
    You are probably running into undefined behavior due to `line++;`. That should be `++(*line);`. – R Sahu Sep 18 '14 at 17:41
  • Additionally, you may find `ferror()` and testing the result of your `fseek()` both interesting. – WhozCraig Sep 18 '14 at 17:44
  • I'm surprised the compiler didn't warn me, but dereferencing the pointer didn't fix the EOF problem, although it probably would have crashed when the EOF problem is fixed. – 2trill2spill Sep 18 '14 at 17:46
  • 1
    Don't use `feof` to determine when to terminate the loop. Use the value returned by `fgetc`. (You can call `feof` and/or `ferror` after the loop terminates to find out *why* it terminated.) The common idiom is `while ((ch = fgetc(list)) != EOF) { /* ... */ }` – Keith Thompson Sep 18 '14 at 17:48
  • when i added error Checking to fseek it fails with illegal seek. Does that mean the File stream is bad? or i just used fseek wrong? – 2trill2spill Sep 18 '14 at 17:49
  • I switched to using fgetc in a loop like williams link and keiths comment said, but ferror gets set and perror returns Device not configured. – 2trill2spill Sep 18 '14 at 18:10
  • 2
    You're passing in a file descriptor and using `fdopen` to open a `FILE*` that refers to it. Why not just use a `FILE*` in the first place? Where did that file descriptor come from, and what does it refer to? You say the "file" exists and is about 1KB in size -- **what file?** – Keith Thompson Sep 18 '14 at 18:19
  • I'm using **fdopen** because the code I copied from my freebsd library was using capscium for sandboxing and that works on file descriptors. That descriptor was used earlier when creating the file so i just passed in the descriptor to this function so i didn't close it then reopen it. – 2trill2spill Sep 18 '14 at 18:27
  • Found the problem a previous function was closing the descriptor before passing it in, thanks everyone for their help and have a nice day. – 2trill2spill Sep 18 '14 at 18:38

1 Answers1

0

fgetc() should eventually return EOF. Code's other problem is certainly confusing the behavior and diagnosis. Good to test results of IO funcitons.

int getLines(int listFd, int *lines) {
  ...
  *lines = 0;
  ...
  if (fseek(list, 0, SEEK_SET)) puts("Seek error");
  ...
    ch = fgetc(list);
    if (ch == '\n') {
      // lines++;
      (*lines)++;  // @R Sahu 
    }
   ...
   printf("lines: %d\n", *lines);

   if (ferror(list)) puts("File Error - unexpected");
   if (feof(list)) puts("File EOF - expected");
}

Other stuff:

The below code is a redundant test of the end-of-file condition

/* Get Number of Lines */
while(!feof(list)) {
  ch = fgetc(list);
  ...
  else if(ch == EOF) {
    break;
  }
}

Suggested simplification (@Keith Thompson)

 /* Get Number of Lines */
 while( (ch = fgetc(list)) != EOF) {
   if(ch == '\n') {
     (*lines)++;
   }
 }

Minor point about file line count: If the file had text after the final '\n', would that count as a line? Suggest:

*lines = 0;
int prev = '\n'; 
/* Get Number of Lines */
while( (ch = fgetc(list)) != EOF) {
  if(prev == '\n') {
    (*lines)++;
  }
  prev = ch;
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • I had switched to your bottom suggestion already except with ferror below the loop and ferror keeps getting set and perror returns Device not configured. – 2trill2spill Sep 18 '14 at 18:18
  • 1) Show how `listFd` was set. 2) might as well drop the `fseek()`. – chux - Reinstate Monica Sep 18 '14 at 18:23
  • It is possible the the file has a error all ready associated with it before it called this function. Use `clearerr()` or `rewind()` instead of `fseek(stream, 0L, SEEK_SET)` as that also clears the indicator for the stream and/or test for errors before doing any `fgetc()`. – chux - Reinstate Monica Sep 18 '14 at 18:34