This is the mail archive of the
binutils@sourceware.cygnus.com
mailing list for the binutils project.
Re: Patch to change ARM register name set
- To: ian@zembu.com
- Subject: Re: Patch to change ARM register name set
- From: Nick Clifton <nickc@cygnus.com>
- Date: Tue, 15 Jun 1999 10:57:16 +0100
- CC: binutils@sourceware.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