Re: Patch to change ARM register name set

[Nick Clifton <nickc@cygnus.com>]

Hi Ian,

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

OK - that is actually what I wanted to do in the first place, but I
did not know if there were lots of utilities out there that use the
current disassembler() function, so I did not want to change it.

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

Good point - I will generate a new patch and submit it later on today.

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

OK - although I had hoped that this might turn into a more generic
type of command line switch where the text could be passed to other
components of objdump, not just its disassembler.

Cheers
	Nick