[PATCH v3] libio: Ensure output buffer for wchars (bug 28828)

[PATCH v3] libio: Ensure output buffer for wchars (bug 28828)

[jobol]

From: "jobol@nonadev.net" <jobol@nonadev.net>

When fileops.c checks for nullity of the write pointer,
in order to ensure its allocation, before that patch,
wfileops didn't. This was leading to crashes on some cases,
as described by bug 28828.

The minimal sequence to produce the crash was:

    #include <stdio.h>
    #include <wchar.h>
    int main(int ac, char **av)
    {
            setvbuf(stdout, NULL, _IOLBF, 0);
            fgetwc(stdin);
            fputwc(10, stdout); /*CRASH HERE!*/
            return 0;
    }

The line "fgetwc(stdin);" is necessary. It introduces the
bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
indirectly (file wfileops.c, function _IO_wfile_underflow, line 213).

Signed-off-by: Jose Bollo <jobol@nonadev.net>
---
 libio/Makefile          |  2 +-
 libio/tst-bz28828.c     | 10 ++++++++++
 libio/tst-bz28828.input |  1 +
 libio/wfileops.c        |  3 ++-
 4 files changed, 14 insertions(+), 2 deletions(-)
 create mode 100644 libio/tst-bz28828.c
 create mode 100644 libio/tst-bz28828.input

diff --git a/libio/Makefile b/libio/Makefile
index 0e5f348bea..e97387743f 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -66,7 +66,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
 	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-bz24153 \
-	tst-wfile-sync
+	tst-wfile-sync tst-bz28828
 
 tests-internal = tst-vtables tst-vtables-interposed
 
diff --git a/libio/tst-bz28828.c b/libio/tst-bz28828.c
new file mode 100644
index 0000000000..f5849d2ca6
--- /dev/null
+++ b/libio/tst-bz28828.c
@@ -0,0 +1,10 @@
+#include <stdio.h>
+#include <wchar.h>
+int main(int ac, char **av)
+{
+	setvbuf(stdout, NULL, _IOLBF, 0);
+	fgetwc(stdin);
+	fputwc(10, stdout); /*SHOUDN'T CRASH HERE!*/
+	return 0;
+}
+
diff --git a/libio/tst-bz28828.input b/libio/tst-bz28828.input
new file mode 100644
index 0000000000..ce01362503
--- /dev/null
+++ b/libio/tst-bz28828.input
@@ -0,0 +1 @@
+hello
diff --git a/libio/wfileops.c b/libio/wfileops.c
index fb9d45b677..b59a98881f 100644
--- a/libio/wfileops.c
+++ b/libio/wfileops.c
@@ -412,7 +412,8 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
       return WEOF;
     }
   /* If currently reading or no buffer allocated. */
-  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
+  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
+      || f->_wide_data->_IO_write_base == NULL)
     {
       /* Allocate a buffer if needed. */
       if (f->_wide_data->_IO_write_base == 0)
-- 
2.34.1

Re: [PATCH v3] libio: Ensure output buffer for wchars (bug 28828)

[Adhemerval Zanella]

On 22/02/2022 17:55, jobol@nonadev.net wrote:
> From: "jobol@nonadev.net" <jobol@nonadev.net>
> 
> When fileops.c checks for nullity of the write pointer,
> in order to ensure its allocation, before that patch,
> wfileops didn't. This was leading to crashes on some cases,
> as described by bug 28828.
> 
> The minimal sequence to produce the crash was:
> 
>     #include <stdio.h>
>     #include <wchar.h>
>     int main(int ac, char **av)
>     {
>             setvbuf(stdout, NULL, _IOLBF, 0);
>             fgetwc(stdin);
>             fputwc(10, stdout); /*CRASH HERE!*/
>             return 0;
>     }
> 
> The line "fgetwc(stdin);" is necessary. It introduces the
> bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
> indirectly (file wfileops.c, function _IO_wfile_underflow, line 213).
> 
> Signed-off-by: Jose Bollo <jobol@nonadev.net>
> ---
>  libio/Makefile          |  2 +-
>  libio/tst-bz28828.c     | 10 ++++++++++
>  libio/tst-bz28828.input |  1 +
>  libio/wfileops.c        |  3 ++-
>  4 files changed, 14 insertions(+), 2 deletions(-)
>  create mode 100644 libio/tst-bz28828.c
>  create mode 100644 libio/tst-bz28828.input
> 
> diff --git a/libio/Makefile b/libio/Makefile
> index 0e5f348bea..e97387743f 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -66,7 +66,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
>  	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-bz24153 \
> -	tst-wfile-sync
> +	tst-wfile-sync tst-bz28828
>  
>  tests-internal = tst-vtables tst-vtables-interposed
>  
> diff --git a/libio/tst-bz28828.c b/libio/tst-bz28828.c
> new file mode 100644
> index 0000000000..f5849d2ca6
> --- /dev/null
> +++ b/libio/tst-bz28828.c
> @@ -0,0 +1,10 @@
> +#include <stdio.h>
> +#include <wchar.h>
> +int main(int ac, char **av)
> +{
> +	setvbuf(stdout, NULL, _IOLBF, 0);
> +	fgetwc(stdin);
> +	fputwc(10, stdout); /*SHOUDN'T CRASH HERE!*/
> +	return 0;
> +}
> +

The tests requires a proper Copyright header and to use libsupport (so it handles
test timeout, segfault, or any other error in test itself).  Also, use the proper
indentation.  A skeleton will be something like below:

---
/* Unit test for BZ#28828.
   Copyright (C) 2022 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
   <https://www.gnu.org/licenses/>.  */

#include <support/xstdio.h>
#include <support/check.h>
#include <wchar.h>

static int
do_test (void)
{ 
  /* Add the test here.  */

  return 0;
}

#include <support/test-driver.c>
---

Keep in mind that using fgetwc without a method to actually input some data
will make the test timeout and fail.


> diff --git a/libio/tst-bz28828.input b/libio/tst-bz28828.input
> new file mode 100644
> index 0000000000..ce01362503
> --- /dev/null
> +++ b/libio/tst-bz28828.input
> @@ -0,0 +1 @@
> +hello
> diff --git a/libio/wfileops.c b/libio/wfileops.c
> index fb9d45b677..b59a98881f 100644
> --- a/libio/wfileops.c
> +++ b/libio/wfileops.c
> @@ -412,7 +412,8 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
>        return WEOF;
>      }
>    /* If currently reading or no buffer allocated. */
> -  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
> +  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
> +      || f->_wide_data->_IO_write_base == NULL)
>      {
>        /* Allocate a buffer if needed. */
>        if (f->_wide_data->_IO_write_base == 0)

Based on your test I was intrigued on why you need a fgetwc(stdio) to actually
trigger the issue.  The fgetwc(stdio) in fact changed the stdout internal flags
somewhat, which then triggered the issue.

Digging a little bit deeper it seems this snippet is triggering it:

libio/wfileops.c

 201   /* FIXME This can/should be moved to genops ?? */                                     
 202   if (fp->_flags & (_IO_LINE_BUF | _IO_UNBUFFERED))                                     
 203     {
 204       /* We used to flush all line-buffered stream.  This really isn't                  
 205          required by any standard.  My recollection is that                             
 206          traditional Unix systems did this for stdout.  stderr better
 207          not be line buffered.  So we do just that here                                 
 208          explicitly.  --drepper */                                                      
 209       _IO_acquire_lock (stdout);                                                        
 210  
 211       if ((stdout->_flags & (_IO_LINKED | _IO_NO_WRITES | _IO_LINE_BUF))                
 212           == (_IO_LINKED | _IO_LINE_BUF))
 213         _IO_OVERFLOW (stdout, EOF);                                                     
 214  
 215       _IO_release_lock (stdout);                                                        
 216     }

And the FIXME really makes me wonder if this is really ok, the libio/fileops.c
does not contain such snippet.  Removing this code does not trigger any regression
and also fixes the issue.

Re: [PATCH v3] libio: Ensure output buffer for wchars (bug 28828)

[Andreas Schwab]

On Mär 04 2022, Adhemerval Zanella via Libc-alpha wrote:

> the libio/fileops.c does not contain such snippet.

See line 488.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH v3] libio: Ensure output buffer for wchars (bug 28828)

[Adhemerval Zanella]

On 04/03/2022 16:53, Andreas Schwab wrote:
> On Mär 04 2022, Adhemerval Zanella via Libc-alpha wrote:
> 
>> the libio/fileops.c does not contain such snippet.
> 
> See line 488.
> 

Should we evaluate to remove it as well then?

Re: [PATCH v3] libio: Ensure output buffer for wchars (bug 28828)

[Andreas Schwab]

On Mär 07 2022, Adhemerval Zanella wrote:

> On 04/03/2022 16:53, Andreas Schwab wrote:
>> On Mär 04 2022, Adhemerval Zanella via Libc-alpha wrote:
>> 
>>> the libio/fileops.c does not contain such snippet.
>> 
>> See line 488.
>> 
>
> Should we evaluate to remove it as well then?

Why would we?  It's specified by POSIX.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH v3] libio: Ensure output buffer for wchars (bug 28828)

[Adhemerval Zanella]

On 07/03/2022 08:29, Andreas Schwab wrote:
> On Mär 07 2022, Adhemerval Zanella wrote:
> 
>> On 04/03/2022 16:53, Andreas Schwab wrote:
>>> On Mär 04 2022, Adhemerval Zanella via Libc-alpha wrote:
>>>
>>>> the libio/fileops.c does not contain such snippet.
>>>
>>> See line 488.
>>>
>>
>> Should we evaluate to remove it as well then?
> 
> Why would we?  It's specified by POSIX.
> 

That's no what comment specifies ("This really isn't required by any
standard").  Do you have any link that describes this behavior?

Re: [PATCH v3] libio: Ensure output buffer for wchars (bug 28828)

[Andreas Schwab]

On Mär 07 2022, Adhemerval Zanella wrote:

> That's no what comment specifies ("This really isn't required by any
> standard").

This refers to the preceding sentence.

> Do you have any link that describes this behavior?

https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_05

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH v3] libio: Ensure output buffer for wchars (bug 28828)

[José Bollo]

On Fri, 4 Mar 2022 16:39:36 -0300
Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

Hi Adhemerval, Hi Andreas, Hi all,

I'm going to submit a new version taking your skeleton as a basis.

I added the file named libio/tst-bz28828.input with the understanding
that given its name, the test framework will automatically use it as
input. Can you tell me if it is correct or if I have to add something
some where.

My though about the interaction between stdin and stdout is that it is
sometime needed to get an input to flush the output.

Best regards
José

> On 22/02/2022 17:55, jobol@nonadev.net wrote:
> > From: "jobol@nonadev.net" <jobol@nonadev.net>
> > 
> > When fileops.c checks for nullity of the write pointer,
> > in order to ensure its allocation, before that patch,
> > wfileops didn't. This was leading to crashes on some cases,
> > as described by bug 28828.
> > 
> > The minimal sequence to produce the crash was:
> > 
> >     #include <stdio.h>
> >     #include <wchar.h>
> >     int main(int ac, char **av)
> >     {
> >             setvbuf(stdout, NULL, _IOLBF, 0);
> >             fgetwc(stdin);
> >             fputwc(10, stdout); /*CRASH HERE!*/
> >             return 0;
> >     }
> > 
> > The line "fgetwc(stdin);" is necessary. It introduces the
> > bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
> > indirectly (file wfileops.c, function _IO_wfile_underflow, line
> > 213).
> > 
> > Signed-off-by: Jose Bollo <jobol@nonadev.net>
> > ---
> >  libio/Makefile          |  2 +-
> >  libio/tst-bz28828.c     | 10 ++++++++++
> >  libio/tst-bz28828.input |  1 +
> >  libio/wfileops.c        |  3 ++-
> >  4 files changed, 14 insertions(+), 2 deletions(-)
> >  create mode 100644 libio/tst-bz28828.c
> >  create mode 100644 libio/tst-bz28828.input
> > 
> > diff --git a/libio/Makefile b/libio/Makefile
> > index 0e5f348bea..e97387743f 100644
> > --- a/libio/Makefile
> > +++ b/libio/Makefile
> > @@ -66,7 +66,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf
> > tst_wscanf tst_getwc tst_putwc   \ 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-bz24153 \
> > -	tst-wfile-sync
> > +	tst-wfile-sync tst-bz28828
> >  
> >  tests-internal = tst-vtables tst-vtables-interposed
> >  
> > diff --git a/libio/tst-bz28828.c b/libio/tst-bz28828.c
> > new file mode 100644
> > index 0000000000..f5849d2ca6
> > --- /dev/null
> > +++ b/libio/tst-bz28828.c
> > @@ -0,0 +1,10 @@
> > +#include <stdio.h>
> > +#include <wchar.h>
> > +int main(int ac, char **av)
> > +{
> > +	setvbuf(stdout, NULL, _IOLBF, 0);
> > +	fgetwc(stdin);
> > +	fputwc(10, stdout); /*SHOUDN'T CRASH HERE!*/
> > +	return 0;
> > +}
> > +  
> 
> The tests requires a proper Copyright header and to use libsupport
> (so it handles test timeout, segfault, or any other error in test
> itself).  Also, use the proper indentation.  A skeleton will be
> something like below:
> 
> ---
> /* Unit test for BZ#28828.
>    Copyright (C) 2022 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
>    <https://www.gnu.org/licenses/>.  */
> 
> #include <support/xstdio.h>
> #include <support/check.h>
> #include <wchar.h>
> 
> static int
> do_test (void)
> { 
>   /* Add the test here.  */
> 
>   return 0;
> }
> 
> #include <support/test-driver.c>
> ---
> 
> Keep in mind that using fgetwc without a method to actually input
> some data will make the test timeout and fail.
> 
> 
> > diff --git a/libio/tst-bz28828.input b/libio/tst-bz28828.input
> > new file mode 100644
> > index 0000000000..ce01362503
> > --- /dev/null
> > +++ b/libio/tst-bz28828.input
> > @@ -0,0 +1 @@
> > +hello
> > diff --git a/libio/wfileops.c b/libio/wfileops.c
> > index fb9d45b677..b59a98881f 100644
> > --- a/libio/wfileops.c
> > +++ b/libio/wfileops.c
> > @@ -412,7 +412,8 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
> >        return WEOF;
> >      }
> >    /* If currently reading or no buffer allocated. */
> > -  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
> > +  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
> > +      || f->_wide_data->_IO_write_base == NULL)
> >      {
> >        /* Allocate a buffer if needed. */
> >        if (f->_wide_data->_IO_write_base == 0)  
> 
> Based on your test I was intrigued on why you need a fgetwc(stdio) to
> actually trigger the issue.  The fgetwc(stdio) in fact changed the
> stdout internal flags somewhat, which then triggered the issue.
> 
> Digging a little bit deeper it seems this snippet is triggering it:
> 
> libio/wfileops.c
> 
>  201   /* FIXME This can/should be moved to genops ?? */
> 202   if (fp->_flags & (_IO_LINE_BUF | _IO_UNBUFFERED))
> 203     {
>  204       /* We used to flush all line-buffered stream.  This really
> isn't 205          required by any standard.  My recollection is that
> 206          traditional Unix systems did this for stdout.  stderr
> better 207          not be line buffered.  So we do just that here
> 208          explicitly.  --drepper */
> 209       _IO_acquire_lock (stdout);
> 210  
>  211       if ((stdout->_flags & (_IO_LINKED | _IO_NO_WRITES |
> _IO_LINE_BUF)) 212           == (_IO_LINKED | _IO_LINE_BUF))
>  213         _IO_OVERFLOW (stdout, EOF);
> 214  
>  215       _IO_release_lock (stdout);
> 216     }
> 
> And the FIXME really makes me wonder if this is really ok, the
> libio/fileops.c does not contain such snippet.  Removing this code
> does not trigger any regression and also fixes the issue.

[PATCH] libio: Ensure output buffer for wchars (bug 28828)

[jobol]

From: José Bollo <jobol@nonadev.net>

When fileops.c checks for nullity of the write pointer,
in order to ensure its allocation, before that patch,
wfileops didn't. This was leading to crashes on some cases,
as described by bug 28828.

The minimal sequence to produce the crash was:

    #include <stdio.h>
    #include <wchar.h>
    int main(int ac, char **av)
    {
            setvbuf(stdout, NULL, _IOLBF, 0);
            fgetwc(stdin);
            fputwc(10, stdout); /*CRASH HERE!*/
            return 0;
    }

The line "fgetwc(stdin);" is necessary. It introduces the
bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
indirectly (file wfileops.c, function _IO_wfile_underflow, line 213).

Signed-off-by: Jose Bollo <jobol@nonadev.net>
---
 libio/Makefile          |  2 +-
 libio/tst-bz28828.c     | 33 +++++++++++++++++++++++++++++++++
 libio/tst-bz28828.input |  1 +
 libio/wfileops.c        |  3 ++-
 4 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 libio/tst-bz28828.c
 create mode 100644 libio/tst-bz28828.input

diff --git a/libio/Makefile b/libio/Makefile
index 0e5f348bea..e97387743f 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -66,7 +66,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
 	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-bz24153 \
-	tst-wfile-sync
+	tst-wfile-sync tst-bz28828
 
 tests-internal = tst-vtables tst-vtables-interposed
 
diff --git a/libio/tst-bz28828.c b/libio/tst-bz28828.c
new file mode 100644
index 0000000000..22a70dd954
--- /dev/null
+++ b/libio/tst-bz28828.c
@@ -0,0 +1,33 @@
+/* Unit test for BZ#28828.
+   Copyright (C) 2022 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/xstdio.h>
+#include <support/check.h>
+#include <wchar.h>
+
+static int
+do_test (void)
+{ 
+  setvbuf(stdout, NULL, _IOLBF, 0);
+  fgetwc(stdin);
+  fputwc(10, stdout); /*SHOUDN'T CRASH HERE!*/
+  return 0;
+}
+
+#include <support/test-driver.c>
+
diff --git a/libio/tst-bz28828.input b/libio/tst-bz28828.input
new file mode 100644
index 0000000000..ce01362503
--- /dev/null
+++ b/libio/tst-bz28828.input
@@ -0,0 +1 @@
+hello
diff --git a/libio/wfileops.c b/libio/wfileops.c
index fb9d45b677..b59a98881f 100644
--- a/libio/wfileops.c
+++ b/libio/wfileops.c
@@ -412,7 +412,8 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
       return WEOF;
     }
   /* If currently reading or no buffer allocated. */
-  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
+  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
+      || f->_wide_data->_IO_write_base == NULL)
     {
       /* Allocate a buffer if needed. */
       if (f->_wide_data->_IO_write_base == 0)
-- 
2.34.1

[PATCH v4] libio: Ensure output buffer for wchars (bug 28828)

[jobol]

From: José Bollo <jobol@nonadev.net>

When fileops.c checks for nullity of the write pointer,
in order to ensure its allocation, before that patch,
wfileops didn't. This was leading to crashes on some cases,
as described by bug 28828.

The minimal sequence to produce the crash was:

    #include <stdio.h>
    #include <wchar.h>
    int main(int ac, char **av)
    {
            setvbuf(stdout, NULL, _IOLBF, 0);
            fgetwc(stdin);
            fputwc(10, stdout); /*CRASH HERE!*/
            return 0;
    }

The line "fgetwc(stdin);" is necessary. It introduces the
bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
indirectly (file wfileops.c, function _IO_wfile_underflow, line 213).

Signed-off-by: Jose Bollo <jobol@nonadev.net>
---
 libio/Makefile          |  2 +-
 libio/tst-bz28828.c     | 33 +++++++++++++++++++++++++++++++++
 libio/tst-bz28828.input |  1 +
 libio/wfileops.c        |  3 ++-
 4 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 libio/tst-bz28828.c
 create mode 100644 libio/tst-bz28828.input

diff --git a/libio/Makefile b/libio/Makefile
index 0e5f348bea..e97387743f 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -66,7 +66,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
 	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-bz24153 \
-	tst-wfile-sync
+	tst-wfile-sync tst-bz28828
 
 tests-internal = tst-vtables tst-vtables-interposed
 
diff --git a/libio/tst-bz28828.c b/libio/tst-bz28828.c
new file mode 100644
index 0000000000..22a70dd954
--- /dev/null
+++ b/libio/tst-bz28828.c
@@ -0,0 +1,33 @@
+/* Unit test for BZ#28828.
+   Copyright (C) 2022 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/xstdio.h>
+#include <support/check.h>
+#include <wchar.h>
+
+static int
+do_test (void)
+{ 
+  setvbuf(stdout, NULL, _IOLBF, 0);
+  fgetwc(stdin);
+  fputwc(10, stdout); /*SHOUDN'T CRASH HERE!*/
+  return 0;
+}
+
+#include <support/test-driver.c>
+
diff --git a/libio/tst-bz28828.input b/libio/tst-bz28828.input
new file mode 100644
index 0000000000..ce01362503
--- /dev/null
+++ b/libio/tst-bz28828.input
@@ -0,0 +1 @@
+hello
diff --git a/libio/wfileops.c b/libio/wfileops.c
index fb9d45b677..b59a98881f 100644
--- a/libio/wfileops.c
+++ b/libio/wfileops.c
@@ -412,7 +412,8 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
       return WEOF;
     }
   /* If currently reading or no buffer allocated. */
-  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
+  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
+      || f->_wide_data->_IO_write_base == NULL)
     {
       /* Allocate a buffer if needed. */
       if (f->_wide_data->_IO_write_base == 0)
-- 
2.34.1

Re: [PATCH v4] libio: Ensure output buffer for wchars (bug 28828)

[Adhemerval Zanella]

On 08/03/2022 05:58, jobol@nonadev.net wrote:
> From: José Bollo <jobol@nonadev.net>
> 
> When fileops.c checks for nullity of the write pointer,
> in order to ensure its allocation, before that patch,
> wfileops didn't. This was leading to crashes on some cases,
> as described by bug 28828.
> 
> The minimal sequence to produce the crash was:
> 
>     #include <stdio.h>
>     #include <wchar.h>
>     int main(int ac, char **av)
>     {
>             setvbuf(stdout, NULL, _IOLBF, 0);
>             fgetwc(stdin);
>             fputwc(10, stdout); /*CRASH HERE!*/
>             return 0;
>     }
> 
> The line "fgetwc(stdin);" is necessary. It introduces the
> bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
> indirectly (file wfileops.c, function _IO_wfile_underflow, line 213).
> 
> Signed-off-by: Jose Bollo <jobol@nonadev.net>

This patch looks ok, I will fix the style issues below and install it.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  libio/Makefile          |  2 +-
>  libio/tst-bz28828.c     | 33 +++++++++++++++++++++++++++++++++
>  libio/tst-bz28828.input |  1 +
>  libio/wfileops.c        |  3 ++-
>  4 files changed, 37 insertions(+), 2 deletions(-)
>  create mode 100644 libio/tst-bz28828.c
>  create mode 100644 libio/tst-bz28828.input
> 
> diff --git a/libio/Makefile b/libio/Makefile
> index 0e5f348bea..e97387743f 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -66,7 +66,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
>  	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-bz24153 \
> -	tst-wfile-sync
> +	tst-wfile-sync tst-bz28828
>  
>  tests-internal = tst-vtables tst-vtables-interposed
>  
> diff --git a/libio/tst-bz28828.c b/libio/tst-bz28828.c
> new file mode 100644
> index 0000000000..22a70dd954
> --- /dev/null
> +++ b/libio/tst-bz28828.c
> @@ -0,0 +1,33 @@
> +/* Unit test for BZ#28828.
> +   Copyright (C) 2022 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/xstdio.h>
> +#include <support/check.h>
> +#include <wchar.h>
> +
> +static int
> +do_test (void)
> +{ 

Extra space after '{'.

> +  setvbuf(stdout, NULL, _IOLBF, 0);

Space after before '(', also for the rest on function calls.

> +  fgetwc(stdin);
> +  fputwc(10, stdout); /*SHOUDN'T CRASH HERE!*/

We usually do not add comments in upper cases, I will replace with.

  /* It should not crash here.  */

> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> +

Extra newline.

> diff --git a/libio/tst-bz28828.input b/libio/tst-bz28828.input
> new file mode 100644
> index 0000000000..ce01362503
> --- /dev/null
> +++ b/libio/tst-bz28828.input
> @@ -0,0 +1 @@
> +hello
> diff --git a/libio/wfileops.c b/libio/wfileops.c
> index fb9d45b677..b59a98881f 100644
> --- a/libio/wfileops.c
> +++ b/libio/wfileops.c
> @@ -412,7 +412,8 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
>        return WEOF;
>      }
>    /* If currently reading or no buffer allocated. */
> -  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
> +  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
> +      || f->_wide_data->_IO_write_base == NULL)
>      {
>        /* Allocate a buffer if needed. */
>        if (f->_wide_data->_IO_write_base == 0)

Ok.

Re: [PATCH v4] libio: Ensure output buffer for wchars (bug 28828)

[José Bollo]

On Tue, 8 Mar 2022 14:12:48 -0300
Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:


> ... I will fix the style issues below and install it.

thank you for fixing it -_-

[PATCH] libio: Ensure output buffer for wchars (bug 28828)

[José Bollo]

From: "jobol@nonadev.net" <jobol@nonadev.net>

When fileops.c checks for nullity of the write pointer,
in order to ensure its allocation, before that patch,
wfileops didn't. This was leading to crashes on some cases,
as described by bug 28828.

The minimal sequence to produce the crash was:

    #include <stdio.h>
    #include <wchar.h>
    int main(int ac, char **av)
    {
            setvbuf(stdout, NULL, _IOLBF, 0);
            fgetwc(stdin);
            fputwc(10, stdout); /*CRASH HERE!*/
            return 0;
    }

The line "fgetwc(stdin);" is necessary. It introduces the
bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
indirectly (file wfileops.c, function _IO_wfile_underflow, line 213).

Signed-off-by: Jose Bollo <jobol@nonadev.net>
---
 libio/Makefile      |  2 +-
 libio/tst-bz28828.c | 10 ++++++++++
 libio/wfileops.c    |  5 +++--
 3 files changed, 14 insertions(+), 3 deletions(-)
 create mode 100644 libio/tst-bz28828.c

diff --git a/libio/Makefile b/libio/Makefile
index 0e5f348bea..e97387743f 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -66,7 +66,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
 	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-bz24153 \
-	tst-wfile-sync
+	tst-wfile-sync tst-bz28828
 
 tests-internal = tst-vtables tst-vtables-interposed
 
diff --git a/libio/tst-bz28828.c b/libio/tst-bz28828.c
new file mode 100644
index 0000000000..f5849d2ca6
--- /dev/null
+++ b/libio/tst-bz28828.c
@@ -0,0 +1,10 @@
+#include <stdio.h>
+#include <wchar.h>
+int main(int ac, char **av)
+{
+	setvbuf(stdout, NULL, _IOLBF, 0);
+	fgetwc(stdin);
+	fputwc(10, stdout); /*SHOUDN'T CRASH HERE!*/
+	return 0;
+}
+
diff --git a/libio/wfileops.c b/libio/wfileops.c
index fb9d45b677..aa94a22983 100644
--- a/libio/wfileops.c
+++ b/libio/wfileops.c
@@ -412,10 +412,11 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
       return WEOF;
     }
   /* If currently reading or no buffer allocated. */
-  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
+  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
+      || f->_wide_data->_IO_write_base == NULL)
     {
       /* Allocate a buffer if needed. */
-      if (f->_wide_data->_IO_write_base == 0)
+      if (f->_wide_data->_IO_write_base == NULL)
 	{
 	  _IO_wdoallocbuf (f);
 	  _IO_free_wbackup_area (f);
-- 
2.34.1

Re: [PATCH] libio: Ensure output buffer for wchars (bug 28828)

[Adhemerval Zanella]

On 14/02/2022 17:23, José Bollo wrote:
> From: "jobol@nonadev.net" <jobol@nonadev.net>
> 
> When fileops.c checks for nullity of the write pointer,
> in order to ensure its allocation, before that patch,
> wfileops didn't. This was leading to crashes on some cases,
> as described by bug 28828.
> 
> The minimal sequence to produce the crash was:
> 
>     #include <stdio.h>
>     #include <wchar.h>
>     int main(int ac, char **av)
>     {
>             setvbuf(stdout, NULL, _IOLBF, 0);
>             fgetwc(stdin);
>             fputwc(10, stdout); /*CRASH HERE!*/
>             return 0;
>     }
> 
> The line "fgetwc(stdin);" is necessary. It introduces the
> bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
> indirectly (file wfileops.c, function _IO_wfile_underflow, line 213).
> 
> Signed-off-by: Jose Bollo <jobol@nonadev.net>
> ---
>  libio/Makefile      |  2 +-
>  libio/tst-bz28828.c | 10 ++++++++++
>  libio/wfileops.c    |  5 +++--
>  3 files changed, 14 insertions(+), 3 deletions(-)
>  create mode 100644 libio/tst-bz28828.c
> 
> diff --git a/libio/Makefile b/libio/Makefile
> index 0e5f348bea..e97387743f 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -66,7 +66,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
>  	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-bz24153 \
> -	tst-wfile-sync
> +	tst-wfile-sync tst-bz28828
>  
>  tests-internal = tst-vtables tst-vtables-interposed
>  
> diff --git a/libio/tst-bz28828.c b/libio/tst-bz28828.c
> new file mode 100644
> index 0000000000..f5849d2ca6
> --- /dev/null
> +++ b/libio/tst-bz28828.c
> @@ -0,0 +1,10 @@
> +#include <stdio.h>
> +#include <wchar.h>
> +int main(int ac, char **av)
> +{
> +	setvbuf(stdout, NULL, _IOLBF, 0);
> +	fgetwc(stdin);
> +	fputwc(10, stdout); /*SHOUDN'T CRASH HERE!*/
> +	return 0;
> +}
> +

Sorry but this test is unacceptable, it blocks the make check indefinitely
(triggering a failure) and does not use libsupport.  This fix requires a
proper testcase, there multiple ways to do, so please check an already
test on libio.  

> diff --git a/libio/wfileops.c b/libio/wfileops.c
> index fb9d45b677..aa94a22983 100644
> --- a/libio/wfileops.c
> +++ b/libio/wfileops.c
> @@ -412,10 +412,11 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
>        return WEOF;
>      }
>    /* If currently reading or no buffer allocated. */
> -  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
> +  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
> +      || f->_wide_data->_IO_write_base == NULL)
>      {

It seems ok, the _IOFBF buffer type has a specific case for buf being
null where it ended up calling _IO_DOALLOCATE (which _IOLBF does not).

>        /* Allocate a buffer if needed. */
> -      if (f->_wide_data->_IO_write_base == 0)
> +      if (f->_wide_data->_IO_write_base == NULL)

I would prefer to avoid make style change on a bug fix. 

>  	{
>  	  _IO_wdoallocbuf (f);
>  	  _IO_free_wbackup_area (f);

[PATCH] libio: Ensure output buffer for wchars (bug 28828)

[jobol]

When fileops.c checks for nullity of the write pointer,
in order to ensure its allocation, before that patch,
wfileops didn't. This was leading to crashes on some cases,
as described by bug 28828.

The minimal sequence to produce the crash was:

     #include <stdio.h>
     #include <wchar.h>
     int main(int ac, char **av)
     {
             setvbuf(stdout, NULL, _IOLBF, 0);
             fgetwc(stdin);
             fputwc(10, stdout); /*CRASH HERE!*/
             return 0;
     }

The line "fgetwc(stdin);" is necessary. It introduces the
bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
indirectly (file wfileops.c, function _IO_wfile_underflow, line 213).

Signed-off-by: Jose Bollo <jobol@nonadev.net>
---
  libio/wfileops.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libio/wfileops.c b/libio/wfileops.c
index fb9d45b677..aa94a22983 100644
--- a/libio/wfileops.c
+++ b/libio/wfileops.c
@@ -412,10 +412,11 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
        return WEOF;
      }
    /* If currently reading or no buffer allocated. */
-  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
+  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
+      || f->_wide_data->_IO_write_base == NULL)
      {
        /* Allocate a buffer if needed. */
-      if (f->_wide_data->_IO_write_base == 0)
+      if (f->_wide_data->_IO_write_base == NULL)
  	{
  	  _IO_wdoallocbuf (f);
  	  _IO_free_wbackup_area (f);
-- 
2.34.1

Re: [PATCH] libio: Ensure output buffer for wchars (bug 28828)

[Adhemerval Zanella]

On 13/02/2022 16:52, jobol@nonadev.net wrote:
> When fileops.c checks for nullity of the write pointer,
> in order to ensure its allocation, before that patch,
> wfileops didn't. This was leading to crashes on some cases,
> as described by bug 28828.
> 
> The minimal sequence to produce the crash was:
> 
>     #include <stdio.h>
>     #include <wchar.h>
>     int main(int ac, char **av)
>     {
>             setvbuf(stdout, NULL, _IOLBF, 0);
>             fgetwc(stdin);
>             fputwc(10, stdout); /*CRASH HERE!*/
>             return 0;
>     }
> 
> The line "fgetwc(stdin);" is necessary. It introduces the
> bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
> indirectly (file wfileops.c, function _IO_wfile_underflow, line 213).
> 
> Signed-off-by: Jose Bollo <jobol@nonadev.net>

Patch looks ok, but git-pw fails to apply it:

$ git-pw patch apply 51088
Applying: libio: Ensure output buffer for wchars (bug 28828)
error: patch failed: libio/wfileops.c:412
error: libio/wfileops.c: patch does not apply
error: Did you hand edit your patch?

Could you resend it? 

Also, could you also provide a regression testcase for this fix?

> ---
>  libio/wfileops.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libio/wfileops.c b/libio/wfileops.c
> index fb9d45b677..aa94a22983 100644
> --- a/libio/wfileops.c
> +++ b/libio/wfileops.c
> @@ -412,10 +412,11 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
>        return WEOF;
>      }
>    /* If currently reading or no buffer allocated. */
> -  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
> +  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
> +      || f->_wide_data->_IO_write_base == NULL)
>      {
>        /* Allocate a buffer if needed. */
> -      if (f->_wide_data->_IO_write_base == 0)
> +      if (f->_wide_data->_IO_write_base == NULL)
>      {
>        _IO_wdoallocbuf (f);
>        _IO_free_wbackup_area (f);

Re: [PATCH] libio: Ensure output buffer for wchars (bug 28828)

[José Bollo]

On Mon, 14 Feb 2022 11:15:46 -0300
Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> On 13/02/2022 16:52, jobol@nonadev.net wrote:
> > When fileops.c checks for nullity of the write pointer,
> > in order to ensure its allocation, before that patch,
> > wfileops didn't. This was leading to crashes on some cases,
> > as described by bug 28828.
> > 
> > The minimal sequence to produce the crash was:
> > 
> >     #include <stdio.h>
> >     #include <wchar.h>
> >     int main(int ac, char **av)
> >     {
> >             setvbuf(stdout, NULL, _IOLBF, 0);
> >             fgetwc(stdin);
> >             fputwc(10, stdout); /*CRASH HERE!*/
> >             return 0;
> >     }
> > 
> > The line "fgetwc(stdin);" is necessary. It introduces the
> > bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
> > indirectly (file wfileops.c, function _IO_wfile_underflow, line
> > 213).
> > 
> > Signed-off-by: Jose Bollo <jobol@nonadev.net>  
> 
> Patch looks ok, but git-pw fails to apply it:
> 
> $ git-pw patch apply 51088
> Applying: libio: Ensure output buffer for wchars (bug 28828)
> error: patch failed: libio/wfileops.c:412
> error: libio/wfileops.c: patch does not apply
> error: Did you hand edit your patch?

I copied the ouput of [git format-patch], following as much as possible
https://sourceware.org/glibc/wiki/Contribution%20checklist but didn't
used [git send-email]. I don't have the command [git pw] that you are
using. I can see that [git apply] produce the same issue but [git am]
perfectly works.

> Could you resend it? 

I can try but maybe you have a hint avoiding me to do the same...

> Also, could you also provide a regression testcase for this fix?

Why not. Where? a libio/bug-XXX.c ?

> > ---
> >  libio/wfileops.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libio/wfileops.c b/libio/wfileops.c
> > index fb9d45b677..aa94a22983 100644
> > --- a/libio/wfileops.c
> > +++ b/libio/wfileops.c
> > @@ -412,10 +412,11 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
> >        return WEOF;
> >      }
> >    /* If currently reading or no buffer allocated. */
> > -  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
> > +  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
> > +      || f->_wide_data->_IO_write_base == NULL)
> >      {
> >        /* Allocate a buffer if needed. */
> > -      if (f->_wide_data->_IO_write_base == 0)
> > +      if (f->_wide_data->_IO_write_base == NULL)
> >      {
> >        _IO_wdoallocbuf (f);
> >        _IO_free_wbackup_area (f);  

Re: [PATCH] libio: Ensure output buffer for wchars (bug 28828)

[Adhemerval Zanella]

On 14/02/2022 15:10, José Bollo wrote:
> On Mon, 14 Feb 2022 11:15:46 -0300
> Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> On 13/02/2022 16:52, jobol@nonadev.net wrote:
>>> When fileops.c checks for nullity of the write pointer,
>>> in order to ensure its allocation, before that patch,
>>> wfileops didn't. This was leading to crashes on some cases,
>>> as described by bug 28828.
>>>
>>> The minimal sequence to produce the crash was:
>>>
>>>     #include <stdio.h>
>>>     #include <wchar.h>
>>>     int main(int ac, char **av)
>>>     {
>>>             setvbuf(stdout, NULL, _IOLBF, 0);
>>>             fgetwc(stdin);
>>>             fputwc(10, stdout); /*CRASH HERE!*/
>>>             return 0;
>>>     }
>>>
>>> The line "fgetwc(stdin);" is necessary. It introduces the
>>> bug by setting the flag _IO_CURRENTLY_PUTTING of stdout
>>> indirectly (file wfileops.c, function _IO_wfile_underflow, line
>>> 213).
>>>
>>> Signed-off-by: Jose Bollo <jobol@nonadev.net>  
>>
>> Patch looks ok, but git-pw fails to apply it:
>>
>> $ git-pw patch apply 51088
>> Applying: libio: Ensure output buffer for wchars (bug 28828)
>> error: patch failed: libio/wfileops.c:412
>> error: libio/wfileops.c: patch does not apply
>> error: Did you hand edit your patch?
> 
> I copied the ouput of [git format-patch], following as much as possible
> https://sourceware.org/glibc/wiki/Contribution%20checklist but didn't
> used [git send-email]. I don't have the command [git pw] that you are
> using. I can see that [git apply] produce the same issue but [git am]
> perfectly works.

The main problem is even without using git-pw it does fail (the buildbot
has throw an issue as well).  From the weekly call it seems that your
email client (or the way you have sent the patch) replaces the already
existing tab with whitesapce.

> 
>> Could you resend it? 
> 
> I can try but maybe you have a hint avoiding me to do the same...
> 
>> Also, could you also provide a regression testcase for this fix?
> 
> Why not. Where? a libio/bug-XXX.c ?

I think it preferable to follow current trend, so libio/tst-bz28828.c.

> 
>>> ---
>>>  libio/wfileops.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libio/wfileops.c b/libio/wfileops.c
>>> index fb9d45b677..aa94a22983 100644
>>> --- a/libio/wfileops.c
>>> +++ b/libio/wfileops.c
>>> @@ -412,10 +412,11 @@ _IO_wfile_overflow (FILE *f, wint_t wch)
>>>        return WEOF;
>>>      }
>>>    /* If currently reading or no buffer allocated. */
>>> -  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0)
>>> +  if ((f->_flags & _IO_CURRENTLY_PUTTING) == 0
>>> +      || f->_wide_data->_IO_write_base == NULL)
>>>      {
>>>        /* Allocate a buffer if needed. */
>>> -      if (f->_wide_data->_IO_write_base == 0)
>>> +      if (f->_wide_data->_IO_write_base == NULL)
>>>      {
>>>        _IO_wdoallocbuf (f);
>>>        _IO_free_wbackup_area (f);  
>