Re: [PATCH 3/3] opcodes/i386: partially implement disassembler style support

[Jan Beulich <jbeulich@suse.com>]

On 16.02.2022 21:53, Andrew Burgess via Binutils wrote:
> @@ -9456,12 +9456,18 @@ print_insn (bfd_vma pc, instr_info *ins)
>  	{
>  	  name = prefix_name (ins, priv.the_buffer[0], priv.orig_sizeflag);
>  	  if (name != NULL)
> -	    (*ins->info->fprintf_func) (ins->info->stream, "%s", name);
> +	    (*ins->info->fprintf_styled_func)
> +	      (ins->info->stream, dis_style_mnemonic, "%s", name);
>  	  else
>  	    {
>  	      /* Just print the first byte as a .byte instruction.  */
> -	      (*ins->info->fprintf_func) (ins->info->stream, ".byte 0x%x",
> -				     (unsigned int) priv.the_buffer[0]);
> +	      (*ins->info->fprintf_styled_func)
> +		(ins->info->stream, dis_style_mnemonic, ".byte");

Perhaps better have dis_style_directive for this? It's certainly not
an insn mnemonic.

> +	      (*ins->info->fprintf_styled_func)
> +		(ins->info->stream, dis_style_text, " ");
> +	      (*ins->info->fprintf_styled_func)
> +		(ins->info->stream, dis_style_immediate, "0x%x",
> +		 (unsigned int) priv.the_buffer[0]);

I wonder if the naming (dis_style_immediate) isn't misleading. As per
the comment next to its definition it really appears to mean any kind
of number (like is the case here), not just immediate operands of
instructions. Hence maybe dis_style_number (as replacement for or in
addition to dis_style_immediate)?

> @@ -9497,10 +9503,15 @@ print_insn (bfd_vma pc, instr_info *ins)
>        /* Handle ins->prefixes before fwait.  */
>        for (i = 0; i < ins->fwait_prefix && ins->all_prefixes[i];
>  	   i++)
> -	(*ins->info->fprintf_func) (ins->info->stream, "%s ",
> -				    prefix_name (ins, ins->all_prefixes[i],
> -						 sizeflag));
> -      (*ins->info->fprintf_func) (ins->info->stream, "fwait");
> +	{
> +	  (*ins->info->fprintf_styled_func)
> +	    (ins->info->stream, dis_style_mnemonic, "%s",
> +	     prefix_name (ins, ins->all_prefixes[i], sizeflag));
> +	  (*ins->info->fprintf_styled_func)
> +	    (ins->info->stream, dis_style_mnemonic, " ");

Does the style matter for blanks? If so, why "mnemonic" here, but ...

> @@ -9744,13 +9759,17 @@ print_insn (bfd_vma pc, instr_info *ins)
>  	if (name == NULL)
>  	  abort ();
>  	prefix_length += strlen (name) + 1;
> -	(*ins->info->fprintf_func) (ins->info->stream, "%s ", name);
> +	(*ins->info->fprintf_styled_func)
> +	  (ins->info->stream, dis_style_mnemonic, "%s", name);
> +	(*ins->info->fprintf_styled_func)
> +	  (ins->info->stream, dis_style_text, " ");

... "text" here? If the style didn't matter, a single call (as it was
before) would seem to suffice in both cases.

Jan