From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Clifton To: ian@zembu.com Cc: binutils@sourceware.cygnus.com Subject: Re: Patch to change ARM register name set Date: Thu, 01 Jul 1999 00:00:00 -0000 Message-id: <199906150957.KAA18137@pathia.cygnus.co.uk> X-SW-Source: 1999-q2/msg00249.html 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