“while( !feof( file ) )” is always wrong

It's wrong because (in the absence of a read error) it enters the loop one more time than the author expects. If there is a read error, the loop never terminates.

Although there may be cases when it is correct to use this form, I cannot think of any and in 100% of the cases that I've seen this used it has been incorrect. Further, it is so gratingly ugly that even if a case were encountered where it would generate the correct semantics, it should be avoided because it looks so wrong.

Consider the following code:

/* WARNING: demonstration of bad coding technique*/

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

FILE *Fopen( const char *path, const char *mode );
int
main( int argc, char **argv )
{
    FILE *in;
    unsigned count;

    in = argc > 1 ? Fopen( argv[ 1 ], "r" ) : stdin;
    count = 0;

    /* WARNING: this is a bug */
    while( !feof( in )) {  /* This is WRONG! */
        (void) fgetc( in );
        count++;
    }
    printf( "Number of characters read: %u\n", count );
    return EXIT_SUCCESS;
}

FILE *
Fopen( const char *path, const char *mode )
{
    FILE *f = fopen( path, mode );
    if( f == NULL ) {
        perror( path );
        exit( EXIT_FAILURE );
    }
    return f;
}

This program will consistently print one greater than the number of characters in the input stream (assuming no read errors). Consider the case where the input stream is empty:

$ ./a.out < /dev/null
Number of characters read: 1

In this case, feof() is called before any data has been read, so it returns false. The loop is entered,fgetc() is called (and returns EOF), and count is incremented. Then feof() is called and returns true, causing the loop to abort.

This happens in all such cases. feof() does not return true until after a read on the stream encounters the end of file. The purpose of feof() is NOT the check if the next read will reach the end of file. The purpose of feof() is distinguish between a read error and having reached the end of the file. If fread() returns 0, you must use feof/ferror to decide. Similarly if fgetc returns EOF.feof() is only useful after fread has returned zero or fgetc has returned EOF. Before that happens, feof() will always return 0.

It is always necessary to check the return value of a read (either an fread(), or an fscanf(), or anfgetc()) before calling feof().

Even worse, consider the case where a read error occurs. In that case, fgetc() returns EOF,feof() returns false, and the loop never terminates. In all cases where while(!feof(p)) is used, there must be at least a check inside the loop for ferror(), or at the very least the while condition should be replaced with while(!feof(p) && !ferror(p)) or there is a very real possibility of an infinite loop, probably spewing all sorts of garbage as invalid data is being processed.

So, in summary, although I cannot state with certainty that there is never a situation in which it may be semantically correct to write "while(!feof(f))" (although there must be another check inside the loop with a break to avoid a infinite loop on a read error), it is the case that it is almost certainly always wrong. And even if a case ever arose where it would be correct, it is so idiomatically wrong that it would not be the right way to write the code. Anyone seeing that code should immediately hesitate and say, "that's a bug". And possibly slap the author (unless the author is your boss in which case discretion is advised.)

Posted in: C faq