[v3] libio: Use __wunderflow in _IO_getwline_info
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_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
Commit Message
After a buffer refill, _IO_getwline_info reads the first character
through __wuflow and handles it on a separate path from the rest of
the buffer. When extract_delim is negative, that path calls
_IO_sputbackc, the narrow-stream putback, on a wide stream, so it
would operate on fp->_IO_read_ptr instead of
fp->_wide_data->_IO_read_ptr. No current caller passes a negative
extract_delim, so this is not reachable today.
Refill the buffer with __wunderflow instead, which does not consume
anything, and let the wmemchr path handle every character including
the delimiter. This removes the duplicated delimiter handling and
the wrong putback. Every wide underflow implementation either
returns WEOF or leaves at least one character in the buffer, so the
loop cannot spin.
No functional change for fgetws and fgetws_unlocked. Tested with the
libio tests on x86_64-linux-gnu, including tst-fgetws and tst-widetext.
Signed-off-by: Shamil Abdulaev <ashamil435@gmail.com>
---
Notes:
v3: Drop the blank lines that were only cosmetic, as suggested by
Adhemerval Zanella.
v2: Check for an empty buffer with a pointer comparison and make len
size_t, as suggested by Andreas Schwab.
Andreas also suggested removing the extract_delim parameter from
_IO_getwline and folding _IO_getwline_info into it. I can do that as
a separate follow-up patch if you would like.
libio/iogetwline.c | 66 ++++++++++++++++++----------------------------
1 file changed, 25 insertions(+), 41 deletions(-)
Comments
friendly ping
ср, 30 сент. 2026 г. в 16:13, Shamil Abdulaev <ashamil435@gmail.com>:
> After a buffer refill, _IO_getwline_info reads the first character
> through __wuflow and handles it on a separate path from the rest of
> the buffer. When extract_delim is negative, that path calls
> _IO_sputbackc, the narrow-stream putback, on a wide stream, so it
> would operate on fp->_IO_read_ptr instead of
> fp->_wide_data->_IO_read_ptr. No current caller passes a negative
> extract_delim, so this is not reachable today.
>
> Refill the buffer with __wunderflow instead, which does not consume
> anything, and let the wmemchr path handle every character including
> the delimiter. This removes the duplicated delimiter handling and
> the wrong putback. Every wide underflow implementation either
> returns WEOF or leaves at least one character in the buffer, so the
> loop cannot spin.
>
> No functional change for fgetws and fgetws_unlocked. Tested with the
> libio tests on x86_64-linux-gnu, including tst-fgetws and tst-widetext.
>
> Signed-off-by: Shamil Abdulaev <ashamil435@gmail.com>
> ---
>
> Notes:
> v3: Drop the blank lines that were only cosmetic, as suggested by
> Adhemerval Zanella.
>
> v2: Check for an empty buffer with a pointer comparison and make len
> size_t, as suggested by Andreas Schwab.
>
> Andreas also suggested removing the extract_delim parameter from
> _IO_getwline and folding _IO_getwline_info into it. I can do that as
> a separate follow-up patch if you would like.
>
> libio/iogetwline.c | 66 ++++++++++++++++++----------------------------
> 1 file changed, 25 insertions(+), 41 deletions(-)
>
> diff --git a/libio/iogetwline.c b/libio/iogetwline.c
> index 6a5adf92cb..e6f94e44e3 100644
> --- a/libio/iogetwline.c
> +++ b/libio/iogetwline.c
> @@ -54,56 +54,40 @@ _IO_getwline_info (FILE *fp, wchar_t *buf, size_t n,
> wint_t delim,
> _IO_fwide (fp, 1);
> while (n != 0)
> {
> - ssize_t len = (fp->_wide_data->_IO_read_end
> - - fp->_wide_data->_IO_read_ptr);
> - if (len <= 0)
> + wchar_t *read_ptr = fp->_wide_data->_IO_read_ptr;
> + if (fp->_wide_data->_IO_read_end <= read_ptr)
> {
> - wint_t wc = __wuflow (fp);
> - if (wc == WEOF)
> + /* Refill the buffer without consuming anything, so that the
> + delimiter is always handled by the block path below. */
> + if (__wunderflow (fp) == WEOF)
> {
> - if (eof)
> - *eof = wc;
> + if (eof != NULL)
> + *eof = WEOF;
> break;
> }
> - if (wc == delim)
> + continue;
> + }
> + size_t len = fp->_wide_data->_IO_read_end - read_ptr;
> + if (len > n)
> + len = n;
> + wchar_t *t = wmemchr (read_ptr, delim, len);
> + if (t != NULL)
> + {
> + len = t - read_ptr;
> + if (extract_delim >= 0)
> {
> - if (extract_delim > 0)
> - *ptr++ = wc;
> - else if (extract_delim < 0)
> - _IO_sputbackc (fp, wc);
> + ++t;
> if (extract_delim > 0)
> ++len;
> - return ptr - buf;
> }
> - *ptr++ = wc;
> - n--;
> - }
> - else
> - {
> - wchar_t *t;
> - if ((size_t) len >= n)
> - len = n;
> - t = wmemchr ((void *) fp->_wide_data->_IO_read_ptr, delim, len);
> - if (t != NULL)
> - {
> - size_t old_len = ptr - buf;
> - len = t - fp->_wide_data->_IO_read_ptr;
> - if (extract_delim >= 0)
> - {
> - ++t;
> - if (extract_delim > 0)
> - ++len;
> - }
> - __wmemcpy ((void *) ptr, (void *)
> fp->_wide_data->_IO_read_ptr,
> - len);
> - fp->_wide_data->_IO_read_ptr = t;
> - return old_len + len;
> - }
> - __wmemcpy ((void *) ptr, (void *) fp->_wide_data->_IO_read_ptr,
> len);
> - fp->_wide_data->_IO_read_ptr += len;
> - ptr += len;
> - n -= len;
> + __wmemcpy (ptr, read_ptr, len);
> + fp->_wide_data->_IO_read_ptr = t;
> + return (ptr - buf) + len;
> }
> + __wmemcpy (ptr, read_ptr, len);
> + fp->_wide_data->_IO_read_ptr = read_ptr + len;
> + ptr += len;
> + n -= len;
> }
> return ptr - buf;
> }
> --
> 2.55.0
>
>
On 30/09/26 15:13, Shamil Abdulaev wrote:
> After a buffer refill, _IO_getwline_info reads the first character
> through __wuflow and handles it on a separate path from the rest of
> the buffer. When extract_delim is negative, that path calls
> _IO_sputbackc, the narrow-stream putback, on a wide stream, so it
> would operate on fp->_IO_read_ptr instead of
> fp->_wide_data->_IO_read_ptr. No current caller passes a negative
> extract_delim, so this is not reachable today.
>
> Refill the buffer with __wunderflow instead, which does not consume
> anything, and let the wmemchr path handle every character including
> the delimiter. This removes the duplicated delimiter handling and
> the wrong putback. Every wide underflow implementation either
> returns WEOF or leaves at least one character in the buffer, so the
> loop cannot spin.
>
> No functional change for fgetws and fgetws_unlocked. Tested with the
> libio tests on x86_64-linux-gnu, including tst-fgetws and tst-widetext.
>
> Signed-off-by: Shamil Abdulaev <ashamil435@gmail.com>
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
>
> Notes:
> v3: Drop the blank lines that were only cosmetic, as suggested by
> Adhemerval Zanella.
>
> v2: Check for an empty buffer with a pointer comparison and make len
> size_t, as suggested by Andreas Schwab.
>
> Andreas also suggested removing the extract_delim parameter from
> _IO_getwline and folding _IO_getwline_info into it. I can do that as
> a separate follow-up patch if you would like.
>
> libio/iogetwline.c | 66 ++++++++++++++++++----------------------------
> 1 file changed, 25 insertions(+), 41 deletions(-)
>
> diff --git a/libio/iogetwline.c b/libio/iogetwline.c
> index 6a5adf92cb..e6f94e44e3 100644
> --- a/libio/iogetwline.c
> +++ b/libio/iogetwline.c
> @@ -54,56 +54,40 @@ _IO_getwline_info (FILE *fp, wchar_t *buf, size_t n, wint_t delim,
> _IO_fwide (fp, 1);
> while (n != 0)
> {
> - ssize_t len = (fp->_wide_data->_IO_read_end
> - - fp->_wide_data->_IO_read_ptr);
> - if (len <= 0)
> + wchar_t *read_ptr = fp->_wide_data->_IO_read_ptr;
> + if (fp->_wide_data->_IO_read_end <= read_ptr)
> {
> - wint_t wc = __wuflow (fp);
> - if (wc == WEOF)
> + /* Refill the buffer without consuming anything, so that the
> + delimiter is always handled by the block path below. */
> + if (__wunderflow (fp) == WEOF)
> {
> - if (eof)
> - *eof = wc;
> + if (eof != NULL)
> + *eof = WEOF;
> break;
> }
> - if (wc == delim)
> + continue;
> + }
> + size_t len = fp->_wide_data->_IO_read_end - read_ptr;
> + if (len > n)
> + len = n;
> + wchar_t *t = wmemchr (read_ptr, delim, len);
> + if (t != NULL)
> + {
> + len = t - read_ptr;
> + if (extract_delim >= 0)
> {
> - if (extract_delim > 0)
> - *ptr++ = wc;
> - else if (extract_delim < 0)
> - _IO_sputbackc (fp, wc);
> + ++t;
> if (extract_delim > 0)
> ++len;
> - return ptr - buf;
> }
> - *ptr++ = wc;
> - n--;
> - }
> - else
> - {
> - wchar_t *t;
> - if ((size_t) len >= n)
> - len = n;
> - t = wmemchr ((void *) fp->_wide_data->_IO_read_ptr, delim, len);
> - if (t != NULL)
> - {
> - size_t old_len = ptr - buf;
> - len = t - fp->_wide_data->_IO_read_ptr;
> - if (extract_delim >= 0)
> - {
> - ++t;
> - if (extract_delim > 0)
> - ++len;
> - }
> - __wmemcpy ((void *) ptr, (void *) fp->_wide_data->_IO_read_ptr,
> - len);
> - fp->_wide_data->_IO_read_ptr = t;
> - return old_len + len;
> - }
> - __wmemcpy ((void *) ptr, (void *) fp->_wide_data->_IO_read_ptr, len);
> - fp->_wide_data->_IO_read_ptr += len;
> - ptr += len;
> - n -= len;
> + __wmemcpy (ptr, read_ptr, len);
> + fp->_wide_data->_IO_read_ptr = t;
> + return (ptr - buf) + len;
> }
> + __wmemcpy (ptr, read_ptr, len);
> + fp->_wide_data->_IO_read_ptr = read_ptr + len;
> + ptr += len;
> + n -= len;
> }
> return ptr - buf;
> }
Thanks for the review!
I don't have commit access to glibc -- could you push this on my behalf
when convenient? Happy to rebase or address anything if needed.
Thanks,
Shamil
вт, 6 окт. 2026 г. в 16:50, Adhemerval Zanella Netto <
adhemerval.zanella@linaro.org>:
>
>
> On 30/09/26 15:13, Shamil Abdulaev wrote:
> > After a buffer refill, _IO_getwline_info reads the first character
> > through __wuflow and handles it on a separate path from the rest of
> > the buffer. When extract_delim is negative, that path calls
> > _IO_sputbackc, the narrow-stream putback, on a wide stream, so it
> > would operate on fp->_IO_read_ptr instead of
> > fp->_wide_data->_IO_read_ptr. No current caller passes a negative
> > extract_delim, so this is not reachable today.
> >
> > Refill the buffer with __wunderflow instead, which does not consume
> > anything, and let the wmemchr path handle every character including
> > the delimiter. This removes the duplicated delimiter handling and
> > the wrong putback. Every wide underflow implementation either
> > returns WEOF or leaves at least one character in the buffer, so the
> > loop cannot spin.
> >
> > No functional change for fgetws and fgetws_unlocked. Tested with the
> > libio tests on x86_64-linux-gnu, including tst-fgetws and tst-widetext.
> >
> > Signed-off-by: Shamil Abdulaev <ashamil435@gmail.com>
>
> LGTM, thanks.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> > ---
> >
> > Notes:
> > v3: Drop the blank lines that were only cosmetic, as suggested by
> > Adhemerval Zanella.
> >
> > v2: Check for an empty buffer with a pointer comparison and make len
> > size_t, as suggested by Andreas Schwab.
> >
> > Andreas also suggested removing the extract_delim parameter from
> > _IO_getwline and folding _IO_getwline_info into it. I can do that as
> > a separate follow-up patch if you would like.
> >
> > libio/iogetwline.c | 66 ++++++++++++++++++----------------------------
> > 1 file changed, 25 insertions(+), 41 deletions(-)
> >
> > diff --git a/libio/iogetwline.c b/libio/iogetwline.c
> > index 6a5adf92cb..e6f94e44e3 100644
> > --- a/libio/iogetwline.c
> > +++ b/libio/iogetwline.c
> > @@ -54,56 +54,40 @@ _IO_getwline_info (FILE *fp, wchar_t *buf, size_t n,
> wint_t delim,
> > _IO_fwide (fp, 1);
> > while (n != 0)
> > {
> > - ssize_t len = (fp->_wide_data->_IO_read_end
> > - - fp->_wide_data->_IO_read_ptr);
> > - if (len <= 0)
> > + wchar_t *read_ptr = fp->_wide_data->_IO_read_ptr;
> > + if (fp->_wide_data->_IO_read_end <= read_ptr)
> > {
> > - wint_t wc = __wuflow (fp);
> > - if (wc == WEOF)
> > + /* Refill the buffer without consuming anything, so that the
> > + delimiter is always handled by the block path below. */
> > + if (__wunderflow (fp) == WEOF)
> > {
> > - if (eof)
> > - *eof = wc;
> > + if (eof != NULL)
> > + *eof = WEOF;
> > break;
> > }
> > - if (wc == delim)
> > + continue;
> > + }
> > + size_t len = fp->_wide_data->_IO_read_end - read_ptr;
> > + if (len > n)
> > + len = n;
> > + wchar_t *t = wmemchr (read_ptr, delim, len);
> > + if (t != NULL)
> > + {
> > + len = t - read_ptr;
> > + if (extract_delim >= 0)
> > {
> > - if (extract_delim > 0)
> > - *ptr++ = wc;
> > - else if (extract_delim < 0)
> > - _IO_sputbackc (fp, wc);
> > + ++t;
> > if (extract_delim > 0)
> > ++len;
> > - return ptr - buf;
> > }
> > - *ptr++ = wc;
> > - n--;
> > - }
> > - else
> > - {
> > - wchar_t *t;
> > - if ((size_t) len >= n)
> > - len = n;
> > - t = wmemchr ((void *) fp->_wide_data->_IO_read_ptr, delim, len);
> > - if (t != NULL)
> > - {
> > - size_t old_len = ptr - buf;
> > - len = t - fp->_wide_data->_IO_read_ptr;
> > - if (extract_delim >= 0)
> > - {
> > - ++t;
> > - if (extract_delim > 0)
> > - ++len;
> > - }
> > - __wmemcpy ((void *) ptr, (void *)
> fp->_wide_data->_IO_read_ptr,
> > - len);
> > - fp->_wide_data->_IO_read_ptr = t;
> > - return old_len + len;
> > - }
> > - __wmemcpy ((void *) ptr, (void *) fp->_wide_data->_IO_read_ptr,
> len);
> > - fp->_wide_data->_IO_read_ptr += len;
> > - ptr += len;
> > - n -= len;
> > + __wmemcpy (ptr, read_ptr, len);
> > + fp->_wide_data->_IO_read_ptr = t;
> > + return (ptr - buf) + len;
> > }
> > + __wmemcpy (ptr, read_ptr, len);
> > + fp->_wide_data->_IO_read_ptr = read_ptr + len;
> > + ptr += len;
> > + n -= len;
> > }
> > return ptr - buf;
> > }
>
>
Done, thanks for working on this.
On 07/10/26 10:53, Shamil Abdulaev wrote:
> Thanks for the review!
>
> I don't have commit access to glibc -- could you push this on my behalf
> when convenient? Happy to rebase or address anything if needed.
>
> Thanks,
> Shamil
>
> вт, 6 окт. 2026 г. в 16:50, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org <mailto:adhemerval.zanella@linaro.org>>:
>
>
>
> On 30/09/26 15:13, Shamil Abdulaev wrote:
> > After a buffer refill, _IO_getwline_info reads the first character
> > through __wuflow and handles it on a separate path from the rest of
> > the buffer. When extract_delim is negative, that path calls
> > _IO_sputbackc, the narrow-stream putback, on a wide stream, so it
> > would operate on fp->_IO_read_ptr instead of
> > fp->_wide_data->_IO_read_ptr. No current caller passes a negative
> > extract_delim, so this is not reachable today.
> >
> > Refill the buffer with __wunderflow instead, which does not consume
> > anything, and let the wmemchr path handle every character including
> > the delimiter. This removes the duplicated delimiter handling and
> > the wrong putback. Every wide underflow implementation either
> > returns WEOF or leaves at least one character in the buffer, so the
> > loop cannot spin.
> >
> > No functional change for fgetws and fgetws_unlocked. Tested with the
> > libio tests on x86_64-linux-gnu, including tst-fgetws and tst-widetext.
> >
> > Signed-off-by: Shamil Abdulaev <ashamil435@gmail.com <mailto:ashamil435@gmail.com>>
>
> LGTM, thanks.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org <mailto:adhemerval.zanella@linaro.org>>
>
> > ---
> >
> > Notes:
> > v3: Drop the blank lines that were only cosmetic, as suggested by
> > Adhemerval Zanella.
> >
> > v2: Check for an empty buffer with a pointer comparison and make len
> > size_t, as suggested by Andreas Schwab.
> >
> > Andreas also suggested removing the extract_delim parameter from
> > _IO_getwline and folding _IO_getwline_info into it. I can do that as
> > a separate follow-up patch if you would like.
> >
> > libio/iogetwline.c | 66 ++++++++++++++++++----------------------------
> > 1 file changed, 25 insertions(+), 41 deletions(-)
> >
> > diff --git a/libio/iogetwline.c b/libio/iogetwline.c
> > index 6a5adf92cb..e6f94e44e3 100644
> > --- a/libio/iogetwline.c
> > +++ b/libio/iogetwline.c
> > @@ -54,56 +54,40 @@ _IO_getwline_info (FILE *fp, wchar_t *buf, size_t n, wint_t delim,
> > _IO_fwide (fp, 1);
> > while (n != 0)
> > {
> > - ssize_t len = (fp->_wide_data->_IO_read_end
> > - - fp->_wide_data->_IO_read_ptr);
> > - if (len <= 0)
> > + wchar_t *read_ptr = fp->_wide_data->_IO_read_ptr;
> > + if (fp->_wide_data->_IO_read_end <= read_ptr)
> > {
> > - wint_t wc = __wuflow (fp);
> > - if (wc == WEOF)
> > + /* Refill the buffer without consuming anything, so that the
> > + delimiter is always handled by the block path below. */
> > + if (__wunderflow (fp) == WEOF)
> > {
> > - if (eof)
> > - *eof = wc;
> > + if (eof != NULL)
> > + *eof = WEOF;
> > break;
> > }
> > - if (wc == delim)
> > + continue;
> > + }
> > + size_t len = fp->_wide_data->_IO_read_end - read_ptr;
> > + if (len > n)
> > + len = n;
> > + wchar_t *t = wmemchr (read_ptr, delim, len);
> > + if (t != NULL)
> > + {
> > + len = t - read_ptr;
> > + if (extract_delim >= 0)
> > {
> > - if (extract_delim > 0)
> > - *ptr++ = wc;
> > - else if (extract_delim < 0)
> > - _IO_sputbackc (fp, wc);
> > + ++t;
> > if (extract_delim > 0)
> > ++len;
> > - return ptr - buf;
> > }
> > - *ptr++ = wc;
> > - n--;
> > - }
> > - else
> > - {
> > - wchar_t *t;
> > - if ((size_t) len >= n)
> > - len = n;
> > - t = wmemchr ((void *) fp->_wide_data->_IO_read_ptr, delim, len);
> > - if (t != NULL)
> > - {
> > - size_t old_len = ptr - buf;
> > - len = t - fp->_wide_data->_IO_read_ptr;
> > - if (extract_delim >= 0)
> > - {
> > - ++t;
> > - if (extract_delim > 0)
> > - ++len;
> > - }
> > - __wmemcpy ((void *) ptr, (void *) fp->_wide_data->_IO_read_ptr,
> > - len);
> > - fp->_wide_data->_IO_read_ptr = t;
> > - return old_len + len;
> > - }
> > - __wmemcpy ((void *) ptr, (void *) fp->_wide_data->_IO_read_ptr, len);
> > - fp->_wide_data->_IO_read_ptr += len;
> > - ptr += len;
> > - n -= len;
> > + __wmemcpy (ptr, read_ptr, len);
> > + fp->_wide_data->_IO_read_ptr = t;
> > + return (ptr - buf) + len;
> > }
> > + __wmemcpy (ptr, read_ptr, len);
> > + fp->_wide_data->_IO_read_ptr = read_ptr + len;
> > + ptr += len;
> > + n -= len;
> > }
> > return ptr - buf;
> > }
>
>
>
> --
> С Уважением,
> Абдулаев Ш. М.
@@ -54,56 +54,40 @@ _IO_getwline_info (FILE *fp, wchar_t *buf, size_t n, wint_t delim,
_IO_fwide (fp, 1);
while (n != 0)
{
- ssize_t len = (fp->_wide_data->_IO_read_end
- - fp->_wide_data->_IO_read_ptr);
- if (len <= 0)
+ wchar_t *read_ptr = fp->_wide_data->_IO_read_ptr;
+ if (fp->_wide_data->_IO_read_end <= read_ptr)
{
- wint_t wc = __wuflow (fp);
- if (wc == WEOF)
+ /* Refill the buffer without consuming anything, so that the
+ delimiter is always handled by the block path below. */
+ if (__wunderflow (fp) == WEOF)
{
- if (eof)
- *eof = wc;
+ if (eof != NULL)
+ *eof = WEOF;
break;
}
- if (wc == delim)
+ continue;
+ }
+ size_t len = fp->_wide_data->_IO_read_end - read_ptr;
+ if (len > n)
+ len = n;
+ wchar_t *t = wmemchr (read_ptr, delim, len);
+ if (t != NULL)
+ {
+ len = t - read_ptr;
+ if (extract_delim >= 0)
{
- if (extract_delim > 0)
- *ptr++ = wc;
- else if (extract_delim < 0)
- _IO_sputbackc (fp, wc);
+ ++t;
if (extract_delim > 0)
++len;
- return ptr - buf;
}
- *ptr++ = wc;
- n--;
- }
- else
- {
- wchar_t *t;
- if ((size_t) len >= n)
- len = n;
- t = wmemchr ((void *) fp->_wide_data->_IO_read_ptr, delim, len);
- if (t != NULL)
- {
- size_t old_len = ptr - buf;
- len = t - fp->_wide_data->_IO_read_ptr;
- if (extract_delim >= 0)
- {
- ++t;
- if (extract_delim > 0)
- ++len;
- }
- __wmemcpy ((void *) ptr, (void *) fp->_wide_data->_IO_read_ptr,
- len);
- fp->_wide_data->_IO_read_ptr = t;
- return old_len + len;
- }
- __wmemcpy ((void *) ptr, (void *) fp->_wide_data->_IO_read_ptr, len);
- fp->_wide_data->_IO_read_ptr += len;
- ptr += len;
- n -= len;
+ __wmemcpy (ptr, read_ptr, len);
+ fp->_wide_data->_IO_read_ptr = t;
+ return (ptr - buf) + len;
}
+ __wmemcpy (ptr, read_ptr, len);
+ fp->_wide_data->_IO_read_ptr = read_ptr + len;
+ ptr += len;
+ n -= len;
}
return ptr - buf;
}