[PATCH] Return 'int' rather than 'unsigned short' in avrdis_opcode

[PATCH] Return 'int' rather than 'unsigned short' in avrdis_opcode

[Yao Qi]

avrdis_opcode return type is unsigned short, but -1 at the end of
this function is returned.  Additionally, print_insn_avr doesn't
handle when -1 (in case of memory error) is returned from
avrdis_opcode.

This patch changes avrdis_opcode returning int indicating the error,
and adds a new argument for instruction we got on success.  The
opcode is 16-bit, so I change local variables type to uint16_t,
and include "bfd_stdint.h" as a result.  On memory error,
print_insn_avr returns -1, which is a common convention among most
of print_insn_$ARCH functions.

Regression tested with all targets enabled.  Is it OK?

opcodes:

2016-12-12  Yao Qi  <yao.qi@linaro.org>

	* avr-dis.c: Include "bfd_stdint.h"
	(avrdis_opcode): Change return type to int, add argument
	insn.  Set *INSN on success.
	(print_insn_avr): Check return value of avrdis_opcode, and
	return -1 on error.
---
 opcodes/avr-dis.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/opcodes/avr-dis.c b/opcodes/avr-dis.c
index 748cb2d..e32c2b6 100644
--- a/opcodes/avr-dis.c
+++ b/opcodes/avr-dis.c
@@ -25,6 +25,7 @@
 #include "dis-asm.h"
 #include "opintl.h"
 #include "libiberty.h"
+#include "bfd_stdint.h"
 
 struct avr_opcodes_s
 {
@@ -271,8 +272,11 @@ avr_operand (unsigned int insn, unsigned int insn2, unsigned int pc, int constra
     return ok;
 }
 
-static unsigned short
-avrdis_opcode (bfd_vma addr, disassemble_info *info)
+/* Read the opcode from ADDR.  Return 0 in success and save opcode
+   in *INSN, otherwise, return -1.  */
+
+static int
+avrdis_opcode (bfd_vma addr, disassemble_info *info, uint16_t *insn)
 {
   bfd_byte buffer[2];
   int status;
@@ -280,7 +284,10 @@ avrdis_opcode (bfd_vma addr, disassemble_info *info)
   status = info->read_memory_func (addr, buffer, 2, info);
 
   if (status == 0)
-    return bfd_getl16 (buffer);
+    {
+      *insn = bfd_getl16 (buffer);
+      return 0;
+    }
 
   info->memory_error_func (status, addr, info);
   return -1;
@@ -290,7 +297,7 @@ avrdis_opcode (bfd_vma addr, disassemble_info *info)
 int
 print_insn_avr (bfd_vma addr, disassemble_info *info)
 {
-  unsigned int insn, insn2;
+  uint16_t insn, insn2;
   const struct avr_opcodes_s *opcode;
   static unsigned int *maskptr;
   void *stream = info->stream;
@@ -341,7 +348,8 @@ print_insn_avr (bfd_vma addr, disassemble_info *info)
       initialized = 1;
     }
 
-  insn = avrdis_opcode (addr, info);
+  if (avrdis_opcode (addr, info, &insn)  != 0)
+    return -1;
 
   for (opcode = avr_opcodes, maskptr = avr_bin_masks;
        opcode->name;
@@ -374,7 +382,8 @@ print_insn_avr (bfd_vma addr, disassemble_info *info)
 
       if (opcode->insn_size > 1)
 	{
-	  insn2 = avrdis_opcode (addr + 2, info);
+	  if (avrdis_opcode (addr + 2, info, &insn2) != 0)
+	    return -1;
 	  cmd_len = 4;
 	}
 
-- 
1.9.1

Re: [PATCH] Return 'int' rather than 'unsigned short' in avrdis_opcode

[Yao Qi]

On Mon, Dec 12, 2016 at 9:50 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> avrdis_opcode return type is unsigned short, but -1 at the end of
> this function is returned.  Additionally, print_insn_avr doesn't
> handle when -1 (in case of memory error) is returned from
> avrdis_opcode.
>
> This patch changes avrdis_opcode returning int indicating the error,
> and adds a new argument for instruction we got on success.  The
> opcode is 16-bit, so I change local variables type to uint16_t,
> and include "bfd_stdint.h" as a result.  On memory error,
> print_insn_avr returns -1, which is a common convention among most
> of print_insn_$ARCH functions.
>
> Regression tested with all targets enabled.  Is it OK?
>
> opcodes:
>
> 2016-12-12  Yao Qi  <yao.qi@linaro.org>
>
>         * avr-dis.c: Include "bfd_stdint.h"
>         (avrdis_opcode): Change return type to int, add argument
>         insn.  Set *INSN on success.
>         (print_insn_avr): Check return value of avrdis_opcode, and
>         return -1 on error.

Ping.

-- 
Yao (齐尧)

Re: [PATCH] Return 'int' rather than 'unsigned short' in avrdis_opcode

[Alan Modra]

On Mon, Dec 19, 2016 at 03:51:10PM +0000, Yao Qi wrote:
> On Mon, Dec 12, 2016 at 9:50 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> > avrdis_opcode return type is unsigned short, but -1 at the end of
> > this function is returned.  Additionally, print_insn_avr doesn't
> > handle when -1 (in case of memory error) is returned from
> > avrdis_opcode.
> >
> > This patch changes avrdis_opcode returning int indicating the error,
> > and adds a new argument for instruction we got on success.  The
> > opcode is 16-bit, so I change local variables type to uint16_t,
> > and include "bfd_stdint.h" as a result.  On memory error,
> > print_insn_avr returns -1, which is a common convention among most
> > of print_insn_$ARCH functions.
> >
> > Regression tested with all targets enabled.  Is it OK?
> >
> > opcodes:
> >
> > 2016-12-12  Yao Qi  <yao.qi@linaro.org>
> >
> >         * avr-dis.c: Include "bfd_stdint.h"
> >         (avrdis_opcode): Change return type to int, add argument
> >         insn.  Set *INSN on success.
> >         (print_insn_avr): Check return value of avrdis_opcode, and
> >         return -1 on error.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] Return 'int' rather than 'unsigned short' in avrdis_opcode

[Yao Qi]

On Fri, Dec 23, 2016 at 2:02 PM, Alan Modra <amodra@gmail.com> wrote:
>> > opcodes:
>> >
>> > 2016-12-12  Yao Qi  <yao.qi@linaro.org>
>> >
>> >         * avr-dis.c: Include "bfd_stdint.h"
>> >         (avrdis_opcode): Change return type to int, add argument
>> >         insn.  Set *INSN on success.
>> >         (print_insn_avr): Check return value of avrdis_opcode, and
>> >         return -1 on error.
>
> OK.
>

Thanks for the review.  Patch is pushed in.

-- 
Yao (齐尧)