Re: [PATCH v2 1/2] [ARC] Fix handling of cpu=... disassembler option value

[Pedro Alves <palves at redhat dot com>]

On 06/19/2017 03:40 PM, Anton Kolesov wrote:
> Changes in V2:
> 
> * Use disassembler_options_cmp to compare string instead of using strtok, which
>   required string duplication.
> * Add a second patch that introduces usage of FOR_EACH_DISASSEMBLER_OPTION to
>   ARC.

Thanks for doing this.

Both patches look good to me (but note I'm not an opcodes maintainer).

>From the nit department:

- I'm not sure on coding standard on the bfd side, but on the gdb side
  you'd write "disassembler_options_cmp (...) == 0" instead of 
  !disassembler_options_cmp, since the return value isn't a boolean.

- Spurious leading space after TAB in the ChangeLog entry.

It's kind of a shame IMHO to fix bugs like these without adding
regression tests, to prevent regressions (and to prove it
works :-)).  Could something be easily added to
testsuite/binutils-all/arc/ perhaps?

Both ARM and Power test these FOR_EACH_DISASSEMBLER_OPTION-related paths
using GDB's testsuite.  Grep for "set disassembler-options" under
gdb/testsuite/gdb.arch/.  But IIUC, ARC doesn't support
"set disassembler-options" yet, right?

Thanks,
Pedro Alves