By William Pursell


2011-03-25 11:42:33 8 Comments

I've seen people trying to read files like this in a lot of posts lately.

Code

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

int main(int argc, char **argv)
{
    char *path = argc > 1 ? argv[1] : "input.txt";

    FILE *fp = fopen(path, "r");
    if( fp == NULL ) {
        perror(path);
        return EXIT_FAILURE;
    }

    while( !feof(fp) ) {  /* THIS IS WRONG */
        /* Read and process data from file… */
    }
    if( fclose(fp) == 0 ) {
        return EXIT_SUCCESS;
    } else {
        perror(path);
        return EXIT_FAILURE;
    }
}

What is wrong with this loop?

4 comments

@William Pursell 2011-03-25 12:39:08

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.

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! */
        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 to check if the next read will reach the end of file. The purpose of feof() is to distinguish between a read error and having reached the end of the file. If fread() returns 0, you must use feof/ferror to decide whether an error was encountered or if all of the data was consumed. 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 an fgetc()) 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.)

@nobar 2013-07-07 00:53:39

Sure it's wrong -- but aside from that it isn't "gratingly ugly".

@jleahy 2013-07-12 16:27:14

You should add an example of correct code, as I imagine lots of people will come here looking for a quick fix.

@Thomas 2014-08-26 22:48:54

Is this different from file.eof()?

@William Pursell 2014-08-26 23:36:53

@Thomas: I'm not a C++ expert, but I believe file.eof() returns effectively the same result as feof(file) || ferror(file), so it is very different. But this question is not intended to be applicable to C++.

@m-ric 2014-10-23 14:23:18

I suppose correct code could be: change while(!feof(in)) into do { ... } while (!feof(in)); and add a conditioned break to avoid infinite loop.

@Mark Ransom 2015-04-09 18:25:58

@m-ric that's not right either, because you'll still try to process a read that failed.

@Jack 2017-01-28 16:06:29

this is the actual correct answer. feof() is used to know the outcome of previous read attempt. Thus probably you don't want to use it as your loop break condition. +1

@luizfls 2019-08-25 02:16:10

In the middle of the answer we read "[...] fgetc() is called (and returns EOF), and count is incremented. Then feof() is called and returns true", and then a few lines below we read is "[...] fgetc() returns EOF, feof() returns false, and the loop never terminates". Why does feof() return different values in these two scenarios?

@William Pursell 2019-08-25 21:02:13

@luizfls The second case is when a read error occurs.

@AProgrammer 2012-02-10 10:22:04

feof() indicates if one has tried to read past the end of file. That means it has little predictive effect: if it is true, you are sure that the next input operation will fail (you aren't sure the previous one failed BTW), but if it is false, you aren't sure the next input operation will succeed. More over, input operations may fail for other reasons than the end of file (a format error for formatted input, a pure IO failure -- disk failure, network timeout -- for all input kinds), so even if you could be predictive about the end of file (and anybody who has tried to implement Ada one, which is predictive, will tell you it can complex if you need to skip spaces, and that it has undesirable effects on interactive devices -- sometimes forcing the input of the next line before starting the handling of the previous one), you would have to be able to handle a failure.

So the correct idiom in C is to loop with the IO operation success as loop condition, and then test the cause of the failure. For instance:

while (fgets(line, sizeof(line), file)) {
    /* note that fgets don't strip the terminating \n, checking its
       presence allow to handle lines longer that sizeof(line), not showed here */
    ...
}
if (ferror(file)) {
   /* IO failure */
} else if (feof(file)) {
   /* format error (not possible with fgets, but would be with fscanf) or end of file */
} else {
   /* format error (not possible with fgets, but would be with fscanf) */
}

@William Pursell 2012-09-29 12:59:56

Getting to the end of a file is not an error, so I question the phrasing "input operations may fail for other reasons than the end of file".

@AProgrammer 2012-09-29 15:12:09

@WilliamPursell, reaching the eof isn't necessarily an error, but being unable to do an input operation because of eof is one. And it is impossible in C to detect reliably the eof without having made an input operation fails.

@chux - Reinstate Monica 2015-03-26 21:21:03

Agree last else not possible with sizeof(line) >= 2 and fgets(line, sizeof(line), file) but possible with pathological size <= 0 and fgets(line, size, file). Maybe even possible with sizeof(line) == 1.

@BitTickler 2017-09-24 20:36:59

All that "predictive value" talk... I never thought about it that way. In my world, feof(f) does not PREDICT anything. It states that a PREVIOUS operation has hit the end of the file. Nothing more, nothing less. And if there was no previous operation (just opened it), it does not report end of file even if the file was empty to start with. So, apart of the concurrency explanation in another answer above, I do not think there is any reason not to loop on feof(f).

@supercat 2019-02-03 15:49:48

@AProgrammer: A "read up to N bytes" request that yields zero, whether because of a "permanent" EOF or because no more data is available yet, is not an error. While feof() may not reliably predict that future requests will yield data, it may reliably indicate that future requests won't. Perhaps there should be a status function that would indicate "It is plausible that future read requests will succeed", with semantics that after reading to the end of an ordinary file, a quality implementation should say future reads are unlikely to succeed absent some reason to believe they might.

@supercat 2019-02-03 15:54:39

@AProgrammer: The circumstances in which an implementation might treat end-of-file as a transitory condition would vary among implementations, but a typical reason might be that the the file was opened using "r" or "rb" mode while it was also open with "w" or "wb" mode.

@Kerrek SB 2014-10-24 22:28:36

I'd like to provide an abstract, high-level perspective.

Concurrency and simultaneity

I/O operations interact with the environment. The environment is not part of your program, and not under your control. The environment truly exists "concurrently" with your program. As with all things concurrent, questions about the "current state" don't make sense: There is no concept of "simultaneity" across concurrent events. Many properties of state simply don't exist concurrently.

Let me make this more precise: Suppose you want to ask, "do you have more data". You could ask this of a concurrent container, or of your I/O system. But the answer is generally unactionable, and thus meaningless. So what if the container says "yes" – by the time you try reading, it may no longer have data. Similarly, if the answer is "no", by the time you try reading, data may have arrived. The conclusion is that there simply is no property like "I have data", since you cannot act meaningfully in response to any possible answer. (The situation is slightly better with buffered input, where you might conceivably get a "yes, I have data" that constitutes some kind of guarantee, but you would still have to be able to deal with the opposite case. And with output the situation is certainly just as bad as I described: you never know if that disk or that network buffer is full.)

So we conclude that it is impossible, and in fact unreasonable, to ask an I/O system whether it will be able to perform an I/O operation. The only possible way we can interact with it (just as with a concurrent container) is to attempt the operation and check whether it succeeded or failed. At that moment where you interact with the environment, then and only then can you know whether the interaction was actually possible, and at that point you must commit to performing the interaction. (This is a "synchronisation point", if you will.)

EOF

Now we get to EOF. EOF is the response you get from an attempted I/O operation. It means that you were trying to read or write something, but when doing so you failed to read or write any data, and instead the end of the input or output was encountered. This is true for essentially all the I/O APIs, whether it be the C standard library, C++ iostreams, or other libraries. As long as the I/O operations succeed, you simply cannot know whether further, future operations will succeed. You must always first try the operation and then respond to success or failure.

Examples

In each of the examples, note carefully that we first attempt the I/O operation and then consume the result if it is valid. Note further that we always must use the result of the I/O operation, though the result takes different shapes and forms in each example.

  • C stdio, read from a file:

    for (;;) {
        size_t n = fread(buf, 1, bufsize, infile);
        consume(buf, n);
        if (n < bufsize) { break; }
    }
    

    The result we must use is n, the number of elements that were read (which may be as little as zero).

  • C stdio, scanf:

    for (int a, b, c; scanf("%d %d %d", &a, &b, &c) == 3; ) {
        consume(a, b, c);
    }
    

    The result we must use is the return value of scanf, the number of elements converted.

  • C++, iostreams formatted extraction:

    for (int n; std::cin >> n; ) {
        consume(n);
    }
    

    The result we must use is std::cin itself, which can be evaluated in a boolean context and tells us whether the stream is still in the good() state.

  • C++, iostreams getline:

    for (std::string line; std::getline(std::cin, line); ) {
        consume(line);
    }
    

    The result we must use is again std::cin, just as before.

  • POSIX, write(2) to flush a buffer:

    char const * p = buf;
    ssize_t n = bufsize;
    for (ssize_t k = bufsize; (k = write(fd, p, n)) > 0; p += k, n -= k) {}
    if (n != 0) { /* error, failed to write complete buffer */ }
    

    The result we use here is k, the number of bytes written. The point here is that we can only know how many bytes were written after the write operation.

  • POSIX getline()

    char *buffer = NULL;
    size_t bufsiz = 0;
    ssize_t nbytes;
    while ((nbytes = getline(&buffer, &bufsiz, fp)) != -1)
    {
        /* Use nbytes of data in buffer */
    }
    free(buffer);
    

    The result we must use is nbytes, the number of bytes up to and including the newline (or EOF if the file did not end with a newline).

    Note that the function explicitly returns -1 (and not EOF!) when an error occurs or it reaches EOF.

You may notice that we very rarely spell out the actual word "EOF". We usually detect the error condition in some other way that is more immediately interesting to us (e.g. failure to perform as much I/O as we had desired). In every example there is some API feature that could tell us explicitly that the EOF state has been encountered, but this is in fact not a terribly useful piece of information. It is much more of a detail than we often care about. What matters is whether the I/O succeeded, more-so than how it failed.

  • A final example that actually queries the EOF state: Suppose you have a string and want to test that it represents an integer in its entirety, with no extra bits at the end except whitespace. Using C++ iostreams, it goes like this:

    std::string input = "   123   ";   // example
    
    std::istringstream iss(input);
    int value;
    if (iss >> value >> std::ws && iss.get() == EOF) {
        consume(value);
    } else {
        // error, "input" is not parsable as an integer
    }
    

    We use two results here. The first is iss, the stream object itself, to check that the formatted extraction to value succeeded. But then, after also consuming whitespace, we perform another I/O/ operation, iss.get(), and expect it to fail as EOF, which is the case if the entire string has already been consumed by the formatted extraction.

    In the C standard library you can achieve something similar with the strto*l functions by checking that the end pointer has reached the end of the input string.

The answer

while(!eof) is wrong because it tests for something that is irrelevant and fails to test for something that you need to know. The result is that you are erroneously executing code that assumes that it is accessing data that was read successfully, when in fact this never happened.

@CiaPan 2015-01-29 11:16:28

The list item header '• C stdio, scanf' is wrong (or incomplete, at least). scanf is actually C stdio, however its use context is C++: C does not allow declaring variables in the for() initialization expression.

@Kerrek SB 2015-01-29 12:10:59

@CiaPan: I don't think that's true. Both C99 and C11 allow this.

@CiaPan 2015-01-29 12:25:38

But ANSI C does not.

@Jonathan Mee 2015-02-03 13:11:18

@KerrekSB I'm using an ifstream and trying to test good as my loop condition. Can you help give me an example of why this is a bad idea: stackoverflow.com/q/28299761/2642059

@Kerrek SB 2015-02-03 20:52:35

@JonathanMee: It's bad for all the reasons I mention: you cannot look into the future. You cannot tell what will happen in the future.

@Jonathan Mee 2015-02-03 20:59:49

@KerrekSB So I'm not expecting that good means my next read will work. But I am expecting that good means my previous read worked. (Or that the stream itself is readable.) In such a situation it's OK to use good?

@Kerrek SB 2015-02-03 21:02:31

@JonathanMee: Yes, that would be appropriate, though usually you can combine this check into the operation (since most iostreams operations return the stream object, which itself has a boolean conversion), and that way you make it obvious that you're not ignoring the return value.

@Kerrek SB 2015-03-26 18:13:34

@Jonathan Leffler: I think your edit breaks the code: You have to call consume(buf, n) after a short read, otherwise you lose data! Roll back?

@Jonathan Leffler 2015-03-26 18:19:40

@KerrekSB: OK; I see what you mean...my change could be appropriate if, for sake of argument, the call were n = fread(&object, sizeof(object), 1, fp); where you'd only get 0 or 1 returned. I'll revert the lines to the old order; sorry about that.

@ex-bart 2015-07-31 23:41:25

The std::cin and scanf examples happily treat bad input (something worth raising an error) as EOF (which just means leave the loop).

@WorldSEnder 2016-11-18 19:00:21

Isn't C++'s design flawed? Suppose you read some data - which succeeds, but before you're able to check the good flag, an asynchronous failing read occurs. Now you good returns false for you even though the read succeeded

@Kerrek SB 2016-11-18 20:17:05

@WorldSEnder: That sounds like a general problem with modifying shared state concurrently. If you're not the only one accessing some shared state, then you can never know what that state is "at the moment"; in fact, the very notion of "at the moment" stops being meaningful.

@JeremyP 2017-05-24 14:55:17

The Posix write example is wrong. write returns -1 if there is an error which will lead to a segmentation violation when p eventually gets down to zero or p + n references unmapped memory (once k becomes -1, your loop won't terminate but will continually subtract one from p and add one to n). There's also a more subtle problem that returning zero is not necessarily an error - e.g. with non blocking IO.

@Kerrek SB 2017-05-24 15:59:41

@JeremyP: Thanks, I fixed that. I'm not going to get into non-blocking I/O, since you wouldn't generally be able to write that as a single, local loop.

@Ghos3t 2018-06-25 03:56:23

You should provide a example with correct code for the code OP posted, instead of 4 random examples. A lot of people will be coming to read this post for a quick fix rather than a deep dive on why it is wrong. I believe the accepted answer to this post is a correct example: stackoverflow.com/questions/1835986/…

@Kerrek SB 2018-06-25 12:37:27

@Ghos3t: Isn't there a whole list of correct examples in the "Examples" part of the answer?

@Mikko Rantalainen 2018-09-06 12:22:51

I think the C example should test for feof() before exiting. If an error has occurred during fread() the read may be short even if EOF has not been met.

@Kerrek SB 2018-09-06 19:53:08

@MikkoRantalainen: That's true, but that's also true of every example -- none of them tell you the reason for the end of input, and all of them could be augmented with an additional check.

@Mikko Rantalainen 2018-09-07 13:36:08

@KerrekSB I understand. On the other hand, I would love the situation where every example provided as an answer did contain the best possible code that works correctly in corner cases, too and does not contain security vulnerabilities. These examples do not work correctly for the short read corner case as-is.

@Arne Vogel 2018-09-10 11:53:43

Third paragraph is remarkably misleading/inaccurate for an accepted and highly upvoted answer. feof() does not "ask the I/O system whether it has more data". feof(), according to the (Linux) manpage: "tests the end-of-file indicator for the stream pointed to by stream, returning nonzero if it is set." (also, an explicit call to clearerr() is the only way to reset this indicator); In this respect, William Pursell's answer is much better.

@Kerrek SB 2018-09-10 22:20:12

@ArneVogel: Hm, the third paragraph doesn't talk about feof, but about the abstract concept of performing I/O and the inability to know ahead of time whether the operation will succeed. That part is independent of programming language and any particular API. Does that make sense?

@Minh Nghĩa 2019-05-29 16:04:17

But why does Java allow a hasNext() method?

@Kerrek SB 2019-05-31 09:30:50

@MinhNghĩa: That's a blocking method, right? That's basically just a convenience wrapper around "try to read (blocking if necessary), then report the success state, and if successful store the read result in a special buffer". You can implement the same in C and in C++ if you like.

@Erik 2011-03-25 11:49:12

No it's not always wrong. If your loop condition is "while we haven't tried to read past end of file" then you use while (!feof(f)). This is however not a common loop condition - usually you want to test for something else (such as "can I read more"). while (!feof(f)) isn't wrong, it's just used wrong.

@pmg 2011-03-25 11:53:26

I wonder ... f = fopen("A:\\bigfile"); while (!feof(f)) { /* remove diskette */ } or (going to test this) f = fopen(NETWORK_FILE); while (!feof(f)) { /* unplug network cable */ }

@Erik 2011-03-25 11:56:03

@pmg: As said, "not a common loop condition" hehe. I can't really think of any case I've needed it, usually I'm interested in "could I read what I wanted" with all that implies of error handling

@Erik 2011-03-25 12:41:07

@pmg: As said, you rarely want while(!eof(f))

@William Pursell 2013-07-03 15:18:45

More accurately, the condition is "while we haven't tried to read past the end of the file and there was no read error" feof is not about detecting end of file; it is about determining whether a read was short because of an error or because the input is exhausted.

Related Questions

Sponsored Content

16 Answered Questions

[SOLVED] Writing files in Node.js

28 Answered Questions

[SOLVED] How to read a file line-by-line into a list?

16 Answered Questions

[SOLVED] How do I copy a file in Python?

58 Answered Questions

[SOLVED] How do I include a JavaScript file in another JavaScript file?

10 Answered Questions

[SOLVED] How do you append to a file in Python?

  • 2011-01-16 16:20:33
  • user502039
  • 1495835 View
  • 1443 Score
  • 10 Answer
  • Tags:   python file append

37 Answered Questions

[SOLVED] How do I check whether a file exists without exceptions?

18 Answered Questions

[SOLVED] Why should text files end with a newline?

18 Answered Questions

[SOLVED] With arrays, why is it the case that a[5] == 5[a]?

13 Answered Questions

[SOLVED] Undo working copy modifications of one file in Git?

10 Answered Questions

Sponsored Content