[PATCH 3/4] strings: Cleanup print_unicode_buffer

Jan Beulich jbeulich@suse.com
Tue Nov 11 08:01:32 GMT 2025


On 10.11.2025 19:07, Alice Carlotti wrote:
> On Fri, Nov 07, 2025 at 11:32:13AM +0100, Jan Beulich wrote:
>> On 25.09.2025 20:08, Alice Carlotti wrote:
>>> diff --git a/binutils/strings.c b/binutils/strings.c
>>> index b39001711cd9747571c20cef3ede6a50a4a294de..4244911bc6528208d98290e108d1d8e70fd94928 100644
>>> --- a/binutils/strings.c
>>> +++ b/binutils/strings.c
>>> @@ -829,7 +829,7 @@ print_unicode_buffer (const char *            filename,
>>>  
>>>        if (c > 126)
>>>  	{
>>> -	  if (c < 0xc0)
>>> +	  if (unicode_display == unicode_invalid)
>>
>> It's not immediately clear to me why this would be a valid transformation.
> 
> The original check is redundant because it is duplicated by the first check in
> is_valid_utf8.  Unhelpfully, this call is hidden in the gap between the two
> hunks in this patch; I've copied the missing lines below.  The check was
> redundant even before the earlier patches in this series.  

Can a perhaps terse form of this become the patch description then, please?

>> The description being empty doesn't help, and at the very least I'd expect
>> ...
>>
>>>  	    {
>>>  	      num_found = 0;
>>>  	      continue;
>             }
>  
>           if ((char_len = is_valid_utf8 (buffer + i, buflen - i, NULL)) == 0)
>             {
>               char_len = 1;
>>> @@ -841,13 +841,6 @@ print_unicode_buffer (const char *            filename,
>>>  	      num_found = 0;
>>>  	      continue;
>>>  	    }
>>> -
>>> -	  if (unicode_display == unicode_invalid)
>>> -	    {
>>> -	      /* We have found a valid UTF-8 character, but we treat it as non-graphic.  */
>>
>> ... the comment to be retained (in edited form of course).
> 
> I moved this check earlier since can bail out before doing complicated
> multibyte parsing, so the original comment is no longer relevant.  I could
> provide a replacement comment saying:
> 
> + 	      /* Don't bother parsing multibyte UTF-8 characters when we won't
> + 		 accept them anyway.  */

Is "accept" the right word (and not e.g. "print")? Other than that - fine with me.

Jan


More information about the Binutils mailing list