This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Misc cleanups


Ian Lance Taylor wrote:

Also, now that I look more closely at your SKIP_ZEROES change, I don't
like it.  SKIP_ZEROES has a value which you should use.  You shouldn't
test it with #ifdef.
it doesn't, it used #if :)

It seems to me that you are adding a special case for setting
SKIP_ZEROES to 0 to disable it, which I don't understand.  I don't
Currently setting SKIP_ZEROES to zero would result on the disassembler
always skipping no zeros! (And consequently, never advancing.)
Having 0 mean 'don't skip' seemed an obvious meaning to me.

You're right that one could use -z, and a long sequence of zero
bytes is nearly never a valid program, but those don't persuade me
as arguments to prevent a port from doing what I did.

Even if long strings of zeroes do occur frequently, testing
SKIP_ZEROES using #ifdef doesn't feel right to me.  First, why not
just set SKIP_ZEROES to a large value?  Second, if you really resist
that for some reason, just test the value in code, rather than using
#ifdef.  The resulting code will be more or less the same when
optimized, and will be easier to understand than code using #ifdef.
Setting it to a large value strikes me as broken - you'd be forcing an
unnecessary scan. Unfortunately when I put the test into regular code,
it causes gcc to say things like 'comparison of value >= 0 is always
true', which is annoying.

This reworking keeps 'z' within a smaller block (IMHO more readable)
and also fixes the admittedly unlikely problem of never advancing if
SKIP_ZEROES < 4. Is this reworking ok?

nathan

--
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

2004-02-06  Nathan Sidwell  <nathan@codesourcery.com>

	* objdump.c (disassemble_bytes):  Use zero SKIP_ZEROS to mean
	never skip. Ensure we always advance.

Index: objdump.c
===================================================================
RCS file: /cvs/src/src/binutils/objdump.c,v
retrieving revision 1.86
diff -c -3 -p -r1.86 objdump.c
*** objdump.c	22 Dec 2003 10:49:59 -0000	1.86
--- objdump.c	6 Feb 2004 16:47:31 -0000
*************** disassemble_bytes (struct disassemble_in
*** 1281,1314 ****
    addr_offset = start_offset;
    while (addr_offset < stop_offset)
      {
-       bfd_vma z;
        int octets = 0;
        bfd_boolean need_nl = FALSE;
  
!       /* If we see more than SKIP_ZEROES octets of zeroes, we just
! 	 print `...'.  */
!       for (z = addr_offset * opb; z < stop_offset * opb; z++)
! 	if (data[z] != 0)
! 	  break;
        if (! disassemble_zeroes
  	  && (info->insn_info_valid == 0
! 	      || info->branch_delay_insns == 0)
! 	  && (z - addr_offset * opb >= SKIP_ZEROES
! 	      || (z == stop_offset * opb &&
! 		  z - addr_offset * opb < SKIP_ZEROES_AT_END)))
  	{
! 	  printf ("\t...\n");
  
! 	  /* If there are more nonzero octets to follow, we only skip
! 	     zeroes in multiples of 4, to try to avoid running over
! 	     the start of an instruction which happens to start with
! 	     zero.  */
! 	  if (z != stop_offset * opb)
! 	    z = addr_offset * opb + ((z - addr_offset * opb) &~ 3);
! 
! 	  octets = z - addr_offset * opb;
  	}
!       else
  	{
  	  char buf[50];
  	  SFILE sfile;
--- 1281,1320 ----
    addr_offset = start_offset;
    while (addr_offset < stop_offset)
      {
        int octets = 0;
        bfd_boolean need_nl = FALSE;
  
! #if SKIP_ZEROES
        if (! disassemble_zeroes
  	  && (info->insn_info_valid == 0
! 	      || info->branch_delay_insns == 0))
  	{
! 	  bfd_vma z;
! 	  
! 	  /* If we see more than SKIP_ZEROES octets of zeroes, we just
! 	     print `...'.  */
! 	  for (z = addr_offset * opb; z < stop_offset * opb; z++)
! 	    if (data[z] != 0)
! 	      break;
! 	  if ((z - addr_offset * opb >= SKIP_ZEROES
! 	       || (z == stop_offset * opb &&
! 		   z - addr_offset * opb < SKIP_ZEROES_AT_END)))
! 	    {
! 	      printf ("\t...\n");
  
! 	      /* If there are more nonzero octets to follow, we only
! 	     	 skip zeroes in multiples of 4, to try to avoid
! 	     	 running over the start of an instruction which
! 	     	 happens to start with zero.  */
! 	      if (z != stop_offset * opb)
! 		z = addr_offset * opb + ((z - addr_offset * opb) &~ 3);
! 	      
! 	      octets = z - addr_offset * opb;
! 	    }
  	}
! #endif
!       
!       if (!octets)
  	{
  	  char buf[50];
  	  SFILE sfile;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]