[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.

ChangeLog

@@ -1,3 +1,20 @@
+2018-03-12  Zack Weinberg  <zackw@panix.com>
+
+	[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.
+
 2018-03-12  Dmitry V. Levin  <ldv@altlinux.org>
 
 	* po/pt_BR.po: Update translations.

NEWS

@@ -28,6 +28,14 @@ Deprecated and removed features, and other changes affecting compatibility:
    investigate using (f)getc_unlocked and (f)putc_unlocked, and, if
    necessary, flockfile and funlockfile.
 
+ * All stdio functions now treat end-of-file as a sticky condition.  If you
+   read from a file until EOF, and then the file is enlarged by another
+   process, you must call clearerr or another function with the same effect
+   (e.g. fseek, rewind) before you can read the additional data.  This
+   corrects a longstanding C99 conformance bug.  It is most likely to affect
+   programs that use stdio to read interactive input from a terminal.
+   (Bug #1190.)
+
  * The macros 'major', 'minor', and 'makedev' are now only available from
    the header <sys/sysmacros.h>; not from <sys/types.h> or various other
    headers that happen to include <sys/types.h>.  These macros are rarely

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.

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);
+    return EOF;
-#endif
 
   if (fp->_flags & _IO_NO_READS)
     {

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);
+    return EOF;
-#endif
 
   if (fp->_flags & _IO_NO_READS)
     {

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 <stdio.h>
+       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
+   <https://sourceware.org/bugzilla/show_bug.cgi?id=1190>,
+   <https://sourceware.org/bugzilla/show_bug.cgi?id=19476>,
+   and the thread at
+   <https://sourceware.org/ml/libc-alpha/2012-09/msg00343.html>
+   for more detail.  */
+
+#include <support/tty.h>
+#include <support/check.h>
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#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 <support/test-driver.c>

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;

support/Makefile

@@ -52,6 +52,7 @@ libsupport-routines = \
   support_format_hostent \
   support_format_netent \
   support_isolate_in_subprocess \
+  support_openpty \
   support_record_failure \
   support_run_diff \
   support_shared_allocate \

support/support_openpty.c

@@ -0,0 +1,109 @@
+/* Open a pseudoterminal.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/tty.h>
+#include <support/check.h>
+#include <support/support.h>
+
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <fcntl.h>
+#include <termios.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+
+/* As ptsname, but allocates space for an appropriately-sized string
+   using malloc.  */
+static char *
+xptsname (int fd)
+{
+  int rv;
+  size_t buf_len = 128;
+  char *buf = xmalloc (buf_len);
+  for (;;)
+    {
+      rv = ptsname_r (fd, buf, buf_len);
+      if (rv)
+        FAIL_EXIT1 ("ptsname_r: %s", strerror (errno));
+
+      if (memchr (buf, '\0', buf_len))
+        return buf; /* ptsname succeeded and the buffer was not truncated */
+
+      buf_len *= 2;
+      buf = xrealloc (buf, buf_len);
+    }
+}
+
+void
+support_openpty (int *a_outer, int *a_inner, char **a_name,
+                 const struct termios *termp,
+                 const struct winsize *winp)
+{
+  int outer = -1, inner = -1;
+  char *namebuf = 0;
+
+  outer = posix_openpt (O_RDWR | O_NOCTTY);
+  if (outer == -1)
+    FAIL_EXIT1 ("posix_openpt: %s", strerror (errno));
+
+  if (grantpt (outer))
+    FAIL_EXIT1 ("grantpt: %s", strerror (errno));
+
+  if (unlockpt (outer))
+    FAIL_EXIT1 ("unlockpt: %s", strerror (errno));
+
+
+#ifdef TIOCGPTPEER
+  inner = ioctl (outer, TIOCGPTPEER, O_RDWR | O_NOCTTY);
+#endif
+  if (inner == -1)
+    {
+      /* The kernel might not support TIOCGPTPEER, fall back to open
+         by name.  */
+      namebuf = xptsname (outer);
+      inner = open (namebuf, O_RDWR | O_NOCTTY);
+      if (inner == -1)
+        FAIL_EXIT1 ("%s: %s", namebuf, strerror (errno));
+    }
+
+  if (termp)
+    {
+      if (tcsetattr (inner, TCSAFLUSH, termp))
+        FAIL_EXIT1 ("tcsetattr: %s", strerror (errno));
+    }
+#ifdef TIOCSWINSZ
+  if (winp)
+    {
+      if (ioctl (inner, TIOCSWINSZ, winp))
+        FAIL_EXIT1 ("TIOCSWINSZ: %s", strerror (errno));
+    }
+#endif
+
+  if (a_name)
+    {
+      if (!namebuf)
+        namebuf = xptsname (outer);
+      *a_name = namebuf;
+    }
+  else
+    free (namebuf);
+  *a_outer = outer;
+  *a_inner = inner;
+}

support/tty.h

@@ -0,0 +1,45 @@
+/* Support functions related to (pseudo)terminals.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _SUPPORT_TTY_H
+#define _SUPPORT_TTY_H 1
+
+struct termios;
+struct winsize;
+
+/** Open a pseudoterminal pair.  The outer fd is written to the address
+    A_OUTER and the inner fd to A_INNER.
+
+    If A_NAME is not NULL, it will be set to point to a string naming
+    the /dev/pts/NNN device corresponding to the inner fd; space for
+    this string is allocated with malloc and should be freed by the
+    caller when no longer needed.  (This is different from the libutil
+    function 'openpty'.)
+
+    If TERMP is not NULL, the terminal parameters will be initialized
+    according to the termios structure it points to.
+
+    If WINP is not NULL, the terminal window size will be set
+    accordingly.
+
+    Terminates the process on failure (like xmalloc).  */
+extern void support_openpty (int *a_outer, int *a_inner, char **a_name,
+                             const struct termios *termp,
+                             const struct winsize *winp);
+
+#endif

wcsmbs/Makefile

@@ -50,7 +50,7 @@ strop-tests :=  wcscmp wcsncmp wmemcmp wcslen wcschr wcsrchr wcscpy wcsnlen \
 tests := tst-wcstof wcsmbs-tst1 tst-wcsnlen tst-btowc tst-mbrtowc \
 	 tst-wcrtomb tst-wcpncpy tst-mbsrtowcs tst-wchar-h tst-mbrtowc2 \
 	 tst-c16c32-1 wcsatcliff tst-wcstol-locale tst-wcstod-nan-locale \
-	 tst-wcstod-round test-char-types \
+	 tst-wcstod-round test-char-types tst-fgetwc-after-eof \
 	 $(addprefix test-,$(strop-tests))
 
 include ../Rules

wcsmbs/tst-fgetwc-after-eof.c

@@ -0,0 +1,114 @@
+/* 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 fgetwc:
+
+       #include <stdio.h>
+       #include <wchar.h>
+       wint_t fgetwc (FILE *stream);
+
+   Description
+
+     If the end-of-file indicator for the input stream pointed to by
+     stream is not set and a next wide character is present, the
+     fgetwc function obtains that wide character as a wchar_t
+     converted to a wint_t 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 fgetwc function returns WEOF. Otherwise,
+     the fgetwc function returns the next wide character from the
+     input stream pointed to by stream. If a read error occurs, the
+     error indicator for the stream is set and the fgetwc function
+     returns WEOF. If an encoding error occurs (including too few
+     bytes), the value of the macro EILSEQ is stored in errno and the
+     fgetwc function returns WEOF.
+
+   The requirement to return WEOF "if the end-of-file indicator for the
+   stream is set" was new in C99; the language in the 1995 edition of
+   the standard was ambiguous.  Historically, BSD-derived Unix always
+   had the C99 behavior, whereas in System V fgetwc 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
+   <https://sourceware.org/bugzilla/show_bug.cgi?id=1190>,
+   <https://sourceware.org/bugzilla/show_bug.cgi?id=19476>,
+   and the thread at
+   <https://sourceware.org/ml/libc-alpha/2012-09/msg00343.html>
+   for more detail.  */
+
+#include <support/tty.h>
+#include <support/check.h>
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <wchar.h>
+
+#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 (fgetwc (fp), L'a');
+  TEST_COMPARE (fgetwc (fp), L'b');
+  TEST_COMPARE (fgetwc (fp), L'c');
+  TEST_COMPARE (fgetwc (fp), L'\n');
+  TEST_COMPARE (fgetwc (fp), WEOF);
+
+  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 (fgetwc (fp), WEOF);
+
+  /* Clearing EOF should reveal the next line of input.  */
+  clearerr (fp);
+  TEST_COMPARE (fgetwc (fp), L'd');
+  TEST_COMPARE (fgetwc (fp), L'\n');
+
+  fclose (fp);
+  close (outer_fd);
+  return 0;
+}
+
+#include <support/test-driver.c>