[PATCH 1/4] strings: Simplify print_unicode_stream_body

Alice Carlotti alice.carlotti@arm.com
Thu Sep 25 18:07:40 GMT 2025


This patch is essentially a full rewrite of the function, with a roughly
threefold reduction in length and no functional changes.  It includes
the following benefits:

- Combine two near-identical loops into a single loop.

- Eliminate the use of putback_buf.  We only ever need to reparse the
  last read byte in a failed multibyte sequence, because we know that
  the first byte didn't start a valid sequence, and any other bytes were
  continuation bytes.

- Use gotos to allow end of string actions to be implemented in a single
  location (now outside of the loop).  This also has the added benefits
  of eliminating the tail call, and allowing the byte c to be preserved
  directly for reparsing.

- Skip parsing multibyte sequences when --unicode=invalid is set.

- Add a call to is_valid_utf8.  This is currently redundant, but will
  become important once stricter validity checks are added.


diff --git a/binutils/strings.c b/binutils/strings.c
index 38da6381edf58f098238389a9ab2bd74d6770a5a..a188564db0c7995e5dcdb2e0830e7d08b5ddeae4 100644
--- a/binutils/strings.c
+++ b/binutils/strings.c
@@ -906,16 +906,8 @@ print_unicode_buffer (const char *            filename,
 
 static int
 get_unicode_byte (FILE *          stream,
-		  unsigned char * putback,
-		  unsigned int *  num_putback,
 		  unsigned int *  num_read)
 {
-  if (* num_putback > 0)
-    {
-      * num_putback = * num_putback - 1;
-      return putback [* num_putback];
-    }
-
   * num_read = * num_read + 1;
 
 #if defined(HAVE_GETC_UNLOCKED) && HAVE_DECL_GETC_UNLOCKED
@@ -931,8 +923,6 @@ static void
 print_unicode_stream_body (const char *     filename,
 			   file_ptr         address,
 			   FILE *           stream,
-			   unsigned char *  putback_buf,
-			   unsigned int     num_putback,
 			   unsigned char *  print_buf)
 {
   /* It would be nice if we could just read the stream into a buffer
@@ -940,273 +930,111 @@ print_unicode_stream_body (const char *     filename,
      might be huge or it might time-locked (eg stdin).  So instead
      we go one byte at a time...  */
 
-  file_ptr start_point = 0;
   unsigned int num_read = 0;
   unsigned int num_chars = 0;
   unsigned int num_print = 0;
-  int c = 0;
-
-  /* Find a series of string_min characters.  Put them into print_buf.  */
-  do
-    {
-      if (num_chars >= string_min)
-	break;
-
-      c = get_unicode_byte (stream, putback_buf, & num_putback, & num_read);
-      if (c == EOF)
-	break;
-
-      if (! STRING_ISGRAPHIC (c))
-	{
-	  num_chars = num_print = 0;
-	  continue;
-	}
-
-      if (num_chars == 0)
-	start_point = num_read - 1;
-
-      if (c < 127)
-	{
-	  print_buf[num_print] = c;
-	  num_chars ++;
-	  num_print ++;
-	  continue;
-	}
-
-      if (c < 0xc0)
-	{
-	  num_chars = num_print = 0;
-	  continue;
-	}
-
-      /* We *might* have a UTF-8 sequence.  Time to start peeking.  */
-      char utf8[4];
-
-      utf8[0] = c;
-      c = get_unicode_byte (stream, putback_buf, & num_putback, & num_read);
-      if (c == EOF)
-	break;
-      utf8[1] = c;
-
-      if ((utf8[1] & 0xc0) != 0x80)
-	{
-	  /* Invalid UTF-8.  */
-	  putback_buf[num_putback++] = utf8[1];
-	  num_chars = num_print = 0;
-	  continue;
-	}
-      else if ((utf8[0] & 0x20) == 0)
-	{
-	  /* A valid 2-byte UTF-8 encoding.  */
-	  if (unicode_display == unicode_invalid)
-	    {
-	      putback_buf[num_putback++] = utf8[1];
-	      num_chars = num_print = 0;
-	    }
-	  else
-	    {
-	      print_buf[num_print ++] = utf8[0];
-	      print_buf[num_print ++] = utf8[1];
-	      num_chars ++;
-	    }
-	  continue;
-	}
-
-      c = get_unicode_byte (stream, putback_buf, & num_putback, & num_read);
-      if (c == EOF)
-	break;
-      utf8[2] = c;
-
-      if ((utf8[2] & 0xc0) != 0x80)
-	{
-	  /* Invalid UTF-8.  */
-	  putback_buf[num_putback++] = utf8[2];
-	  putback_buf[num_putback++] = utf8[1];
-	  num_chars = num_print = 0;
-	  continue;
-	}
-      else if ((utf8[0] & 0x10) == 0)
-	{
-	  /* A valid 3-byte UTF-8 encoding.  */
-	  if (unicode_display == unicode_invalid)
-	    {
-	      putback_buf[num_putback++] = utf8[2];
-	      putback_buf[num_putback++] = utf8[1];
-	      num_chars = num_print = 0;
-	    }
-	  else
-	    {
-	      print_buf[num_print ++] = utf8[0];
-	      print_buf[num_print ++] = utf8[1];
-	      print_buf[num_print ++] = utf8[2];
-	      num_chars ++;
-	    }
-	  continue;
-	}
-
-      c = get_unicode_byte (stream, putback_buf, & num_putback, & num_read);
-      if (c == EOF)
-	break;
-      utf8[3] = c;
-
-      if ((utf8[3] & 0xc0) != 0x80)
-	{
-	  /* Invalid UTF-8.  */
-	  putback_buf[num_putback++] = utf8[3];
-	  putback_buf[num_putback++] = utf8[2];
-	  putback_buf[num_putback++] = utf8[1];
-	  num_chars = num_print = 0;
-	}
-      /* We have a valid 4-byte UTF-8 encoding.  */
-      else if (unicode_display == unicode_invalid)
-	{
-	  putback_buf[num_putback++] = utf8[3];
-	  putback_buf[num_putback++] = utf8[1];
-	  putback_buf[num_putback++] = utf8[2];
-	  num_chars = num_print = 0;
-	}
-      else
-	{
-	  print_buf[num_print ++] = utf8[0];
-	  print_buf[num_print ++] = utf8[1];
-	  print_buf[num_print ++] = utf8[2];
-	  print_buf[num_print ++] = utf8[3];
-	  num_chars ++;
-	}
-    }
-  while (1);
-
-  if (num_chars >= string_min)
-    {
-      /* We know that we have string_min valid characters in print_buf,
-	 and there may be more to come in the stream.  Start displaying
-	 them.  */
-
-      print_filename_and_address (filename, address + start_point);
-
-      unsigned int i;
-      for (i = 0; i < num_print;)
-	{
-	  if (print_buf[i] < 127)
-	    putchar (print_buf[i++]);
-	  else
-	    i += display_utf8_char (print_buf + i);
-	}
-
-      /* OK so now we have to start read unchecked bytes.  */
-
-      /* Find a series of string_min characters.  Put them into print_buf.  */
-      do
-	{
-	  c = get_unicode_byte (stream, putback_buf, & num_putback, & num_read);
-	  if (c == EOF)
-	    break;
-
-	  if (! STRING_ISGRAPHIC (c))
-	    break;
-
-	  if (c < 127)
-	    {
-	      putchar (c);
-	      continue;
-	    }
-
-	  if (c < 0xc0)
-	    break;
-
-	  /* We *might* have a UTF-8 sequence.  Time to start peeking.  */
-	  unsigned char utf8[4];
-
-	  utf8[0] = c;
-	  c = get_unicode_byte (stream, putback_buf, & num_putback, & num_read);
-	  if (c == EOF)
-	    break;
-	  utf8[1] = c;
-
-	  if ((utf8[1] & 0xc0) != 0x80)
-	    {
-	      /* Invalid UTF-8.  */
-	      putback_buf[num_putback++] = utf8[1];
-	      break;
-	    }
-	  else if ((utf8[0] & 0x20) == 0)
-	    {
-	      /* Valid 2-byte UTF-8.  */
-	      if (unicode_display == unicode_invalid)
-		{
-		  putback_buf[num_putback++] = utf8[1];
-		  break;
-		}
-	      else
-		{
-		  (void) display_utf8_char (utf8);
-		  continue;
-		}
-	    }
-
-	  c = get_unicode_byte (stream, putback_buf, & num_putback, & num_read);
-	  if (c == EOF)
-	    break;
-	  utf8[2] = c;
-
-	  if ((utf8[2] & 0xc0) != 0x80)
-	    {
-	      /* Invalid UTF-8.  */
-	      putback_buf[num_putback++] = utf8[2];
-	      putback_buf[num_putback++] = utf8[1];
-	      break;
-	    }
-	  else if ((utf8[0] & 0x10) == 0)
-	    {
-	      /* Valid 3-byte UTF-8.  */
-	      if (unicode_display == unicode_invalid)
-		{
-		  putback_buf[num_putback++] = utf8[2];
-		  putback_buf[num_putback++] = utf8[1];
-		  break;
-		}
-	      else
-		{
-		  (void) display_utf8_char (utf8);
-		  continue;
-		}
-	    }
-
-	  c = get_unicode_byte (stream, putback_buf, & num_putback, & num_read);
-	  if (c == EOF)
-	    break;
-	  utf8[3] = c;
-
-	  if ((utf8[3] & 0xc0) != 0x80)
-	    {
-	      /* Invalid UTF-8.  */
-	      putback_buf[num_putback++] = utf8[3];
-	      putback_buf[num_putback++] = utf8[2];
-	      putback_buf[num_putback++] = utf8[1];
-	      break;
-	    }
-	  else if (unicode_display == unicode_invalid)
-	    {
-	      putback_buf[num_putback++] = utf8[3];
-	      putback_buf[num_putback++] = utf8[2];
-	      putback_buf[num_putback++] = utf8[1];
-	      break;
-	    }
-	  else
-	    /* A valid 4-byte UTF-8 encoding.  */
-	    (void) display_utf8_char (utf8);
-	}
-      while (1);
-
-      if (output_separator)
-	fputs (output_separator, stdout);
-      else
-	putchar ('\n');
-    }
-
-  if (c != EOF)
-    /* FIXME: Using tail recursion here is lazy, but it works.  */
-    print_unicode_stream_body (filename, address + num_read, stream, putback_buf, num_putback, print_buf);
+  int c;
+
+  restart_next:
+  c = get_unicode_byte (stream, & num_read);
+
+  restart_last:
+  if (num_chars >= string_min)
+    {
+      if (output_separator)
+	fputs (output_separator, stdout);
+      else
+	putchar ('\n');
+    }
+
+  if (c == EOF)
+    return;
+
+  num_chars = 0;
+  num_print = 0;
+  file_ptr start_point = num_read - 1;
+
+  /* Find a series of string_min characters.  Put them into print_buf.  */
+  for (;; c = get_unicode_byte (stream, & num_read))
+    {
+      if (num_chars == string_min)
+	{
+	  /* We know that we have string_min valid characters in print_buf,
+	     and there may be more to come in the stream.  Display them now,
+	     then display the rest as we find them.  */
+
+	  print_filename_and_address (filename, address + start_point);
+
+	  for (unsigned int i = 0; i < num_print;)
+	    {
+	      if (print_buf[i] < 127)
+		putchar (print_buf[i++]);
+	      else
+		i += display_utf8_char (print_buf + i);
+	    }
+	}
+
+      if (c == EOF)
+	goto restart_last;
+
+      if (! STRING_ISGRAPHIC (c))
+	  goto restart_next;
+
+      if (c < 127)
+	{
+	  num_chars ++;
+	  if (num_chars > string_min)
+	      putchar (c);
+	  else
+	    print_buf[num_print ++] = c;
+	  continue;
+	}
+      else if (c < 0xc0 || unicode_display == unicode_invalid)
+	goto restart_next;
+
+      /* We *might* have a UTF-8 sequence.  Time to start peeking.  */
+      unsigned int utf8_len;
+      switch (c & 0x30)
+	{
+	case 0x00:
+	case 0x10:
+	  utf8_len = 2;
+	  break;
+	case 0x20:
+	  utf8_len = 3;
+	  break;
+	default: /* 0x30 */
+	  utf8_len = 4;
+	}
+
+      unsigned char utf8[4];
+      utf8[0] = c;
+      for (unsigned int i = 1; i < utf8_len; i++)
+	{
+	  c = get_unicode_byte (stream, & num_read);
+	  if (c == EOF)
+	    goto restart_last;
+
+	  if ((c & 0xc0) != 0x80)
+	    {
+	      /* Invalid UTF-8.  */
+	      goto restart_last;
+	    }
+	  utf8[i] = c;
+	}
+      if (is_valid_utf8 (utf8, utf8_len) == 0)
+	{
+	  /* Invalid UTF-8, but last byte read was a continuation byte.  */
+	  goto restart_next;
+	}
+      /* Valid UTF-8.  */
+      num_chars++;
+      if (num_chars > string_min)
+	display_utf8_char (utf8);
+      else
+	for (unsigned int i = 0; i < utf8_len; i++)
+	  print_buf[num_print++] = utf8[i];
+    }
 }
 
 /* Display strings read in from STREAM.  Treat any UTF-8 encoded characters
@@ -1233,11 +1061,8 @@ print_unicode_stream (const char * filename,
   size_t amt = string_min;
   amt = (4 * amt) + 1;
   unsigned char * print_buf = xmalloc (amt);
-  /* We should never have to put back more than 4 bytes.  */
-  unsigned char putback_buf[5];
-  unsigned int num_putback = 0;
 
-  print_unicode_stream_body (filename, address, stream, putback_buf, num_putback, print_buf);
+  print_unicode_stream_body (filename, address, stream, print_buf);
   free (print_buf);
 }
 



More information about the Binutils mailing list