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

[Fam Zheng <fam@euphon.net>]

On Thu, Sep 24, 2020 at 3:25 AM Simon Marchi <simark@simark.ca> wrote:
>
>
> Hi,

Hi Simon,

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

All make sense!

>
> > @@ -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.

Okay, will change.

>
> > @@ -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.

Thanks, will send v3 later addressing above.

Fam

>
> Simon