[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