io.endOfFile is buggy

[timotheecour]

io.endOfFile is buggy

Example

proc endOfFile*(f: File): bool {.tags: [], benign.} =
  ## Returns true if `f` is at the end.
  var c = c_fgetc(f)
  discard c_ungetc(c, f)
  return c < 0'i32
  #result = c_feof(f) != 0

Current Output

wrong as fgetc returns EOF on either end-of-file or a read error (which could be for any reason, including EINTR) so endOfFile can have false positives.

If the stream is at end-of-file or a read error occurs, the routines return EOF.

Possible Solution

from man feof, we should use this: result = c_feof(f) != 0

The function feof() tests the end-of-file indicator for the stream pointed to by stream, returning non-zero if it is set. The end-of-file indicator may be cleared by explicitly calling clearerr(), or as a side-effect of other operations, e.g. fseek().

Additional Information

• devel 1.5.1 c39fa0d
• as mentioned in https://forum.nim-lang.org/t/2787 (good material, some of it might still be applicable today and should be addressed):

D/ endOfFile()
Why isn't C's feof() used?
If getc() makes an error, endOfFile will signal an end-of-file, which is wrong. It doesn't differentiate between file reading error and an end-of-file, it returns an end-of-file in both cases.
The return value of ungetc() is discard ed, so the function endOfFile can return a file in a broken condition without mentioning it.

• this was fixed in 794d36c (fixes IO routines are poor at handling errors #5349)
• then it was silently reverted in 0036014 as part of system refactorings (system refactorings #10559); I'm curious whether there were other subtle changes in that PR?

note

in that thread:

That said, "EOF" was considered to be the same error as "error during read" (since in both cases not much can be done about the situation)

that's not true, for eg there's EINTR (in which case you should retry) but there could be more errors; and conflating EOF with error even in case where you can't retry is wrong in many instances, for eg in the context of sameFileContent where it could lead to false positives

links

• reference: http://cplusplus.com/reference/cstdio/feof/
• FWIW D also uses feof, see io.endOfFile is buggy #16011
• found this out while reviewing Change os.sameFileContent #16008 (comment)
• https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong