diff mbox series

Use memchr in _IO_new_file_xsputn

Message ID 20260611174205.3283678-1-bero@lindev.ch (mailing list archive)
State Changes Requested
Headers
Series Use memchr in _IO_new_file_xsputn |

Checks

Context Check Description
redhat-pt-bot/TryBot-32bit fail Patch caused testsuite regressions

Commit Message

Bernhard Rosenkränzer June 11, 2026, 5:42 p.m. UTC 
  Use memchr instead of implementing a bytewise lookup, this should be
slightly faster and more readable

Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch>
---
 libio/fileops.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

Comments

Adhemerval Zanella Netto June 11, 2026, 7:06 p.m. UTC | #1 
On 11/06/26 14:42, Bernhard Rosenkränzer wrote:
> Use memchr instead of implementing a bytewise lookup, this should be

I think you meant memrchr here.

> slightly faster and more readable
> 
> Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch>
> ---
>  libio/fileops.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/libio/fileops.c b/libio/fileops.c
> index 9348d7c3a1..a614e7c528 100644
> --- a/libio/fileops.c
> +++ b/libio/fileops.c
> @@ -1290,15 +1290,11 @@ _IO_new_file_xsputn (FILE *f, const void *data, size_t n)
>        count = f->_IO_buf_end - f->_IO_write_ptr;
>        if (count >= n)
>  	{
> -	  const char *p;
> -	  for (p = s + n; p > s; )
> +	  const char *p = memrchr (s, '\n', n);

You should use __memrchr within glibc, otherwise you will see a elf/check-localplt
failure.

> +	  if (p != NULL)
>  	    {
> -	      if (*--p == '\n')
> -		{
> -		  count = p - s + 1;
> -		  must_flush = 1;
> -		  break;
> -		}
> + 	      count = p - s + 1;
> +	      must_flush = 1;
>  	    }
>  	}
>      }
Florian Weimer July 10, 2026, 9:28 a.m. UTC | #2 
* Adhemerval Zanella Netto:

> On 11/06/26 14:42, Bernhard Rosenkränzer wrote:
>> Use memchr instead of implementing a bytewise lookup, this should be
>
> I think you meant memrchr here.
>
>> slightly faster and more readable
>> 
>> Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch>
>> ---
>>  libio/fileops.c | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>> 
>> diff --git a/libio/fileops.c b/libio/fileops.c
>> index 9348d7c3a1..a614e7c528 100644
>> --- a/libio/fileops.c
>> +++ b/libio/fileops.c
>> @@ -1290,15 +1290,11 @@ _IO_new_file_xsputn (FILE *f, const void *data, size_t n)
>>        count = f->_IO_buf_end - f->_IO_write_ptr;
>>        if (count >= n)
>>  	{
>> -	  const char *p;
>> -	  for (p = s + n; p > s; )
>> +	  const char *p = memrchr (s, '\n', n);
>
> You should use __memrchr within glibc, otherwise you will see a
> elf/check-localplt failure.

Bernhard, would you be able to send updated patch?

Thanks,
Florian
diff mbox series

Patch

diff --git a/libio/fileops.c b/libio/fileops.c
index 9348d7c3a1..a614e7c528 100644
--- a/libio/fileops.c
+++ b/libio/fileops.c
@@ -1290,15 +1290,11 @@  _IO_new_file_xsputn (FILE *f, const void *data, size_t n)
       count = f->_IO_buf_end - f->_IO_write_ptr;
       if (count >= n)
 	{
-	  const char *p;
-	  for (p = s + n; p > s; )
+	  const char *p = memrchr (s, '\n', n);
+	  if (p != NULL)
 	    {
-	      if (*--p == '\n')
-		{
-		  count = p - s + 1;
-		  must_flush = 1;
-		  break;
-		}
+ 	      count = p - s + 1;
+	      must_flush = 1;
 	    }
 	}
     }