public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Discard any push-back from a memory stream before finishing it (bug 34020)
@ 2026-03-30 12:47 Andreas Schwab
  2026-03-30 19:07 ` DJ Delorie
  0 siblings, 1 reply; 5+ messages in thread
From: Andreas Schwab @ 2026-03-30 12:47 UTC (permalink / raw)
  To: libc-alpha

This makes sure that the write base is pointing to the main area, not the
backup area.
---
The wide char test requires
https://patchwork.sourceware.org/project/glibc/patch/20260323171742.1039768-1-marocketbd@gmail.com/
to pass.
---
 libio/Makefile          |  2 +
 libio/memstream.c       |  3 ++
 libio/tst-memstream.h   |  4 ++
 libio/tst-memstream6.c  | 81 +++++++++++++++++++++++++++++++++++++++++
 libio/tst-wmemstream6.c | 20 ++++++++++
 libio/wmemstream.c      |  3 ++
 6 files changed, 113 insertions(+)
 create mode 100644 libio/tst-memstream6.c
 create mode 100644 libio/tst-wmemstream6.c

diff --git a/libio/Makefile b/libio/Makefile
index da838cdecc..4582291b24 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -121,6 +121,7 @@ tests = \
   tst-memstream2 \
   tst-memstream3 \
   tst-memstream4 \
+  tst-memstream6 \
   tst-mmap-eofsync \
   tst-mmap-fflushsync \
   tst-mmap-offend \
@@ -144,6 +145,7 @@ tests = \
   tst-wmemstream3 \
   tst-wmemstream4 \
   tst-wmemstream5 \
+  tst-wmemstream6 \
   tst_getwc \
   tst_putwc \
   tst_swprintf \
diff --git a/libio/memstream.c b/libio/memstream.c
index 0456adb92f..fcc925df87 100644
--- a/libio/memstream.c
+++ b/libio/memstream.c
@@ -100,6 +100,9 @@ _IO_mem_finish (FILE *fp, int dummy)
 {
   struct _IO_FILE_memstream *mp = (struct _IO_FILE_memstream *) fp;
 
+  if (_IO_in_backup (fp))
+    _IO_switch_to_main_get_area (fp);
+
   *mp->bufloc = (char *) realloc (fp->_IO_write_base,
 				  fp->_IO_write_ptr - fp->_IO_write_base + 1);
   if (*mp->bufloc != NULL)
diff --git a/libio/tst-memstream.h b/libio/tst-memstream.h
index b420e32e66..581cfd2cc3 100644
--- a/libio/tst-memstream.h
+++ b/libio/tst-memstream.h
@@ -50,6 +50,8 @@ fwwrite (const void *ptr, size_t size, size_t nmemb, FILE *arq)
 # define FWRITE fwwrite
 # define FPUTC  fputwc
 # define FPUTS  fputws
+# define FGETC  fgetwc
+# define UNGETC	ungetwc
 # define STRCMP wcscmp
 # define STRLEN wcslen
 #else
@@ -60,6 +62,8 @@ fwwrite (const void *ptr, size_t size, size_t nmemb, FILE *arq)
 # define FWRITE fwrite
 # define FPUTC fputc
 # define FPUTS  fputs
+# define FGETC  fgetc
+# define UNGETC	ungetc
 # define STRCMP strcmp
 # define STRLEN strlen
 #endif
diff --git a/libio/tst-memstream6.c b/libio/tst-memstream6.c
new file mode 100644
index 0000000000..128e733b98
--- /dev/null
+++ b/libio/tst-memstream6.c
@@ -0,0 +1,81 @@
+/* Test for open_memstream BZ #34020.
+   Copyright (C) 2018-2026 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 "tst-memstream.h"
+
+static void
+mcheck_abort (enum mcheck_status ev)
+{
+  printf ("mecheck failed with status %d\n", (int) ev);
+  exit (1);
+}
+
+
+void
+do_test_ungetc_1 (void)
+{
+  CHAR_T *buf;
+  size_t size;
+
+  FILE *fp = OPEN_MEMSTREAM (&buf, &size);
+  TEST_VERIFY_EXIT (fp != NULL);
+
+  FPUTC (W('A'), fp);
+  fflush (fp);
+
+  TEST_COMPARE (FGETC (fp), W('A'));
+  TEST_COMPARE (UNGETC (W('B'), fp), W('B'));
+
+  TEST_COMPARE (fclose (fp), 0);
+
+  TEST_COMPARE (buf[0], W('A'));
+
+  free (buf);
+}
+
+
+void
+do_test_ungetc_2 (void)
+{
+  CHAR_T *buf;
+  size_t size;
+
+  FILE *fp = OPEN_MEMSTREAM (&buf, &size);
+  TEST_VERIFY_EXIT (fp != NULL);
+
+  TEST_COMPARE (UNGETC (W('A'), fp), W('A'));
+
+  TEST_COMPARE (fclose (fp), 0);
+
+  TEST_COMPARE (buf[0], 0);
+
+  free (buf);
+}
+
+static int
+do_test (void)
+{
+  mcheck_pedantic (mcheck_abort);
+
+  do_test_ungetc_1 ();
+  do_test_ungetc_2 ();
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/libio/tst-wmemstream6.c b/libio/tst-wmemstream6.c
new file mode 100644
index 0000000000..23c50197cf
--- /dev/null
+++ b/libio/tst-wmemstream6.c
@@ -0,0 +1,20 @@
+/* Test for open_wmemstream BZ #34020.
+   Copyright (C) 2026 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/>.  */
+
+#define TEST_WCHAR
+#include <libio/tst-memstream6.c>
diff --git a/libio/wmemstream.c b/libio/wmemstream.c
index d0c639be70..a0c61a627f 100644
--- a/libio/wmemstream.c
+++ b/libio/wmemstream.c
@@ -101,6 +101,9 @@ _IO_wmem_finish (FILE *fp, int dummy)
 {
   struct _IO_FILE_wmemstream *mp = (struct _IO_FILE_wmemstream *) fp;
 
+  if (_IO_in_backup (fp))
+    _IO_switch_to_main_wget_area (fp);
+
   *mp->bufloc = (wchar_t *) realloc (fp->_wide_data->_IO_write_base,
 				     (fp->_wide_data->_IO_write_ptr
 				      - fp->_wide_data->_IO_write_base + 1)
-- 
2.53.0


-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Discard any push-back from a memory stream before finishing it (bug 34020)
  2026-03-30 12:47 [PATCH] Discard any push-back from a memory stream before finishing it (bug 34020) Andreas Schwab
@ 2026-03-30 19:07 ` DJ Delorie
  2026-04-02 19:12   ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 5+ messages in thread
From: DJ Delorie @ 2026-03-30 19:07 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha

Andreas Schwab <schwab@suse.de> writes:
> This makes sure that the write base is pointing to the main area, not the
> backup area.

So I just realized, after working in this code for years, that "backup"
here doesn't mean "in case we run out" it means "going backwards" :-P No
wonder it seemed weird to me.

One question below about fseek, but otherwise OK.

> diff --git a/libio/memstream.c b/libio/memstream.c
> index 0456adb92f..fcc925df87 100644
> --- a/libio/memstream.c
> +++ b/libio/memstream.c
> @@ -100,6 +100,9 @@ _IO_mem_finish (FILE *fp, int dummy)
>  {
>    struct _IO_FILE_memstream *mp = (struct _IO_FILE_memstream *) fp;
>  
> +  if (_IO_in_backup (fp))
> +    _IO_switch_to_main_get_area (fp);
> +
>    *mp->bufloc = (char *) realloc (fp->_IO_write_base,
>  				  fp->_IO_write_ptr - fp->_IO_write_base + 1);
>    if (*mp->bufloc != NULL)

Ok.

> diff --git a/libio/tst-memstream6.c b/libio/tst-memstream6.c
> +static void
> +mcheck_abort (enum mcheck_status ev)
> +{
> +  printf ("mecheck failed with status %d\n", (int) ev);
> +  exit (1);
> +}

Ok.

> +void
> +do_test_ungetc_1 (void)
> +{
> +  CHAR_T *buf;
> +  size_t size;
> +
> +  FILE *fp = OPEN_MEMSTREAM (&buf, &size);
> +  TEST_VERIFY_EXIT (fp != NULL);
> +
> +  FPUTC (W('A'), fp);
> +  fflush (fp);
> +
> +  TEST_COMPARE (FGETC (fp), W('A'));

I would have expected you to need an fseek here.  fflush() on a
memstream synchronizes the buffer and size pointers but says nothing
about moving the file pointer.

> +  TEST_COMPARE (UNGETC (W('B'), fp), W('B'));
> +
> +  TEST_COMPARE (fclose (fp), 0);
> +
> +  TEST_COMPARE (buf[0], W('A'));
> +
> +  free (buf);
> +}

Ok.

> +void
> +do_test_ungetc_2 (void)
> +{
> +  CHAR_T *buf;
> +  size_t size;
> +
> +  FILE *fp = OPEN_MEMSTREAM (&buf, &size);
> +  TEST_VERIFY_EXIT (fp != NULL);
> +
> +  TEST_COMPARE (UNGETC (W('A'), fp), W('A'));
> +
> +  TEST_COMPARE (fclose (fp), 0);
> +
> +  TEST_COMPARE (buf[0], 0);
> +
> +  free (buf);
> +}

Ok.

> +static int
> +do_test (void)
> +{
> +  mcheck_pedantic (mcheck_abort);
> +
> +  do_test_ungetc_1 ();
> +  do_test_ungetc_2 ();
> +
> +  return 0;
> +}

Ok.

> diff --git a/libio/tst-wmemstream6.c b/libio/tst-wmemstream6.c
> +#define TEST_WCHAR
> +#include <libio/tst-memstream6.c>

Ok.

> diff --git a/libio/wmemstream.c b/libio/wmemstream.c
> index d0c639be70..a0c61a627f 100644
> --- a/libio/wmemstream.c
> +++ b/libio/wmemstream.c
> @@ -101,6 +101,9 @@ _IO_wmem_finish (FILE *fp, int dummy)
>  {
>    struct _IO_FILE_wmemstream *mp = (struct _IO_FILE_wmemstream *) fp;
>  
> +  if (_IO_in_backup (fp))
> +    _IO_switch_to_main_wget_area (fp);
> +
>    *mp->bufloc = (wchar_t *) realloc (fp->_wide_data->_IO_write_base,
>  				     (fp->_wide_data->_IO_write_ptr
>  				      - fp->_wide_data->_IO_write_base + 1)

Ok.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Discard any push-back from a memory stream before finishing it (bug 34020)
  2026-03-30 19:07 ` DJ Delorie
@ 2026-04-02 19:12   ` Adhemerval Zanella Netto
  2026-04-07 10:26     ` Andreas Schwab
  0 siblings, 1 reply; 5+ messages in thread
From: Adhemerval Zanella Netto @ 2026-04-02 19:12 UTC (permalink / raw)
  To: DJ Delorie, Andreas Schwab; +Cc: libc-alpha



On 30/03/26 16:07, DJ Delorie wrote:
> Andreas Schwab <schwab@suse.de> writes:
>> This makes sure that the write base is pointing to the main area, not the
>> backup area.
> 
> So I just realized, after working in this code for years, that "backup"
> here doesn't mean "in case we run out" it means "going backwards" :-P No
> wonder it seemed weird to me.
> 
> One question below about fseek, but otherwise OK.

The new testcase triggers some regression on both redhad and Linaro CI [1],
and it seems a real one.


[1] https://patchwork.sourceware.org/project/glibc/patch/mvm7bqtiivh.fsf@suse.de/

> 
>> diff --git a/libio/memstream.c b/libio/memstream.c
>> index 0456adb92f..fcc925df87 100644
>> --- a/libio/memstream.c
>> +++ b/libio/memstream.c
>> @@ -100,6 +100,9 @@ _IO_mem_finish (FILE *fp, int dummy)
>>  {
>>    struct _IO_FILE_memstream *mp = (struct _IO_FILE_memstream *) fp;
>>  
>> +  if (_IO_in_backup (fp))
>> +    _IO_switch_to_main_get_area (fp);
>> +
>>    *mp->bufloc = (char *) realloc (fp->_IO_write_base,
>>  				  fp->_IO_write_ptr - fp->_IO_write_base + 1);
>>    if (*mp->bufloc != NULL)
> 
> Ok.
> 
>> diff --git a/libio/tst-memstream6.c b/libio/tst-memstream6.c
>> +static void
>> +mcheck_abort (enum mcheck_status ev)
>> +{
>> +  printf ("mecheck failed with status %d\n", (int) ev);
>> +  exit (1);
>> +}
> 
> Ok.
> 
>> +void
>> +do_test_ungetc_1 (void)
>> +{
>> +  CHAR_T *buf;
>> +  size_t size;
>> +
>> +  FILE *fp = OPEN_MEMSTREAM (&buf, &size);
>> +  TEST_VERIFY_EXIT (fp != NULL);
>> +
>> +  FPUTC (W('A'), fp);
>> +  fflush (fp);
>> +
>> +  TEST_COMPARE (FGETC (fp), W('A'));
> 
> I would have expected you to need an fseek here.  fflush() on a
> memstream synchronizes the buffer and size pointers but says nothing
> about moving the file pointer.
> 
>> +  TEST_COMPARE (UNGETC (W('B'), fp), W('B'));
>> +
>> +  TEST_COMPARE (fclose (fp), 0);
>> +
>> +  TEST_COMPARE (buf[0], W('A'));
>> +
>> +  free (buf);
>> +}
> 
> Ok.
> 
>> +void
>> +do_test_ungetc_2 (void)
>> +{
>> +  CHAR_T *buf;
>> +  size_t size;
>> +
>> +  FILE *fp = OPEN_MEMSTREAM (&buf, &size);
>> +  TEST_VERIFY_EXIT (fp != NULL);
>> +
>> +  TEST_COMPARE (UNGETC (W('A'), fp), W('A'));
>> +
>> +  TEST_COMPARE (fclose (fp), 0);
>> +
>> +  TEST_COMPARE (buf[0], 0);
>> +
>> +  free (buf);
>> +}
> 
> Ok.
> 
>> +static int
>> +do_test (void)
>> +{
>> +  mcheck_pedantic (mcheck_abort);
>> +
>> +  do_test_ungetc_1 ();
>> +  do_test_ungetc_2 ();
>> +
>> +  return 0;
>> +}
> 
> Ok.
> 
>> diff --git a/libio/tst-wmemstream6.c b/libio/tst-wmemstream6.c
>> +#define TEST_WCHAR
>> +#include <libio/tst-memstream6.c>
> 
> Ok.
> 
>> diff --git a/libio/wmemstream.c b/libio/wmemstream.c
>> index d0c639be70..a0c61a627f 100644
>> --- a/libio/wmemstream.c
>> +++ b/libio/wmemstream.c
>> @@ -101,6 +101,9 @@ _IO_wmem_finish (FILE *fp, int dummy)
>>  {
>>    struct _IO_FILE_wmemstream *mp = (struct _IO_FILE_wmemstream *) fp;
>>  
>> +  if (_IO_in_backup (fp))
>> +    _IO_switch_to_main_wget_area (fp);
>> +
>>    *mp->bufloc = (wchar_t *) realloc (fp->_wide_data->_IO_write_base,
>>  				     (fp->_wide_data->_IO_write_ptr
>>  				      - fp->_wide_data->_IO_write_base + 1)
> 
> Ok.
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Discard any push-back from a memory stream before finishing it (bug 34020)
  2026-04-02 19:12   ` Adhemerval Zanella Netto
@ 2026-04-07 10:26     ` Andreas Schwab
  2026-04-07 16:14       ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 5+ messages in thread
From: Andreas Schwab @ 2026-04-07 10:26 UTC (permalink / raw)
  To: Adhemerval Zanella Netto; +Cc: DJ Delorie, libc-alpha

On Apr 02 2026, Adhemerval Zanella Netto wrote:

> The new testcase triggers some regression on both redhad and Linaro CI [1],
> and it seems a real one.

Yes, as I said.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Discard any push-back from a memory stream before finishing it (bug 34020)
  2026-04-07 10:26     ` Andreas Schwab
@ 2026-04-07 16:14       ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 5+ messages in thread
From: Adhemerval Zanella Netto @ 2026-04-07 16:14 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: DJ Delorie, libc-alpha



On 07/04/26 07:26, Andreas Schwab wrote:
> On Apr 02 2026, Adhemerval Zanella Netto wrote:
> 
>> The new testcase triggers some regression on both redhad and Linaro CI [1],
>> and it seems a real one.
> 
> Yes, as I said.
> 

Maybe approve it first, and they send the patch? It kinda confusing to check
on commit message to understand why CI has failed.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-07 16:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-30 12:47 [PATCH] Discard any push-back from a memory stream before finishing it (bug 34020) Andreas Schwab
2026-03-30 19:07 ` DJ Delorie
2026-04-02 19:12   ` Adhemerval Zanella Netto
2026-04-07 10:26     ` Andreas Schwab
2026-04-07 16:14       ` Adhemerval Zanella Netto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).