diff options
| -rw-r--r-- | ChangeLog | 17 | ||||
| -rw-r--r-- | NEWS | 8 | ||||
| -rw-r--r-- | libio/Makefile | 3 | ||||
| -rw-r--r-- | libio/fileops.c | 7 | ||||
| -rw-r--r-- | libio/oldfileops.c | 7 | ||||
| -rw-r--r-- | libio/tst-fgetc-after-eof.c | 109 | ||||
| -rw-r--r-- | libio/wfileops.c | 4 | ||||
| -rw-r--r-- | support/Makefile | 1 | ||||
| -rw-r--r-- | support/support_openpty.c | 109 | ||||
| -rw-r--r-- | support/tty.h | 45 | ||||
| -rw-r--r-- | wcsmbs/Makefile | 2 | ||||
| -rw-r--r-- | wcsmbs/tst-fgetwc-after-eof.c | 114 |
12 files changed, 416 insertions, 10 deletions
diff --git a/ChangeLog b/ChangeLog index 59ab3efccb..203d274c6c 100644 --- a/ChangeLog +++ b/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. diff --git a/NEWS b/NEWS index 60dd2f778d..03330d4132 100644 --- a/NEWS +++ b/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 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 <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> |