Re: objdump: i386 nasm-like output

[Alan Modra <amodra at bigpond dot net dot au>]

On Fri, Jul 27, 2001 at 12:47:21PM +0200, Matthias Kramm wrote:
> 
> I discovered -m i386:intel does a subset of what I wanted, at least it
> swaps the registers ($0x10,%ax -> %ax,$0x10).
> 
> Also, I wrote a patch to get rid of the '%'s, '$'s and change 
> [bx,di] to [bx+di], which is actually sufficient to achieve
> an output in the format described above.
> Are my changes to objdump (actually, libopcodes) interesting 
> for a broader audience?

Yes thanks!

> Programmers coming from dos or windows 
> systems are mostly used to nasm-like assemblers, like tasm or masm 
> which use exactly the format above.
> 
> The patch in question is applied. I was wondering whether 
> the decision how to disassemble should be implemented in 
> a different way. At the moment, the patch simply overloads
> the i386:intel machine format. 
> Should one...
> (a) introduce a new machine format (objdump -m)?
> (b) use an disassembly option, which selects, for intel-architectures,
>     which format to use? (objdump -M)
> (c) Leave libopcodes alone, and only change objdump.c (I.e. filter the
>     output from the disassembler-function), introducing
> 	a new objdump command line option?

None of the above.  I would say your patch consists of bug fixes for
the current intel syntax mode, and therefore you don't need a new mode
or an `intel_nasm' variable.  Unfortunately, your patch doesn't meet
http://www.gnu.org/prep/standards_toc.html, and you have neglected to
provide a ChangeLog entry.  I'll highlight some of the coding errors
below, and also make some other suggestions.  Please address these
problems wherever they occur (I'm only giving you one occurrence of
each error) and resubmit your patch.

> +  names64=standard_names64;

need spaces either side of `='

>  {
> +  if(intel_nasm)

add space after `if', s/intel_nasm/intel_syntax/

>    sprintf (scratchbuf, "%%st(%d)", rm);
> +    else
> +  sprintf (scratchbuf, "st(%d)", rm);
>    oappend (scratchbuf);

or even better, get rid of the `if' entirely, using
`oappend (scratchbuf + intel_syntax);'

> @@ -3409,8 +3487,8 @@
>  
>    op &= mask;
>    scratchbuf[0] = '$';
> -  print_operand_value (scratchbuf + !intel_syntax, 1, op);
> -  oappend (scratchbuf);
> +  print_operand_value (scratchbuf + (intel_nasm || !intel_syntax), 1, op);
> +  oappend (scratchbuf + intel_nasm);

This hunk becomes unnecessary when you get rid of intel_nasm.  In any case,
it's a bit convoluted.

> +  oappend (sreg[reg]+intel_nasm);

spaces around `+'