[v2] libio: Use __wunderflow in _IO_getwline_info

Message ID cae084fb8b4f0225f6fc97c10d07d6cec94c25bc.1789979069.git.ashamil435@gmail.com (mailing list archive)
State New
Delegated to: Andreas Schwab
Headers
Series [v2] 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
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
redhat-pt-bot/TryBot-32bit success Build for i686
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

Shamil Abdulaev Sept. 21, 2026, 8:24 a.m. UTC
  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>
---

v2: Check for an empty buffer with a pointer comparison and make len
size_t, as suggested by Andreas Schwab.
 libio/iogetwline.c | 73 ++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 41 deletions(-)
  

Comments

Shamil Abdulaev Sept. 23, 2026, 8:07 p.m. UTC | #1
PING

пн, 21 сент. 2026 г., 11:24 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>
> ---
>
> v2: Check for an empty buffer with a pointer comparison and make len
> size_t, as suggested by Andreas Schwab.
>  libio/iogetwline.c | 73 ++++++++++++++++++++--------------------------
>  1 file changed, 32 insertions(+), 41 deletions(-)
>
> diff --git a/libio/iogetwline.c b/libio/iogetwline.c
> index 6a5adf92cb..971793881e 100644
> --- a/libio/iogetwline.c
> +++ b/libio/iogetwline.c
> @@ -48,62 +48,53 @@ _IO_getwline_info (FILE *fp, wchar_t *buf, size_t n,
> wint_t delim,
>                    int extract_delim, wint_t *eof)
>  {
>    wchar_t *ptr = buf;
> +
>    if (eof != NULL)
>      *eof = 0;
>    if (__builtin_expect (fp->_mode, 1) == 0)
>      _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
>
>
  
Shamil Abdulaev Sept. 28, 2026, 7:40 a.m. UTC | #2
PING!

ср, 23 сент. 2026 г. в 23:07, Shamil Abdulaev <ashamil435@gmail.com>:

> PING
>
> пн, 21 сент. 2026 г., 11:24 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>
>> ---
>>
>> v2: Check for an empty buffer with a pointer comparison and make len
>> size_t, as suggested by Andreas Schwab.
>>  libio/iogetwline.c | 73 ++++++++++++++++++++--------------------------
>>  1 file changed, 32 insertions(+), 41 deletions(-)
>>
>> diff --git a/libio/iogetwline.c b/libio/iogetwline.c
>> index 6a5adf92cb..971793881e 100644
>> --- a/libio/iogetwline.c
>> +++ b/libio/iogetwline.c
>> @@ -48,62 +48,53 @@ _IO_getwline_info (FILE *fp, wchar_t *buf, size_t n,
>> wint_t delim,
>>                    int extract_delim, wint_t *eof)
>>  {
>>    wchar_t *ptr = buf;
>> +
>>    if (eof != NULL)
>>      *eof = 0;
>>    if (__builtin_expect (fp->_mode, 1) == 0)
>>      _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
>>
>>
  
Adhemerval Zanella Sept. 29, 2026, 1:10 p.m. UTC | #3
On 21/09/26 05:24, 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

I wonder whether _IO_sputbackc is the correct call here, and not the
wide _IO_sputbackwc. The rest looks okay (maybe avoid adding the extra
new lines that are just aesthetic). 

> 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>
> ---
> 
> v2: Check for an empty buffer with a pointer comparison and make len
> size_t, as suggested by Andreas Schwab.
>  libio/iogetwline.c | 73 ++++++++++++++++++++--------------------------
>  1 file changed, 32 insertions(+), 41 deletions(-)
> 
> diff --git a/libio/iogetwline.c b/libio/iogetwline.c
> index 6a5adf92cb..971793881e 100644
> --- a/libio/iogetwline.c
> +++ b/libio/iogetwline.c
> @@ -48,62 +48,53 @@ _IO_getwline_info (FILE *fp, wchar_t *buf, size_t n, wint_t delim,
>  		   int extract_delim, wint_t *eof)
>  {
>    wchar_t *ptr = buf;
> +
>    if (eof != NULL)
>      *eof = 0;
>    if (__builtin_expect (fp->_mode, 1) == 0)
>      _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;
>  }
  
Andreas Schwab Sept. 29, 2026, 1:36 p.m. UTC | #4
On Sep 29 2026, Adhemerval Zanella Netto wrote:

> On 21/09/26 05:24, 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
>
> I wonder whether _IO_sputbackc is the correct call here, and not the
> wide _IO_sputbackwc.

That's one of the points of the patch, it removes that erroneous call.
I think it would make sense to remove the extract_delim parameter from
_IO_getwline and fold _IO_getwline_info into it.
  

Patch

diff --git a/libio/iogetwline.c b/libio/iogetwline.c
index 6a5adf92cb..971793881e 100644
--- a/libio/iogetwline.c
+++ b/libio/iogetwline.c
@@ -48,62 +48,53 @@  _IO_getwline_info (FILE *fp, wchar_t *buf, size_t n, wint_t delim,
 		   int extract_delim, wint_t *eof)
 {
   wchar_t *ptr = buf;
+
   if (eof != NULL)
     *eof = 0;
   if (__builtin_expect (fp->_mode, 1) == 0)
     _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;
 }