From 2cc7bad0ae0a412e75270be5ed41d45c03e7a931 Mon Sep 17 00:00:00 2001 From: Zack Weinberg Date: Wed, 21 Feb 2018 19:12:51 -0500 Subject: [BZ 1190] Make EOF sticky in stdio. C99 specifies that the EOF condition on a file is "sticky": once EOF has been encountered, all subsequent reads should continue to return EOF until the file is closed or something clears the "end-of-file indicator" (e.g. fseek, clearerr). This is arguably a change from C89, where the wording was ambiguous; the BSDs always had sticky EOF, but the System V lineage would attempt to read from the underlying fd again. GNU libc has followed System V for as long as we've been using libio, but nowadays C99 conformance and BSD compatibility are more important than System V compatibility. You might wonder if changing the _underflow impls is sufficient to apply the C99 semantics to all of the many stdio functions that perform input. It should be enough to cover all paths to _IO_SYSREAD, and the only other functions that call _IO_SYSREAD are the _seekoff impls, which is OK because seeking clears EOF, and the _xsgetn impls, which, as far as I can tell, are unused within glibc. The test programs in this patch use a pseudoterminal to set up the necessary conditions. To facilitate this I added a new test-support function that sets up a pair of pty file descriptors for you; it's almost the same as BSD openpty, the only differences are that it allocates the optionally-returned tty pathname with malloc, and that it crashes if anything goes wrong. [BZ #1190] [BZ #19476] * libio/fileops.c (_IO_new_file_underflow): Return EOF immediately if the _IO_EOF_SEEN bit is already set; update commentary. * libio/oldfileops.c (_IO_old_file_underflow): Likewise. * libio/wfileops.c (_IO_wfile_underflow): Likewise. * support/support_openpty.c, support/tty.h: New files. * support/Makefile (libsupport-routines): Add support_openpty. * libio/tst-fgetc-after-eof.c, wcsmbs/test-fgetwc-after-eof.c: New test cases. * libio/Makefile (tests): Add tst-fgetc-after-eof. * wcsmbs/Makefile (tests): Add tst-fgetwc-after-eof. --- libio/Makefile | 3 +- libio/fileops.c | 7 ++- libio/oldfileops.c | 7 ++- libio/tst-fgetc-after-eof.c | 109 ++++++++++++++++++++++++++++++++++++++++++++ libio/wfileops.c | 4 ++ 5 files changed, 121 insertions(+), 9 deletions(-) create mode 100644 libio/tst-fgetc-after-eof.c (limited to 'libio') diff --git a/libio/Makefile b/libio/Makefile index 3e08ed0eeb..cbe14a8e25 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -64,7 +64,8 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \ bug-memstream1 bug-wmemstream1 \ tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \ tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \ - tst-ftell-append tst-fputws tst-bz22415 + tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof + ifeq (yes,$(build-shared)) # Add test-fopenloc only if shared library is enabled since it depends on # shared localedata objects. diff --git a/libio/fileops.c b/libio/fileops.c index 79ad15351f..c9c5cbcc3c 100644 --- a/libio/fileops.c +++ b/libio/fileops.c @@ -468,11 +468,10 @@ int _IO_new_file_underflow (FILE *fp) { ssize_t count; -#if 0 - /* SysV does not make this test; take it out for compatibility */ + + /* C99 requires EOF to be "sticky". */ if (fp->_flags & _IO_EOF_SEEN) - return (EOF); -#endif + return EOF; if (fp->_flags & _IO_NO_READS) { diff --git a/libio/oldfileops.c b/libio/oldfileops.c index 7997ddf90b..5e60c8c168 100644 --- a/libio/oldfileops.c +++ b/libio/oldfileops.c @@ -294,11 +294,10 @@ attribute_compat_text_section _IO_old_file_underflow (FILE *fp) { ssize_t count; -#if 0 - /* SysV does not make this test; take it out for compatibility */ + + /* C99 requires EOF to be "sticky". */ if (fp->_flags & _IO_EOF_SEEN) - return (EOF); -#endif + return EOF; if (fp->_flags & _IO_NO_READS) { diff --git a/libio/tst-fgetc-after-eof.c b/libio/tst-fgetc-after-eof.c new file mode 100644 index 0000000000..81c9cc9940 --- /dev/null +++ b/libio/tst-fgetc-after-eof.c @@ -0,0 +1,109 @@ +/* Bug 1190: EOF conditions are supposed to be sticky. + Copyright (C) 2018 Free Software Foundation. + Copying and distribution of this file, with or without modification, + are permitted in any medium without royalty provided the copyright + notice and this notice are preserved. This file is offered as-is, + without any warranty. */ + +/* ISO C1999 specification of fgetc: + + #include + int fgetc (FILE *stream); + + Description + + If the end-of-file indicator for the input stream pointed to by + stream is not set and a next character is present, the fgetc + function obtains that character as an unsigned char converted to + an int and advances the associated file position indicator for + the stream (if defined). + + Returns + + If the end-of-file indicator for the stream is set, or if the + stream is at end-of-file, the end-of-file indicator for the + stream is set and the fgetc function returns EOF. Otherwise, the + fgetc function returns the next character from the input stream + pointed to by stream. If a read error occurs, the error indicator + for the stream is set and the fgetc function returns EOF. + + The requirement to return EOF "if the end-of-file indicator for the + stream is set" was new in C99; the language in the 1989 edition of + the standard was ambiguous. Historically, BSD-derived Unix always + had the C99 behavior, whereas in System V fgetc would attempt to + call read() again before returning EOF again. Prior to version 2.28, + glibc followed the System V behavior even though this does not + comply with C99. + + See + , + , + and the thread at + + for more detail. */ + +#include +#include + +#include +#include +#include +#include +#include + +#define XWRITE(fd, s, msg) do { \ + if (write (fd, s, sizeof s - 1) != sizeof s - 1) \ + { \ + perror ("write " msg); \ + return 1; \ + } \ + } while (0) + +int +do_test (void) +{ + /* The easiest way to set up the conditions under which you can + notice whether the end-of-file indicator is sticky, is with a + pseudo-tty. This is also the case which applications are most + likely to care about. And it avoids any question of whether and + how it is legitimate to access the same physical file with two + independent FILE objects. */ + int outer_fd, inner_fd; + FILE *fp; + + support_openpty (&outer_fd, &inner_fd, 0, 0, 0); + fp = fdopen (inner_fd, "r+"); + if (!fp) + { + perror ("fdopen"); + return 1; + } + + XWRITE (outer_fd, "abc\n\004", "first line + EOF"); + TEST_COMPARE (fgetc (fp), 'a'); + TEST_COMPARE (fgetc (fp), 'b'); + TEST_COMPARE (fgetc (fp), 'c'); + TEST_COMPARE (fgetc (fp), '\n'); + TEST_COMPARE (fgetc (fp), EOF); + + TEST_VERIFY_EXIT (feof (fp)); + TEST_VERIFY_EXIT (!ferror (fp)); + + XWRITE (outer_fd, "d\n", "second line"); + + /* At this point, there is a new full line of input waiting in the + kernelside input buffer, but we should still observe EOF from + stdio, because the end-of-file indicator has not been cleared. */ + TEST_COMPARE (fgetc (fp), EOF); + + /* Clearing EOF should reveal the next line of input. */ + clearerr (fp); + TEST_COMPARE (fgetc (fp), 'd'); + TEST_COMPARE (fgetc (fp), '\n'); + + fclose (fp); + close (outer_fd); + return 0; +} + +#include diff --git a/libio/wfileops.c b/libio/wfileops.c index 1dbf72f797..63cb687652 100644 --- a/libio/wfileops.c +++ b/libio/wfileops.c @@ -116,6 +116,10 @@ _IO_wfile_underflow (FILE *fp) enum __codecvt_result status; ssize_t count; + /* C99 requires EOF to be "sticky". */ + if (fp->_flags & _IO_EOF_SEEN) + return WEOF; + if (__glibc_unlikely (fp->_flags & _IO_NO_READS)) { fp->_flags |= _IO_ERR_SEEN; -- cgit v1.2.3