From patchwork Thu Jan 3 14:27:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Pluzhnikov X-Patchwork-Id: 30953 Received: (qmail 93340 invoked by alias); 3 Jan 2019 14:28:03 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 93322 invoked by uid 89); 3 Jan 2019 14:28:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-34.9 required=5.0 tests=BAYES_00, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS, USER_IN_DEF_SPF_WL autolearn=ham version=3.3.2 spammy= X-HELO: mail-ua1-f66.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=LBRN+siagLCWSC27r1PZyKX3ZxkjRQZ+DVN+sDgu8Ys=; b=JQvPsF/okFrCW4g0dLaCcek49XLTO6vDs/KGMbj1Im30qVtJlbB/UwpLoK3VIEbqT9 sJ+gixSHBloGPIwzhpKjgIhMV9ysIyuwj9hNORVcrw5/G3BlYmMQIRhFwlBcsTXCa+JQ pgncx8jV0jB6NwdwjyBusDKAJWIDq4PURfNjnIL4WhnGst0e1gAah18SNJ1T7AxXva+R 8BYr1eUkdQUBG2rpkTV/JsTSnVQ6wBveuincrUaJJvzxHCib+Ar9oVDKOOcrCkpc8Sml 1QK5ID91WhhIcOn55oiflnJc9iRenDiBviSjj20HkMWwJFEL0za0/I87XHAiMXXoV1Xt WihA== MIME-Version: 1.0 References: <87imz5vqgs.fsf@oldenburg2.str.redhat.com> <84e7e200-67c1-a51e-4cbe-fb99f57ed509@gotplt.org> In-Reply-To: <84e7e200-67c1-a51e-4cbe-fb99f57ed509@gotplt.org> From: Paul Pluzhnikov Date: Thu, 3 Jan 2019 06:27:27 -0800 Message-ID: Subject: Re: [PATCH] libio: use stdout in puts and putchar, etc [BZ #24051]. To: Siddhesh Poyarekar Cc: Florian Weimer , GLIBC Devel On Thu, Jan 3, 2019 at 5:17 AM Siddhesh Poyarekar wrote: > > On 03/01/19 6:38 PM, Florian Weimer wrote: > > You should use array_length (buf) here (and include ). > > fgetws does not expect a ybte byte count. Thanks for catching that. > Fine from the freeze perspective, but this needs a more descriptive > commit description. Revised patch attached. With respect to timing, I would prefer to postpone pushing this to master until after 2.29 is released, so that both stdout and stdin (which I'll fix next) are fixed together in 2.30. Alternatively, I can extend this patch to also cover stdin. Maybe that's actually better? Thanks! diff --git a/ChangeLog b/ChangeLog index c446abcd4e..ff85ddb4a5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2019-01-03 Paul Pluzhnikov + + [BZ #24051] + * libio/ioputs.c (_IO_puts): Use stdout instead of _IO_stdout. + * libio/fileops.c (_IO_new_file_underflow): Likewise + * libio/wfileops.c (_IO_wfile_underflow): Likewise + * libio/putchar.c (putchar): Likewise. + * libio/putchar_u.c (putchar_unlocked): Likewise. + * libio/putwchar.c (putchar): Likewise. + * libio/putwchar_u.c (putwchar_unlocked): Likewise. + * libio/tst-bz24051.c: New test. + * libio/Makefile (tests): Add tst-bz24051 + 2019-01-03 Adhemerval Zanella * sysdeps/unix/sysv/linux/Makefile (sysdep_headers): Add diff --git a/libio/Makefile b/libio/Makefile index 5bee83e55c..dbeba88fc0 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-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tests-internal = tst-vtables tst-vtables-interposed tst-readline diff --git a/libio/fileops.c b/libio/fileops.c index 43e33820e3..d2070a856e 100644 --- a/libio/fileops.c +++ b/libio/fileops.c @@ -501,13 +501,13 @@ _IO_new_file_underflow (FILE *fp) traditional Unix systems did this for stdout. stderr better not be line buffered. So we do just that here explicitly. --drepper */ - _IO_acquire_lock (_IO_stdout); + _IO_acquire_lock (stdout); - if ((_IO_stdout->_flags & (_IO_LINKED | _IO_NO_WRITES | _IO_LINE_BUF)) + if ((stdout->_flags & (_IO_LINKED | _IO_NO_WRITES | _IO_LINE_BUF)) == (_IO_LINKED | _IO_LINE_BUF)) - _IO_OVERFLOW (_IO_stdout, EOF); + _IO_OVERFLOW (stdout, EOF); - _IO_release_lock (_IO_stdout); + _IO_release_lock (stdout); } _IO_switch_to_get_mode (fp); diff --git a/libio/ioputs.c b/libio/ioputs.c index 04ae323c6d..319e551de5 100644 --- a/libio/ioputs.c +++ b/libio/ioputs.c @@ -33,15 +33,15 @@ _IO_puts (const char *str) { int result = EOF; size_t len = strlen (str); - _IO_acquire_lock (_IO_stdout); + _IO_acquire_lock (stdout); - if ((_IO_vtable_offset (_IO_stdout) != 0 - || _IO_fwide (_IO_stdout, -1) == -1) - && _IO_sputn (_IO_stdout, str, len) == len - && _IO_putc_unlocked ('\n', _IO_stdout) != EOF) + if ((_IO_vtable_offset (stdout) != 0 + || _IO_fwide (stdout, -1) == -1) + && _IO_sputn (stdout, str, len) == len + && _IO_putc_unlocked ('\n', stdout) != EOF) result = MIN (INT_MAX, len + 1); - _IO_release_lock (_IO_stdout); + _IO_release_lock (stdout); return result; } diff --git a/libio/putchar.c b/libio/putchar.c index 665f46685a..a3f4200cdb 100644 --- a/libio/putchar.c +++ b/libio/putchar.c @@ -24,9 +24,9 @@ int putchar (int c) { int result; - _IO_acquire_lock (_IO_stdout); - result = _IO_putc_unlocked (c, _IO_stdout); - _IO_release_lock (_IO_stdout); + _IO_acquire_lock (stdout); + result = _IO_putc_unlocked (c, stdout); + _IO_release_lock (stdout); return result; } diff --git a/libio/putchar_u.c b/libio/putchar_u.c index 37d03ad364..1eebf0fc8f 100644 --- a/libio/putchar_u.c +++ b/libio/putchar_u.c @@ -23,6 +23,6 @@ int putchar_unlocked (int c) { - CHECK_FILE (_IO_stdout, EOF); - return _IO_putc_unlocked (c, _IO_stdout); + CHECK_FILE (stdout, EOF); + return _IO_putc_unlocked (c, stdout); } diff --git a/libio/putwchar.c b/libio/putwchar.c index 8d6b6a4df0..1f5c4176f8 100644 --- a/libio/putwchar.c +++ b/libio/putwchar.c @@ -22,8 +22,8 @@ wint_t putwchar (wchar_t wc) { wint_t result; - _IO_acquire_lock (_IO_stdout); - result = _IO_putwc_unlocked (wc, _IO_stdout); - _IO_release_lock (_IO_stdout); + _IO_acquire_lock (stdout); + result = _IO_putwc_unlocked (wc, stdout); + _IO_release_lock (stdout); return result; } diff --git a/libio/putwchar_u.c b/libio/putwchar_u.c index cfb46fc253..d943220031 100644 --- a/libio/putwchar_u.c +++ b/libio/putwchar_u.c @@ -21,6 +21,6 @@ wint_t putwchar_unlocked (wchar_t wc) { - CHECK_FILE (_IO_stdout, WEOF); - return _IO_putwc_unlocked (wc, _IO_stdout); + CHECK_FILE (stdout, WEOF); + return _IO_putwc_unlocked (wc, stdout); } diff --git a/libio/tst-bz24051.c b/libio/tst-bz24051.c new file mode 100644 index 0000000000..cf0dc8024e --- /dev/null +++ b/libio/tst-bz24051.c @@ -0,0 +1,81 @@ +/* Test that assigning to stdout redirects puts, putchar, etc (BZ#24051) + 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 + . */ + + +/* Prevent putchar -> _IO_putc inline expansion. */ +#define __NO_INLINE__ +#pragma GCC optimize("O0") + +#include +#include +#include + +#include +#include +#include +#include + +#undef putchar +#undef putwchar + +static int +do_test_narrow (void) +{ + char buf[100]; + int fd = create_temp_file ("tst-bz24051", NULL); + stdout = fdopen (fd, "w+"); + TEST_VERIFY_EXIT (stdout != NULL); + + printf ("ab%s", "cd"); + putchar ('e'); + putchar_unlocked ('f'); + puts ("ghi"); + + rewind (stdout); + TEST_VERIFY_EXIT (fgets (buf, sizeof (buf), stdout) != NULL); + TEST_VERIFY (strcmp (buf, "abcdefghi\n") == 0); + + return 0; +} + +static int +do_test_wide (void) +{ + wchar_t buf[100]; + int fd = create_temp_file ("tst-bz24051w", NULL); + stdout = fdopen (fd, "w+"); + TEST_VERIFY_EXIT (stdout != NULL); + + wprintf (L"ab%ls", L"cd"); + putwchar (L'e'); + putwchar_unlocked (L'f'); + + rewind (stdout); + TEST_VERIFY_EXIT (fgetws (buf, array_length (buf), stdout) != NULL); + TEST_VERIFY (wcscmp (buf, L"abcdef") == 0); + + return 0; +} + +static int +do_test (void) +{ + return do_test_narrow () + do_test_wide (); +} + +#include diff --git a/libio/wfileops.c b/libio/wfileops.c index 78f20486e5..0367643703 100644 --- a/libio/wfileops.c +++ b/libio/wfileops.c @@ -208,13 +208,13 @@ _IO_wfile_underflow (FILE *fp) traditional Unix systems did this for stdout. stderr better not be line buffered. So we do just that here explicitly. --drepper */ - _IO_acquire_lock (_IO_stdout); + _IO_acquire_lock (stdout); - if ((_IO_stdout->_flags & (_IO_LINKED | _IO_NO_WRITES | _IO_LINE_BUF)) + if ((stdout->_flags & (_IO_LINKED | _IO_NO_WRITES | _IO_LINE_BUF)) == (_IO_LINKED | _IO_LINE_BUF)) - _IO_OVERFLOW (_IO_stdout, EOF); + _IO_OVERFLOW (stdout, EOF); - _IO_release_lock (_IO_stdout); + _IO_release_lock (stdout); } _IO_switch_to_get_mode (fp);