This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
- From: Andrew Burgess <andrew dot burgess at embecosm dot com>
- To: Nick Clifton <nickc at redhat dot com>
- Cc: binutils at sourceware dot org, Claudiu dot Zissulescu at synopsys dot com, Cupertino dot Miranda at synopsys dot com, noamca at mellanox dot com
- Date: Wed, 13 Apr 2016 15:40:58 +0100
- Subject: Re: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
- Authentication-results: sourceware.org; auth=none
- References: <cover dot 1460458691 dot git dot andrew dot burgess at embecosm dot com> <cb64992909da171def628e036ee5f6cc94e71058 dot 1460458691 dot git dot andrew dot burgess at embecosm dot com> <570E4A76 dot 5020504 at redhat dot 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