This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function


* Nick Clifton <nickc@redhat.com> [2016-04-13 14:32:38 +0100]:

> Hi Andrew,
> 
> > opcodes/ChangeLog:
> > 
> > 	* arc-dis.c (arc_insn_length): New function.
> > 	(print_insn_arc): Use arc_insn_length.
> 
> Approved - please apply, but ...
> 
> > +static int
> > +arc_insn_length (bfd_byte msb, bfd_byte lsb ATTRIBUTE_UNUSED,
> > +                 struct disassemble_info *info)
> 
> Would this function ever return a negative value ?  I assume not, so
> it would make sense for its return type to be "unsigned int".

I'm not sure is the answer.  I agree that arc_insn_length will never
return a negative, however, the return value from arc_insn_length is
used to prime a variable that is then the return value for
print_insn_arc, which is also defined to return 'int', and is part of
the disassembler API, and does return a negative value if there's an
error.

It was this relationship that originally lead me to make
arc_insn_length return an 'int'.

Given that it will only ever return small positive integers there
should be no problem making it return an unsigned value then casting
to int in print_insn_arc - would this be preferred?

Thanks,
Andrew


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]