[v2] libio: Fix wrong reference to byte stream
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
Commit Message
* libio/wgenops.c: Fix _IO_wdefault_pbackfail and _IO_wdefault_finish by
deferencing wide stream pointers, should be a mistake (bug 33998[1] and
bug 33999[2])
The regression test attached can not perform fclose, see bug 34020[3]
[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=33998
[2]: https://sourceware.org/bugzilla/show_bug.cgi?id=33999
[3]: https://sourceware.org/bugzilla/show_bug.cgi?id=34020
Signed-off-by: Rocket Ma <marocketbd@gmail.com>
---
libio/Makefile | 1 +
libio/bug-wgenops.c | 30 ++++++++++++++++++++++++++++++
libio/wgenops.c | 8 ++++----
3 files changed, 35 insertions(+), 4 deletions(-)
create mode 100644 libio/bug-wgenops.c
Comments
On 3/23/26 1:17 PM, Rocket Ma wrote:
> * libio/wgenops.c: Fix _IO_wdefault_pbackfail and _IO_wdefault_finish by
> deferencing wide stream pointers, should be a mistake (bug 33998[1] and
> bug 33999[2])
Please split the patch to only fix one at a time.
For example 33999 is being discussed here with a test case:
https://inbox.sourceware.org/libc-alpha/20260403015721.343918-1-gaoxiang@kylinos.cn/
> The regression test attached can not perform fclose, see bug 34020[3]
Andreas Schwab commented on this one.
>
> [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=33998
> [2]: https://sourceware.org/bugzilla/show_bug.cgi?id=33999
> [3]: https://sourceware.org/bugzilla/show_bug.cgi?id=34020
>
> Signed-off-by: Rocket Ma <marocketbd@gmail.com>
> ---
> libio/Makefile | 1 +
> libio/bug-wgenops.c | 30 ++++++++++++++++++++++++++++++
> libio/wgenops.c | 8 ++++----
> 3 files changed, 35 insertions(+), 4 deletions(-)
> create mode 100644 libio/bug-wgenops.c
>
> diff --git a/libio/Makefile b/libio/Makefile
> index 08e1e0ec25..da838cdecc 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -84,6 +84,7 @@ tests = \
> bug-ungetwc1 \
> bug-ungetwc2 \
> bug-wfflush \
> + bug-wgenops \
> bug-wmemstream1 \
> bug-wsetpos \
> test-fmemopen \
> diff --git a/libio/bug-wgenops.c b/libio/bug-wgenops.c
> new file mode 100644
> index 0000000000..cf67b9d436
> --- /dev/null
> +++ b/libio/bug-wgenops.c
> @@ -0,0 +1,30 @@
May you please add a copyright header for the test case please?
We license the test cases with a matching project license.
May you please add a line at the top that describes the test.
Example:
/* Bug XXXX: Test XXXX does YYYY.
Copyright The GNU Toolchain Authors.
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 <stdio.h>
> +#include <wchar.h>
> +#include <stdlib.h>
> +
> +#define tst_assert(cond) \
> + if (!(cond)) \
> + { \
> + puts ("Failed assertion: " #cond); \
> + return 1; \
> + }
Please see the support/* directory and #include <support/check.h> for TEST_VERIFY.
> +
> +static int
> +do_test (void)
> +{
> + wchar_t *buf = NULL;
> + size_t size = 0;
> + FILE *fp = open_wmemstream (&buf, &size);
> + tst_assert (fp != NULL);
These should be TEST_VERIFY(...);
> + tst_assert (fputwc (L'A', fp) != WEOF);
> + tst_assert (fflush (fp) == 0);
> +
> + tst_assert (fgetwc (fp) == L'A');
> + tst_assert (ungetwc (L'B', fp) == L'B');
> +
> + // fclose (fp); // See BZ 34020; flose will free invalid _IO_save_base
> + free (buf);
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
OK. Good use of the support test driver.
> diff --git a/libio/wgenops.c b/libio/wgenops.c
> index 064d71266d..3ebbb941f0 100644
> --- a/libio/wgenops.c
> +++ b/libio/wgenops.c
> @@ -110,8 +110,8 @@ _IO_wdefault_pbackfail (FILE *fp, wint_t c)
> {
> if (fp->_wide_data->_IO_read_ptr > fp->_wide_data->_IO_read_base
> && !_IO_in_backup (fp)
> - && (wint_t) fp->_IO_read_ptr[-1] == c)
> - --fp->_IO_read_ptr;
> + && (wint_t) fp->_wide_data->_IO_read_ptr[-1] == c)
OK. This is the relevant change to use the correct buffer.
> + --fp->_wide_data->_IO_read_ptr;
> else
> {
> /* Need to handle a filebuf in write mode (switch to read mode). FIXME!*/
> @@ -181,10 +181,10 @@ _IO_wdefault_finish (FILE *fp, int dummy)
> for (mark = fp->_markers; mark != NULL; mark = mark->_next)
> mark->_sbuf = NULL;
>
> - if (fp->_IO_save_base)
> + if (fp->_wide_data->_IO_save_base)
> {
> free (fp->_wide_data->_IO_save_base);
> - fp->_IO_save_base = NULL;
> + fp->_wide_data->_IO_save_base = NULL;
This is being fixed in a different way by Gao Xiang in:
https://inbox.sourceware.org/libc-alpha/20260403015721.343918-1-gaoxiang@kylinos.cn/
> }
>
> #ifdef _IO_MTSAFE_IO
@@ -84,6 +84,7 @@ tests = \
bug-ungetwc1 \
bug-ungetwc2 \
bug-wfflush \
+ bug-wgenops \
bug-wmemstream1 \
bug-wsetpos \
test-fmemopen \
new file mode 100644
@@ -0,0 +1,30 @@
+#include <stdio.h>
+#include <wchar.h>
+#include <stdlib.h>
+
+#define tst_assert(cond) \
+ if (!(cond)) \
+ { \
+ puts ("Failed assertion: " #cond); \
+ return 1; \
+ }
+
+static int
+do_test (void)
+{
+ wchar_t *buf = NULL;
+ size_t size = 0;
+ FILE *fp = open_wmemstream (&buf, &size);
+ tst_assert (fp != NULL);
+ tst_assert (fputwc (L'A', fp) != WEOF);
+ tst_assert (fflush (fp) == 0);
+
+ tst_assert (fgetwc (fp) == L'A');
+ tst_assert (ungetwc (L'B', fp) == L'B');
+
+ // fclose (fp); // See BZ 34020; flose will free invalid _IO_save_base
+ free (buf);
+ return 0;
+}
+
+#include <support/test-driver.c>
@@ -110,8 +110,8 @@ _IO_wdefault_pbackfail (FILE *fp, wint_t c)
{
if (fp->_wide_data->_IO_read_ptr > fp->_wide_data->_IO_read_base
&& !_IO_in_backup (fp)
- && (wint_t) fp->_IO_read_ptr[-1] == c)
- --fp->_IO_read_ptr;
+ && (wint_t) fp->_wide_data->_IO_read_ptr[-1] == c)
+ --fp->_wide_data->_IO_read_ptr;
else
{
/* Need to handle a filebuf in write mode (switch to read mode). FIXME!*/
@@ -181,10 +181,10 @@ _IO_wdefault_finish (FILE *fp, int dummy)
for (mark = fp->_markers; mark != NULL; mark = mark->_next)
mark->_sbuf = NULL;
- if (fp->_IO_save_base)
+ if (fp->_wide_data->_IO_save_base)
{
free (fp->_wide_data->_IO_save_base);
- fp->_IO_save_base = NULL;
+ fp->_wide_data->_IO_save_base = NULL;
}
#ifdef _IO_MTSAFE_IO