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

[Nick Clifton <nickc@redhat.com>]

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".

Also will the LSB parameter ever be used ?  I assume that it is there
for a future extension to this function which might need it, but if
that is only theoretical, then maybe the parameter can be dropped
entirely and the code simplified.

> +  switch (info->mach)
> +    {
> +    case bfd_mach_arc_nps400:
> +    case bfd_mach_arc_arc700:
> +    case bfd_mach_arc_arc600:
> +      len = (major_opcode > 0xb) ? 2 : 4;
> +      break;
> +
> +    case bfd_mach_arc_arcv2:
> +      len = (major_opcode > 0x7) ? 2 : 4;
> +      break;
> +
> +    default:
> +      abort ();
> +    }
> +
> +  return len;

Since len is always returned without any further processing you
could save a bit of space in the function by moving the return 
instruction into the switch statement.  eg:

  case bfd_mach_arc_arc600:
    return (major_opcode > 0xb) ? 2 : 4;

Cheers
  Nick