[BZ 1190] Make EOF sticky in stdio.zack/sticky-eof

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---the relevant chunk of code is

    int
    _IO_new_file_underflow (_IO_FILE *fp)
    {
      _IO_ssize_t count;
    #if 0
      /* SysV does not make this test; take it out for compatibility */
      if (fp->_flags & _IO_EOF_SEEN)
        return (EOF);
    #endif

That's been unchanged since before 1995.  This only matters if the
underlying file has changed in the meantime, of course; perhaps that's
why nobody got around to filing a bug report until 2005, six years
after C99 was published.  And nobody took that bug seriously until
2012, at which time there was a long but inconclusive discussion on
libc-alpha regarding whether it would break applications to change
anything, see <https://sourceware.org/ml/libc-alpha/2012-09/msg00343.html>.

It is my considered opinion that we should just go ahead and fix the
bug, and that a backward compatibility mode is not required, because
the BSDs have always had sticky EOF, so portable code has always had
to be prepared to deal with that behavior.  Nowadays, the lineages we
should be worrying most about compatibility with are all BSD-derived,
anyway.  Thus, this patch.

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.

There is some question as to whether the test case in bug #19476
(one of the several duplicate bug reports) is valid, so instead I have
written a test case that uses a pseudoterminal to set up the necessary
conditions -- actually two test cases, one for narrow and one for wide
streams.  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.

zw

	[BZ 1190]
	* 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.

10 files changed, 391 insertions, 10 deletions

diff --git a/libio/Makefile b/libio/Makefile
index 3e08ed0eeb..a2e1c09e1c 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 <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>
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;
diff --git a/support/Makefile b/support/Makefile
index 1bda81e55e..c632df6053 100644
--- a/support/Makefile
+++ b/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 \
diff --git a/support/support_openpty.c b/support/support_openpty.c
new file mode 100644
index 0000000000..ac779ab91e
--- /dev/null
+++ b/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;
+}
diff --git a/support/tty.h b/support/tty.h
new file mode 100644
index 0000000000..1d37c42279
--- /dev/null
+++ b/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
diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile
index 3ee91d2e1a..63a6fbab58 100644
--- a/wcsmbs/Makefile
+++ b/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
diff --git a/wcsmbs/tst-fgetwc-after-eof.c b/wcsmbs/tst-fgetwc-after-eof.c
new file mode 100644
index 0000000000..9103529246
--- /dev/null
+++ b/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>