[PATCH 3/4] strings: Cleanup print_unicode_buffer

Alice Carlotti alice.carlotti@arm.com
Mon Nov 10 18:07:17 GMT 2025


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.  

> 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.  */

> And if c cannot
> be less than 0xc0 anymore (perhaps as a result of patch 2), I think it
> would still be a good idea to add a respective assertion.

> 
> Jan
> 
> > -	      num_found = 0;
> > -	      continue;
> > -	    }
> >  	}
> >  
> >        if (num_found == 0)
> > 
> 


More information about the Binutils mailing list