Re: Patch to change ARM register name set

[Ian Lance Taylor <ian@zembu.com>]

   Date: Mon, 14 Jun 1999 15:09:19 +0100
   From: Nick Clifton <nickc@cygnus.com>

     Does anyone have any comments on the patch below ?  It changes the
     ARM disassembler so that by default it uses the APCS register naming
     scheme, rather than the ISA register naming scheme.  It also
     introduces a new command line switch to objdump: -M or --target-data,
     which takes an argument which can be processed in a target specific
     manner.  If the target is the ARM then the text of the -M switch is
     checked to see if it is a register name set selector
     ("standard_names" or "apcs_names") and if so it chooses the
     appropriate name set. 

I don't care what ARM register names are used.

I do have some comments on the rest of the patch.  I think adding an
option which can be passed to the disassembler makes sense.  I can
think of two other cases where that would make sense, both for the
ix86: getting Intel syntax and disassembling in 16 bit mode.  Both are
currently done using the -m option to select a machine type, but this
leads to a meaningless bfd_mach_i386_i386_intel_syntax in
bfd/cpu-i386.c.

However, adding a new disassembler2 function does not make sense to
me.  Adding new functions because you have a new parameter is an
approach that leads to ABI complexity.  It's part of what makes the
Win32 ABI so difficult to work with.  Don't tread that path.  Instead,
just change the existing function.

If you must add a new function, a name like disassembler2 is a poor
choice.  Instead, use a meaningful name, such as, perhaps,
disassembler_select (that's not very good either, I admit).

However, in this particular case we should not use a new function at
all.  Instead, the target specific data should be stored in a field of
struct disassemble_info.  This easily permits each disassembler to
examine that field and make its own choices.  After all, the code in
your disassembler2 function which is ARM specific should not be in
disassemble.c at all.  It should be in arm-dis.c.

In objdump.c, I think target_data is a poor choice of names.  This is
an option to the disassembler.  objdump does many things other than
disassemble, so using a word like `target_data' to describe something
which is specific to the disassembler is misleading.  Something like
`disassembler_options' makes more sense to me.  This should be changed
in the suggested documentation as well.

Ian