Re: [PATCH v2] disass: Add /x modifier to print offsets in hex

[Simon Marchi <simark@simark.ca>]

Hi,

That looks reasonable to me.

- This probably deserves a NEWS entry.
- There's probably some doc to update (doc/gdb.texinfo) for the affected
  commands.
- Could you add a test for this?  You probably don't need a new file,
  just enhance some existing basic disasm test.

> @@ -2535,7 +2540,7 @@ can be shown using \"show listsize\"."));
>
>    c = add_com ("disassemble", class_vars, disassemble_command, _("\
>  Disassemble a specified section of memory.\n\
> -Usage: disassemble[/m|/r|/s] START [, END]\n\
> +Usage: disassemble[/m|/r|/s|/r] START [, END]\n\
>  Default is the function surrounding the pc of the selected frame.\n\
>  \n\
>  With a /s modifier, source lines are included (if available).\n\
> @@ -2551,6 +2556,8 @@ in favor of /s.\n\
>  \n\
>  With a /r modifier, raw instructions in hex are included.\n\
>  \n\
> +With a /x modifier, offsets are printed as hex.\n\

Really a nit, but just above we say "in hex", so it would be nice to be
consistent and say "in hex" here too.

> @@ -250,7 +251,10 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
>  	   the offset takes the place of the "+" here.  */
>  	if (offset >= 0)
>  	  m_uiout->text ("+");
> -	m_uiout->field_signed ("offset", offset);
> +	snprintf(offset_buf, sizeof(offset_buf),
> +                 flags & DISASSEMBLY_HEX_OFFSET ? "0x%x" : "%d",
> +                 offset);
> +	m_uiout->field_string("offset", offset_buf);

You could skip the temporary buffer with:

	m_uiout->field_fmt ("offset",
			    ((flags & DISASSEMBLY_HEX_OFFSET) != 0
			     ? "0x%x" : "%d"),
			    offset);

Note that the GNU coding style requires a space before parentheses in
function (and function-like) calls.

Simon