C while loop feof

Question

This is part of a much bigger program to make a filmgenie and for some reason the program crashes as it reaches this while loop and i don't understand what my problem is.

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

FILE *fp, *fopen();
char *arrayoffilms[45];
int main()
{
    char line[100],junk[100];
    fp=fopen("filmtitles.txt","r");
    int i=0;
    while(!feof(fp)) {
        fscanf(fp,"%[^\n]s",line);
        strcpy(arrayoffilms[i],line);
        fscanf(fp,"%[\n]s",junk);
        i++;
    }
    fclose(fp);
    printf("%s\n\n\n",arrayoffilms[i]);
    return 0;
}

Answer 1

A similar thing happens with fgets() so some people say to never use it. Look at it this way, if you say

while (!feof(ipf)) {

by the time feof() is true you've hit the end of the file. The byte you just read is garbage, maybe a NULL. Don't use it. This works:

while (!feof(ipf)) {
  if (!feof(ipf)) {
    ch = fgetc(ipf);

And it works for fgets() too, I've used it this way for years. If this were Pascal (or maybe Perl) and you read "until" feof that would work, it's a pre-test vs a post-test issue. So test twice.

Answer 2

feof will never return true until an actual read attempt is made, and EOF has been reached. The read attempts usually have return values that indicate failure. Why not use those, instead?

Don't confuse the %[ and %s format specifiers; %[ doesn't provide a scanset for %s; %[^\n]s tells scanf to read "one or more non-'\n' characters, followed by a 's' character". Does that make sense? Think about it, carefully. What is the purpose of this format specifier? What happens if the user merely presses enter, and scanf doesn't get it's "one or more non-'\n' characters"? Before we look for non-'\n' characters, it's important to get rid of any '\n' characters. Any whitespace bytes in the format string will cause scanf to consume as much whitespace as possible, up until the first non-whitespace character. I'm going to presume you wanted %[^\n], or perhaps even %99[^\n], which would prevent overflows of line.

Perhaps you'd also want to count the number of bytes processed by scanf, so you can malloc the correct length and copy into arrayoffilms, for some reason I can't imagine. You can use the %n format specifier, which will tell you how many bytes scanf processed.

I noticed that you want to read and discard the remainder of a line. In my example, the remainder of a line will only ever be discarded if 99 characters are read before a newline is encountered. I'll use the assignment suppression '*': %*[^\n].

Combining these format specifiers results in a format string of " %99[^\n]%n%*[^\n]", two arguments (a char * for %[ and an int * for %n), and an expected return value of 1 (because 1 input is being assigned). The loop will end when the return value isn't 1, which will likely be caused by an error such as "reading beyond eof".

int length;
while (fscanf(fp, " %99[^\n]%n%*[^\n]", line, &length) == 1) {
    arrayoffilms[i] = malloc(length + 1);
    strcpy(arrayoffilms[i], line);
    i++;
}

Answer 3

The problem might be about the feof. You want your while loop to terminate when you reach the end of the file, or in other words, when you can not get anything using fscanf.

You can go for the code below:

 while(fscanf(fp,"%[^\n]s",line)) {
    strcpy(arrayoffilms[i],line);
    fscanf(fp,"%[\n]s",junk);
    i++;
 }

Also, error checking associated with file pointers is absolutely necessary and is a good habbit. You would definitely want to use it:

fp=fopen("filmtitles.txt","r");
if(fp == NULL)  /* error handling */
    printf("Could not open file: filename\n"); 
else{
/* do stuff */
}