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

[Nick Clifton <nickc@redhat.com>]

Hi Andrew,

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

Yes.  Or (better IMHO) just explicitly set the return value for 
print_insn_arc based upon testing the return value from arc_insn_length.
Ie something like:

  len = arc_insn_length (...);
  if (len == 0)
    return -1;

(I am assuming here that a returned length of zero should never happen
with a valid instruction, and can therefore be used as an error return
value).

Cheers
  Nick