public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA] Change read_alphacoff_dynamic_symtab to use gdb::def_vector
@ 2018-04-01 16:26 Tom Tromey
  2018-04-01 18:39 ` Simon Marchi
  0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2018-04-01 16:26 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes read_alphacoff_dynamic_symtab to use gdb::def_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-03-31  Tom Tromey  <tom@tromey.com>

	* mipsread.c (read_alphacoff_dynamic_symtab): Use
	gdb::def_vector.
---
 gdb/ChangeLog  |  5 +++++
 gdb/mipsread.c | 64 ++++++++++++++++++----------------------------------------
 2 files changed, 25 insertions(+), 44 deletions(-)

diff --git a/gdb/mipsread.c b/gdb/mipsread.c
index af339e2e1f..046a25bd50 100644
--- a/gdb/mipsread.c
+++ b/gdb/mipsread.c
@@ -180,10 +180,6 @@ 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;
@@ -197,7 +193,6 @@ read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
   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)
@@ -214,43 +209,28 @@ read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
   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,
+  gdb::def_vector<char> sym_secptr (sym_secsize);
+  gdb::def_vector<char> str_secptr (str_secsize);
+  gdb::def_vector<char> dyninfo_secptr (dyninfo_secsize);
+  gdb::def_vector<char> got_secptr (got_secsize);
+
+  if (!bfd_get_section_contents (abfd, si.sym_sect, sym_secptr.data (),
 				 (file_ptr) 0, sym_secsize))
-    {
-      do_cleanups (cleanups);
-      return;
-    }
-  if (!bfd_get_section_contents (abfd, si.str_sect, str_secptr,
+    return;
+  if (!bfd_get_section_contents (abfd, si.str_sect, str_secptr.data (),
 				 (file_ptr) 0, str_secsize))
-    {
-      do_cleanups (cleanups);
-      return;
-    }
-  if (!bfd_get_section_contents (abfd, si.dyninfo_sect, dyninfo_secptr,
+    return;
+  if (!bfd_get_section_contents (abfd, si.dyninfo_sect, dyninfo_secptr.data (),
 				 (file_ptr) 0, dyninfo_secsize))
-    {
-      do_cleanups (cleanups);
-      return;
-    }
-  if (!bfd_get_section_contents (abfd, si.got_sect, got_secptr,
+    return;
+  if (!bfd_get_section_contents (abfd, si.got_sect, got_secptr.data (),
 				 (file_ptr) 0, got_secsize))
-    {
-      do_cleanups (cleanups);
-      return;
-    }
+    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_secptr.data (),
+	dyninfo_end = dyninfo_p + dyninfo_secsize);
        dyninfo_p < dyninfo_end;
        dyninfo_p += sizeof (Elfalpha_External_Dyn))
     {
@@ -274,10 +254,7 @@ 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.  */
@@ -285,7 +262,7 @@ read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
   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_secptr.data () + 1;
        i < sym_count;
        i++, x_symp++)
     {
@@ -300,7 +277,7 @@ read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
       strx = bfd_h_get_32 (abfd, (bfd_byte *) x_symp->st_name);
       if (strx >= str_secsize)
 	continue;
-      name = str_secptr + strx;
+      name = str_secptr.data () + strx;
       if (*name == '\0' || *name == '.')
 	continue;
 
@@ -345,7 +322,8 @@ read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
 		continue;
 	      sym_value =
 		bfd_h_get_64 (abfd,
-			      (bfd_byte *) (got_secptr + got_entry_offset));
+			      (bfd_byte *) (got_secptr.data ()
+					    + got_entry_offset));
 	      if (sym_value == 0)
 		continue;
 	    }
@@ -392,8 +370,6 @@ read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
 
       reader.record (name, sym_value, ms_type);
     }
-
-  do_cleanups (cleanups);
 }
 
 /* Initialization.  */
-- 
2.13.6

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFA] Change read_alphacoff_dynamic_symtab to use gdb::def_vector
  2018-04-01 16:26 [RFA] Change read_alphacoff_dynamic_symtab to use gdb::def_vector Tom Tromey
@ 2018-04-01 18:39 ` Simon Marchi
  2018-04-03 17:29   ` Tom Tromey
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi @ 2018-04-01 18:39 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

> @@ -214,43 +209,28 @@ read_alphacoff_dynamic_symtab (minimal_symbol_reader &reader,
>    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,
> +  gdb::def_vector<char> sym_secptr (sym_secsize);
> +  gdb::def_vector<char> str_secptr (str_secsize);
> +  gdb::def_vector<char> dyninfo_secptr (dyninfo_secsize);
> +  gdb::def_vector<char> got_secptr (got_secsize);

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

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

Simon

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFA] Change read_alphacoff_dynamic_symtab to use gdb::def_vector
  2018-04-01 18:39 ` Simon Marchi
@ 2018-04-03 17:29   ` Tom Tromey
  2018-04-03 17:38     ` Simon Marchi
  0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2018-04-03 17:29 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>>>>> "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.

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.  */

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFA] Change read_alphacoff_dynamic_symtab to use gdb::def_vector
  2018-04-03 17:29   ` Tom Tromey
@ 2018-04-03 17:38     ` Simon Marchi
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Marchi @ 2018-04-03 17:38 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-04-03 17:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-01 16:26 [RFA] Change read_alphacoff_dynamic_symtab to use gdb::def_vector Tom Tromey
2018-04-01 18:39 ` Simon Marchi
2018-04-03 17:29   ` Tom Tromey
2018-04-03 17:38     ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).