libio: Use stdin consistently for input functions [BZ #24153]

Message ID 87tvhnaco6.fsf@oldenburg2.str.redhat.com
State New
Headers show
Series
  • libio: Use stdin consistently for input functions [BZ #24153]
Related show

Commit Message

Florian Weimer Feb. 1, 2019, 12:58 p.m.
The internal _IO_stdin_ variable is not updated when the application
assigns to stdin, which is a GNU extension.

2019-02-01  Florian Weimer  <fweimer@redhat.com>

	[BZ #24153]
	* debug/gets_chk.c (__gets_chk): Use stdin instead of _IO_stdin.
	* libio/getchar.c (getchar): Likewise.
	* libio/getchar_u.c (getchar_unlocked): Likewise.
	* libio/getwchar.c (getwchar): Likewise.
	* libio/getwchar_u.c (getwchar_unlocked): Likewise.
	* libio/iogets.c (_IO_gets): Likewise.
	* libio/vscanf.c (_IO_vscanf): Likewise.
	* libio/vwscanf.c (__vwscanf): Likewise.
	* libio/tst-bz24153.c: New file.
	* libio/Makefile (tests): Add it.

Comments

Paul Pluzhnikov Feb. 2, 2019, 11:58 p.m. | #1
On Fri, Feb 1, 2019 at 4:58 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> The internal _IO_stdin_ variable is not updated when the application
> assigns to stdin, which is a GNU extension.
>
> 2019-02-01  Florian Weimer  <fweimer@redhat.com>
>
>         [BZ #24153]

I didn't realize there was a separate (from BZ 24051) bug for stdin,
and prepared effectively this same patch.

Looks good to me.
Thanks!
Florian Weimer Feb. 3, 2019, 9:52 a.m. | #2
* Paul Pluzhnikov:

> On Fri, Feb 1, 2019 at 4:58 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> The internal _IO_stdin_ variable is not updated when the application
>> assigns to stdin, which is a GNU extension.
>>
>> 2019-02-01  Florian Weimer  <fweimer@redhat.com>
>>
>>         [BZ #24153]
>
> I didn't realize there was a separate (from BZ 24051) bug for stdin,
> and prepared effectively this same patch.
>
> Looks good to me.
> Thanks!

Thanks, and sorry about that.  I have pushed my patch.

I'm testing another patch which removes the internal pointer variables
altogether, similar to what I posted before.

Florian

Patch

diff --git a/debug/gets_chk.c b/debug/gets_chk.c
index 4b5e13d6a6..fc6557a7f7 100644
--- a/debug/gets_chk.c
+++ b/debug/gets_chk.c
@@ -37,8 +37,8 @@  __gets_chk (char *buf, size_t size)
   if (size == 0)
     __chk_fail ();
 
-  _IO_acquire_lock (_IO_stdin);
-  ch = _IO_getc_unlocked (_IO_stdin);
+  _IO_acquire_lock (stdin);
+  ch = _IO_getc_unlocked (stdin);
   if (ch == EOF)
     {
       retval = NULL;
@@ -51,24 +51,24 @@  __gets_chk (char *buf, size_t size)
       /* This is very tricky since a file descriptor may be in the
 	 non-blocking mode. The error flag doesn't mean much in this
 	 case. We return an error only when there is a new error. */
-      int old_error = _IO_stdin->_flags & _IO_ERR_SEEN;
-      _IO_stdin->_flags &= ~_IO_ERR_SEEN;
+      int old_error = stdin->_flags & _IO_ERR_SEEN;
+      stdin->_flags &= ~_IO_ERR_SEEN;
       buf[0] = (char) ch;
-      count = _IO_getline (_IO_stdin, buf + 1, size - 1, '\n', 0) + 1;
-      if (_IO_stdin->_flags & _IO_ERR_SEEN)
+      count = _IO_getline (stdin, buf + 1, size - 1, '\n', 0) + 1;
+      if (stdin->_flags & _IO_ERR_SEEN)
 	{
 	  retval = NULL;
 	  goto unlock_return;
 	}
       else
-	_IO_stdin->_flags |= old_error;
+	stdin->_flags |= old_error;
     }
   if (count >= size)
     __chk_fail ();
   buf[count] = 0;
   retval = buf;
 unlock_return:
-  _IO_release_lock (_IO_stdin);
+  _IO_release_lock (stdin);
   return retval;
 }
 
diff --git a/libio/Makefile b/libio/Makefile
index dbeba88fc0..95b91084dd 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -65,7 +65,7 @@  tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
 	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-fgetc-after-eof \
-	tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051
+	tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153
 
 tests-internal = tst-vtables tst-vtables-interposed tst-readline
 
diff --git a/libio/getchar.c b/libio/getchar.c
index 7e385305ff..90dea38ddb 100644
--- a/libio/getchar.c
+++ b/libio/getchar.c
@@ -33,11 +33,11 @@  int
 getchar (void)
 {
   int result;
-  if (!_IO_need_lock (_IO_stdin))
-    return _IO_getc_unlocked (_IO_stdin);
-  _IO_acquire_lock (_IO_stdin);
-  result = _IO_getc_unlocked (_IO_stdin);
-  _IO_release_lock (_IO_stdin);
+  if (!_IO_need_lock (stdin))
+    return _IO_getc_unlocked (stdin);
+  _IO_acquire_lock (stdin);
+  result = _IO_getc_unlocked (stdin);
+  _IO_release_lock (stdin);
   return result;
 }
 
diff --git a/libio/getchar_u.c b/libio/getchar_u.c
index 9af50be717..a933d9aad0 100644
--- a/libio/getchar_u.c
+++ b/libio/getchar_u.c
@@ -32,5 +32,5 @@ 
 int
 getchar_unlocked (void)
 {
-  return _IO_getc_unlocked (_IO_stdin);
+  return _IO_getc_unlocked (stdin);
 }
diff --git a/libio/getwchar.c b/libio/getwchar.c
index baac916fdd..b6155fa276 100644
--- a/libio/getwchar.c
+++ b/libio/getwchar.c
@@ -33,8 +33,8 @@  wint_t
 getwchar (void)
 {
   wint_t result;
-  _IO_acquire_lock (_IO_stdin);
-  result = _IO_getwc_unlocked (_IO_stdin);
-  _IO_release_lock (_IO_stdin);
+  _IO_acquire_lock (stdin);
+  result = _IO_getwc_unlocked (stdin);
+  _IO_release_lock (stdin);
   return result;
 }
diff --git a/libio/getwchar_u.c b/libio/getwchar_u.c
index 1490bbe79f..23fa18b007 100644
--- a/libio/getwchar_u.c
+++ b/libio/getwchar_u.c
@@ -32,5 +32,5 @@ 
 wint_t
 getwchar_unlocked (void)
 {
-  return _IO_getwc_unlocked (_IO_stdin);
+  return _IO_getwc_unlocked (stdin);
 }
diff --git a/libio/iogets.c b/libio/iogets.c
index 2525056781..6691e4c0fc 100644
--- a/libio/iogets.c
+++ b/libio/iogets.c
@@ -34,8 +34,8 @@  _IO_gets (char *buf)
   int ch;
   char *retval;
 
-  _IO_acquire_lock (_IO_stdin);
-  ch = _IO_getc_unlocked (_IO_stdin);
+  _IO_acquire_lock (stdin);
+  ch = _IO_getc_unlocked (stdin);
   if (ch == EOF)
     {
       retval = NULL;
@@ -48,22 +48,22 @@  _IO_gets (char *buf)
       /* This is very tricky since a file descriptor may be in the
 	 non-blocking mode. The error flag doesn't mean much in this
 	 case. We return an error only when there is a new error. */
-      int old_error = _IO_stdin->_flags & _IO_ERR_SEEN;
-      _IO_stdin->_flags &= ~_IO_ERR_SEEN;
+      int old_error = stdin->_flags & _IO_ERR_SEEN;
+      stdin->_flags &= ~_IO_ERR_SEEN;
       buf[0] = (char) ch;
-      count = _IO_getline (_IO_stdin, buf + 1, INT_MAX, '\n', 0) + 1;
-      if (_IO_stdin->_flags & _IO_ERR_SEEN)
+      count = _IO_getline (stdin, buf + 1, INT_MAX, '\n', 0) + 1;
+      if (stdin->_flags & _IO_ERR_SEEN)
 	{
 	  retval = NULL;
 	  goto unlock_return;
 	}
       else
-	_IO_stdin->_flags |= old_error;
+	stdin->_flags |= old_error;
     }
   buf[count] = 0;
   retval = buf;
 unlock_return:
-  _IO_release_lock (_IO_stdin);
+  _IO_release_lock (stdin);
   return retval;
 }
 
diff --git a/libio/tst-bz24153.c b/libio/tst-bz24153.c
new file mode 100644
index 0000000000..85ad1415df
--- /dev/null
+++ b/libio/tst-bz24153.c
@@ -0,0 +1,114 @@ 
+/* Test that assigning to stdin redirects input (bug 24153).
+   Copyright (C) 2019 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/>.  */
+
+/* Prevent getchar -> getc inline expansion.  */
+#define __NO_INLINE__
+#pragma GCC optimize ("O0")
+
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/temp_file.h>
+#include <support/xstdio.h>
+#include <support/xunistd.h>
+#include <wchar.h>
+
+static int
+call_vscanf (const char *format, ...)
+{
+  va_list ap;
+  va_start (ap, format);
+  int ret = vscanf (format, ap);
+  va_end (ap);
+  return ret;
+}
+
+static int
+call_vwscanf (const wchar_t *format, ...)
+{
+  va_list ap;
+  va_start (ap, format);
+  int ret = vwscanf (format, ap);
+  va_end (ap);
+  return ret;
+}
+
+static void
+narrow (const char *path)
+{
+  FILE *old_stdin = stdin;
+  stdin = xfopen (path, "r");
+
+  TEST_COMPARE (getchar (), 'a');
+  TEST_COMPARE (getchar_unlocked (), 'b');
+  char ch = 1;
+  TEST_COMPARE (scanf ("%c", &ch), 1);
+  TEST_COMPARE (ch, 'c');
+  TEST_COMPARE (call_vscanf ("%c", &ch), 1);
+  TEST_COMPARE (ch, 'd');
+  char buf[8];
+  memset (buf, 'X', sizeof (buf));
+
+  /* Legacy interface.  */
+  extern char *gets (char *);
+  TEST_VERIFY (gets (buf) == buf);
+  TEST_COMPARE_BLOB (buf, sizeof (buf), "ef\0XXXXX", sizeof (buf));
+
+  fclose (stdin);
+  stdin = old_stdin;
+}
+
+static void
+wide (const char *path)
+{
+  FILE *old_stdin = stdin;
+  stdin = xfopen (path, "r");
+
+  TEST_COMPARE (getwchar (), L'a');
+  TEST_COMPARE (getwchar_unlocked (), L'b');
+  wchar_t ch = 1;
+  TEST_COMPARE (wscanf (L"%c", &ch), 1);
+  TEST_COMPARE (ch, L'c');
+  TEST_COMPARE (call_vwscanf (L"%c", &ch), 1);
+  TEST_COMPARE (ch, L'd');
+
+  fclose (stdin);
+  stdin = old_stdin;
+}
+
+static int
+do_test (void)
+{
+  char *path;
+  {
+    int fd = create_temp_file ("tst-bz24153-", &path);
+    TEST_VERIFY_EXIT (fd >= 0);
+    xwrite (fd, "abcdef", strlen ("abcdef"));
+    xclose (fd);
+  }
+
+  narrow (path);
+  wide (path);
+
+  free (path);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/libio/vscanf.c b/libio/vscanf.c
index 342dc3effc..526d486399 100644
--- a/libio/vscanf.c
+++ b/libio/vscanf.c
@@ -37,6 +37,6 @@ 
 int
 _IO_vscanf (const char *format, va_list args)
 {
-  return __vfscanf_internal (_IO_stdin, format, args, 0);
+  return __vfscanf_internal (stdin, format, args, 0);
 }
 ldbl_weak_alias (_IO_vscanf, vscanf)
diff --git a/libio/vwscanf.c b/libio/vwscanf.c
index 696e4c5c99..7b1903c71d 100644
--- a/libio/vwscanf.c
+++ b/libio/vwscanf.c
@@ -35,6 +35,6 @@ 
 int
 __vwscanf (const wchar_t *format, va_list args)
 {
-  return __vfwscanf_internal (_IO_stdin, format, args, 0);
+  return __vfwscanf_internal (stdin, format, args, 0);
 }
 ldbl_strong_alias (__vwscanf, vwscanf)