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]

objdump:avoid some copying


Hi,
Whilst backporting a patch to fix buffer overrun, I notice that
objdump_sprintf does unnecessary copying, and doesn't remember any sizes
from one line to the next.  This patch fixes that.

The common case is that there will be enough space in the buffer, and
in that case vsnprintf will write directly there. When there is insufficient
space, we extend the buffer by at least the amount vsnprintf told us it'd
need, and then print again.

Keeping the buffer live throughtout disassemble_data means we don't keep
rediscovering our buffer is not long enough.

tested on i686-pc-linux-gnu, ok?

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

2004-03-04  Nathan Sidwell  <nathan@codesourcery.com>

	* objdump.c (struct SFILE): Replace current pointer with pos
	offset, rename size to alloc.
	(objdump_sprintf): Avoid unnecessary copies in the common case
	(disassemble_bytes): Keep sfile live throughout the
	function. Adjust usage appropriately.

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	4 Mar 2004 16:47:08 -0000
*************** show_line (bfd *abfd, asection *section,
*** 1142,1149 ****
  typedef struct
  {
    char *buffer;
!   size_t size;
!   char *current;
  } SFILE;
  
  /* sprintf to a "stream".  */
--- 1142,1149 ----
  typedef struct
  {
    char *buffer;
!   size_t pos;
!   size_t alloc;
  } SFILE;
  
  /* sprintf to a "stream".  */
*************** typedef struct
*** 1151,1189 ****
  static int
  objdump_sprintf (SFILE *f, const char *format, ...)
  {
-   char *buf;
    size_t n;
    va_list args;
  
!   va_start (args, format);
! 
!   vasprintf (&buf, format, args);
! 
!   if (buf == NULL)
      {
        va_end (args);
-       fatal (_("Out of virtual memory"));
-     }
- 
-   n = strlen (buf);
  
!   while ((size_t) ((f->buffer + f->size) - f->current) < n + 1)
!     {
!       size_t curroff;
! 
!       curroff = f->current - f->buffer;
!       f->size *= 2;
!       f->buffer = xrealloc (f->buffer, f->size);
!       f->current = f->buffer + curroff;
      }
! 
!   memcpy (f->current, buf, n);
!   f->current += n;
!   f->current[0] = '\0';
! 
!   free (buf);
! 
!   va_end (args);
    return n;
  }
  
--- 1151,1175 ----
  static int
  objdump_sprintf (SFILE *f, const char *format, ...)
  {
    size_t n;
    va_list args;
  
!   while (1)
      {
+       size_t space = f->alloc - f->pos;
+   
+       va_start (args, format);
+       n = vsnprintf (f->buffer + f->pos, space, format, args);
        va_end (args);
  
!       if (space > n)
! 	break;
!       
!       f->alloc = (f->alloc + n) * 2;
!       f->buffer = xrealloc (f->buffer, f->alloc);
      }
!   f->pos += n;
!   
    return n;
  }
  
*************** disassemble_bytes (struct disassemble_in
*** 1243,1252 ****
--- 1229,1243 ----
    int skip_addr_chars;
    bfd_vma addr_offset;
    int opb = info->octets_per_byte;
+   SFILE sfile;
  
    aux = (struct objdump_disasm_info *) info->application_data;
    section = aux->sec;
  
+   sfile.alloc = 120;
+   sfile.buffer = xmalloc (sfile.alloc);
+   sfile.pos = 0;
+   
    if (insns)
      octets_per_line = 4;
    else
*************** disassemble_bytes (struct disassemble_in
*** 1311,1317 ****
        else
  	{
  	  char buf[50];
- 	  SFILE sfile;
  	  int bpc = 0;
  	  int pb = 0;
  
--- 1302,1307 ----
*************** disassemble_bytes (struct disassemble_in
*** 1344,1352 ****
  
  	  if (insns)
  	    {
! 	      sfile.size = 120;
! 	      sfile.buffer = xmalloc (sfile.size);
! 	      sfile.current = sfile.buffer;
  	      info->fprintf_func = (fprintf_ftype) objdump_sprintf;
  	      info->stream = (FILE *) &sfile;
  	      info->bytes_per_line = 0;
--- 1334,1340 ----
  
  	  if (insns)
  	    {
! 	      sfile.pos = 0;
  	      info->fprintf_func = (fprintf_ftype) objdump_sprintf;
  	      info->stream = (FILE *) &sfile;
  	      info->bytes_per_line = 0;
*************** disassemble_bytes (struct disassemble_in
*** 1371,1379 ****
  		octets_per_line = info->bytes_per_line;
  	      if (octets < 0)
  		{
! 		  if (sfile.current != sfile.buffer)
  		    printf ("%s\n", sfile.buffer);
- 		  free (sfile.buffer);
  		  break;
  		}
  	    }
--- 1359,1366 ----
  		octets_per_line = info->bytes_per_line;
  	      if (octets < 0)
  		{
! 		  if (sfile.pos)
  		    printf ("%s\n", sfile.buffer);
  		  break;
  		}
  	    }
*************** disassemble_bytes (struct disassemble_in
*** 1447,1457 ****
  
  	  if (! insns)
  	    printf ("%s", buf);
! 	  else
! 	    {
! 	      printf ("%s", sfile.buffer);
! 	      free (sfile.buffer);
! 	    }
  
  	  if (prefix_addresses
  	      ? show_raw_insn > 0
--- 1434,1441 ----
  
  	  if (! insns)
  	    printf ("%s", buf);
! 	  else if (sfile.pos)
! 	    printf ("%s", sfile.buffer);
  
  	  if (prefix_addresses
  	      ? show_raw_insn > 0
*************** disassemble_bytes (struct disassemble_in
*** 1558,1563 ****
--- 1542,1549 ----
  
        addr_offset += octets / opb;
      }
+ 
+   free (sfile.buffer);
  }
  
  static void

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