[PATCH 1/4] strings: Simplify print_unicode_stream_body
Jan Beulich
jbeulich@suse.com
Fri Nov 7 10:21:56 GMT 2025
On 25.09.2025 20:07, Alice Carlotti wrote:
> 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.
This looks as if it was still pending.
> --- 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;
> + }
Please make the final "break" explicit, too. Okay with that adjustment.
Jan
More information about the Binutils
mailing list