[PATCHv2] opcodes/x86: fix minor missed styling case

[Jiang, Haochen]

> -----Original Message-----
> From: Cui, Lili <lili.cui@intel.com>
> Sent: Thursday, July 25, 2024 9:53 AM
> To: Andrew Burgess <aburgess@redhat.com>; Beulich, Jan <JBeulich@suse.com>
> Cc: H.J. Lu <hjl.tools@gmail.com>; Jiang, Haochen <haochen.jiang@intel.com>;
> binutils@sourceware.org
> Subject: RE: [PATCHv2] opcodes/x86: fix minor missed styling case
> 
> 
> > > On 24.07.2024 04:31, Cui, Lili wrote:
> > >>>> I noticed that the x86 instruction:
> > >>>>
> > >>>>   sar    $0x1,%rsi
> > >>>>
> > >>>> would fail to style the '$0x1' as an immediate.  This commit
> > >>>> fixes that
> > case.
> > >>>>
> > >>
> > >> I'm afraid it is not a bug, it is to distinguish between the two
> > >> formats
> > below.
> > >>
> > >> sar    r/m8, 1
> > >> sar    r/m8, imm8
> > >
> > > It is a bug, but it also is a bug to change 1 to 0x1, as that way
> > > said distinction goes away. (I also don't immediately see how the
> > > code change alone would pass the testsuite; I'm pretty sure we have
> > > expectations which would have required adjustment, which would have
> > > made more obvious that the change wants doing differently.)
> >
> > You are correct, I got sloppy, and I apologise.
> >
> > Thanks to everyone who pointed out the mistake.
> >
> > Here's an update, _fully_ tested patch.
> >
> > Thanks,
> > Andrew
> >
> > ---
> >
> > commit ffe8ab67ab81ae24105433145ca6ee40c3fb92e2
> > Author: Andrew Burgess <aburgess@redhat.com>
> > Date:   Tue Jul 23 17:10:22 2024 +0100
> >
> >     opcodes/x86: fix minor missed styling case
> >
> >     I noticed that the x86 instruction:
> >
> >       sar    $1,%rsi
> >
> >     would fail to style the '$0x1' as an immediate.  This commit fixes
> >     that case.
> >
> > diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c index
> > bc141f31770..59ec771369a 100644
> > --- a/opcodes/i386-dis.c
> > +++ b/opcodes/i386-dis.c
> > @@ -12415,9 +12415,9 @@ OP_I (instr_info *ins, int bytemode, int sizeflag)
> >        break;
> >      case const_1_mode:
> >        if (ins->intel_syntax)
> > -	oappend (ins, "1");
> > +	oappend_with_style (ins, "1", dis_style_immediate);
> >        else
> > -	oappend (ins, "$1");
> > +	oappend_with_style (ins, "$1", dis_style_immediate);
> >        return true;
> >      default:
> >        oappend (ins, INTERNAL_DISASSEMBLER_ERROR);
> 
> I am a little confused, how does this patch fix this case in the comment, the
> output is not changed before and after the patch.

Confused +1. It seems Changelog and the current output doesn't match or I
misunderstood something.

Thx,
Haochen

> 
> Thanks,
> Lili.