diff mbox

[RFA] Change read_alphacoff_dynamic_symtab to use gdb::def_vector

Message ID 87o9j0ci6d.fsf@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey April 3, 2018, 5:29 p.m. UTC 
  >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I am pretty sure that you could use gdb::byte_vector here and it
Simon> would be fine.

It needed one extra cast, but yeah.

Simon> Could you (if it's not too much trouble) remove the size variables (sym_secsize and
Simon> friends)?  This way, there's only one source of truth for the allocated size (the
Simon> vector size).  Also, the "ptr" suffix probably made sense when the variables were
Simon> pointers.  It would make sense to rename them, sym_secptr -> sym_sec, and so forth.

How's this?

Tom

commit 5f17c7b0f85ebb78d38c75884fbf45ae4b217546
Author: Tom Tromey <tom@tromey.com>
Date:   Fri Nov 10 19:52:33 2017 -0700

    Change read_alphacoff_dynamic_symtab to use gdb::byte_vector
    
    This changes read_alphacoff_dynamic_symtab to use gdb::byte_vector.
    This allows for the removal of some cleanups.
    
    Tested by the buildbot; though I don't know whether this code path is
    ever actually run.
    
    gdb/ChangeLog
    2018-04-03  Tom Tromey  <tom@tromey.com>
    
            * mipsread.c (read_alphacoff_dynamic_symtab): Use
            gdb::byte_vector.

Comments

Simon Marchi April 3, 2018, 5:38 p.m. UTC | #1 
On 2018-04-03 13:29, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> I am pretty sure that you could use gdb::byte_vector here and it
> Simon> would be fine.
> 
> It needed one extra cast, but yeah.
> 
> Simon> Could you (if it's not too much trouble) remove the size
> variables (sym_secsize and
> Simon> friends)?  This way, there's only one source of truth for the
> allocated size (the
> Simon> vector size).  Also, the "ptr" suffix probably made sense when
> the variables were
> Simon> pointers.  It would make sense to rename them, sym_secptr ->
> sym_sec, and so forth.
> 
> How's this?
> 
> Tom

LGTM, thanks!

Simon
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7e220ef895..5b5bed5720 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2018-04-03  Tom Tromey  <tom@tromey.com>
+
+	* mipsread.c (read_alphacoff_dynamic_symtab): Use
+	gdb::byte_vector.
+
 2018-04-01  Tom Tromey  <tom@tromey.com>
 
 	* rs6000-nat.c (rs6000_ptrace_ldinfo): Return a byte_vector.
diff --git a/gdb/mipsread.c b/gdb/mipsread.c
index af339e2e1f..7b6ec2e48d 100644
--- a/gdb/mipsread.c
+++ b/gdb/mipsread.c
@@ -180,24 +180,15 @@  read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
 {
   bfd *abfd = objfile->obfd;
   struct alphacoff_dynsecinfo si;
-  char *sym_secptr;
-  char *str_secptr;
-  char *dyninfo_secptr;
-  char *got_secptr;
-  bfd_size_type sym_secsize;
-  bfd_size_type str_secsize;
-  bfd_size_type dyninfo_secsize;
-  bfd_size_type got_secsize;
   int sym_count;
   int i;
   int stripped;
   Elfalpha_External_Sym *x_symp;
-  char *dyninfo_p;
-  char *dyninfo_end;
+  gdb_byte *dyninfo_p;
+  gdb_byte *dyninfo_end;
   int got_entry_size = 8;
   int dt_mips_local_gotno = -1;
   int dt_mips_gotsym = -1;
-  struct cleanup *cleanups;
 
   /* We currently only know how to handle alpha dynamic symbols.  */
   if (bfd_get_arch (abfd) != bfd_arch_alpha)
@@ -210,47 +201,28 @@  read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
       || si.dyninfo_sect == NULL || si.got_sect == NULL)
     return;
 
-  sym_secsize = bfd_get_section_size (si.sym_sect);
-  str_secsize = bfd_get_section_size (si.str_sect);
-  dyninfo_secsize = bfd_get_section_size (si.dyninfo_sect);
-  got_secsize = bfd_get_section_size (si.got_sect);
-  sym_secptr = (char *) xmalloc (sym_secsize);
-  cleanups = make_cleanup (xfree, sym_secptr);
-  str_secptr = (char *) xmalloc (str_secsize);
-  make_cleanup (xfree, str_secptr);
-  dyninfo_secptr = (char *) xmalloc (dyninfo_secsize);
-  make_cleanup (xfree, dyninfo_secptr);
-  got_secptr = (char *) xmalloc (got_secsize);
-  make_cleanup (xfree, got_secptr);
-
-  if (!bfd_get_section_contents (abfd, si.sym_sect, sym_secptr,
-				 (file_ptr) 0, sym_secsize))
-    {
-      do_cleanups (cleanups);
-      return;
-    }
-  if (!bfd_get_section_contents (abfd, si.str_sect, str_secptr,
-				 (file_ptr) 0, str_secsize))
-    {
-      do_cleanups (cleanups);
-      return;
-    }
-  if (!bfd_get_section_contents (abfd, si.dyninfo_sect, dyninfo_secptr,
-				 (file_ptr) 0, dyninfo_secsize))
-    {
-      do_cleanups (cleanups);
-      return;
-    }
-  if (!bfd_get_section_contents (abfd, si.got_sect, got_secptr,
-				 (file_ptr) 0, got_secsize))
-    {
-      do_cleanups (cleanups);
-      return;
-    }
+  gdb::byte_vector sym_sec (bfd_get_section_size (si.sym_sect));
+  gdb::byte_vector str_sec (bfd_get_section_size (si.str_sect));
+  gdb::byte_vector dyninfo_sec (bfd_get_section_size (si.dyninfo_sect));
+  gdb::byte_vector got_sec (bfd_get_section_size (si.got_sect));
+
+  if (!bfd_get_section_contents (abfd, si.sym_sect, sym_sec.data (),
+				 (file_ptr) 0, sym_sec.size ()))
+    return;
+  if (!bfd_get_section_contents (abfd, si.str_sect, str_sec.data (),
+				 (file_ptr) 0, str_sec.size ()))
+    return;
+  if (!bfd_get_section_contents (abfd, si.dyninfo_sect, dyninfo_sec.data (),
+				 (file_ptr) 0, dyninfo_sec.size ()))
+    return;
+  if (!bfd_get_section_contents (abfd, si.got_sect, got_sec.data (),
+				 (file_ptr) 0, got_sec.size ()))
+    return;
 
   /* Find the number of local GOT entries and the index for the
      first dynamic symbol in the GOT.  */
-  for (dyninfo_p = dyninfo_secptr, dyninfo_end = dyninfo_p + dyninfo_secsize;
+  for ((dyninfo_p = dyninfo_sec.data (),
+	dyninfo_end = dyninfo_p + dyninfo_sec.size ());
        dyninfo_p < dyninfo_end;
        dyninfo_p += sizeof (Elfalpha_External_Dyn))
     {
@@ -274,18 +246,15 @@  read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
 	}
     }
   if (dt_mips_local_gotno < 0 || dt_mips_gotsym < 0)
-    {
-      do_cleanups (cleanups);
-      return;
-    }
+    return;
 
   /* Scan all dynamic symbols and enter them into the minimal symbol
      table if appropriate.  */
-  sym_count = sym_secsize / sizeof (Elfalpha_External_Sym);
+  sym_count = sym_sec.size () / sizeof (Elfalpha_External_Sym);
   stripped = (bfd_get_symcount (abfd) == 0);
 
   /* Skip first symbol, which is a null dummy.  */
-  for (i = 1, x_symp = (Elfalpha_External_Sym *) sym_secptr + 1;
+  for (i = 1, x_symp = (Elfalpha_External_Sym *) sym_sec.data () + 1;
        i < sym_count;
        i++, x_symp++)
     {
@@ -298,9 +267,9 @@  read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
       enum minimal_symbol_type ms_type;
 
       strx = bfd_h_get_32 (abfd, (bfd_byte *) x_symp->st_name);
-      if (strx >= str_secsize)
+      if (strx >= str_sec.size ())
 	continue;
-      name = str_secptr + strx;
+      name = (char *) (str_sec.data () + strx);
       if (*name == '\0' || *name == '.')
 	continue;
 
@@ -341,11 +310,13 @@  read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
 	      int got_entry_offset =
 		(i - dt_mips_gotsym + dt_mips_local_gotno) * got_entry_size;
 
-	      if (got_entry_offset < 0 || got_entry_offset >= got_secsize)
+	      if (got_entry_offset < 0
+		  || got_entry_offset >= got_sec.size ())
 		continue;
 	      sym_value =
 		bfd_h_get_64 (abfd,
-			      (bfd_byte *) (got_secptr + got_entry_offset));
+			      (bfd_byte *) (got_sec.data ()
+					    + got_entry_offset));
 	      if (sym_value == 0)
 		continue;
 	    }
@@ -392,8 +363,6 @@  read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
 
       reader.record (name, sym_value, ms_type);
     }
-
-  do_cleanups (cleanups);
 }
 
 /* Initialization.  */