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

[Andrew Burgess <andrew.burgess@embecosm.com>]

* 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