[RFA,06/22] Introduce scoped_minimal_symbol_reader

Message ID 1474949330-4307-7-git-send-email-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Sept. 27, 2016, 4:08 a.m. UTC
  This patch introduced scoped_minimal_symbol_reader, a RAII-based class
which replaces the current make_cleanup_discard_minimal_symbols.

2016-09-26  Tom Tromey  <tom@tromey.com>

	* xcoffread.c (xcoff_initial_scan): Use
	scoped_minimal_symbol_reader.
	* mipsread.c (mipscoff_symfile_read): Use
	scoped_minimal_symbol_reader.
	* minsyms.h (scoped_minimal_symbol_reader): New class.
	(init_minimal_symbol_collection)
	(make_cleanup_discard_minimal_symbols, install_minimal_symbols):
	Don't declare.
	* minsyms.c (scoped_minimal_symbol_reader): Renamed from
	init_minimal_symbol_collection, turned into constructor.
	(~scoped_minimal_symbol_reader): Renamed from
	do_discard_minimal_symbols_cleanup, turned into destructor.
	(make_cleanup_discard_minimal_symbols): Remove.
	(scoped_minimal_symbol_reader::install): Rename form
	install_minimal_symbols.
	* mdebugread.c (elfmdebug_build_psymtabs): Use
	scoped_minimal_symbol_reader.
	* machoread.c (macho_symfile_read): Use
	scoped_minimal_symbol_reader.
	* elfread.c (elf_read_minimal_symbols): Use
	scoped_minimal_symbol_reader.
	* dbxread.c (dbx_symfile_read): Use scoped_minimal_symbol_reader.
	* coffread.c (coff_symfile_read): Use
	scoped_minimal_symbol_reader.
---
 gdb/ChangeLog    | 27 +++++++++++++++++++++++++++
 gdb/coffread.c   | 10 +++-------
 gdb/dbxread.c    |  5 ++---
 gdb/elfread.c    |  6 +++---
 gdb/machoread.c  |  5 ++---
 gdb/mdebugread.c |  7 ++-----
 gdb/minsyms.c    | 56 ++++++++++++++++++++++----------------------------------
 gdb/minsyms.h    | 43 ++++++++++++++++++++++++-------------------
 gdb/mipsread.c   |  7 ++-----
 gdb/xcoffread.c  |  5 ++---
 10 files changed, 89 insertions(+), 82 deletions(-)
  

Comments

Pedro Alves Oct. 6, 2016, 1:09 a.m. UTC | #1
On 09/27/2016 05:08 AM, Tom Tromey wrote:
> -/* Prepare to start collecting minimal symbols.  This should be called
> -   by a symbol reader to initialize the minimal symbol module.
> -   Currently, minimal symbol table creation is not reentrant; it
> -   relies on global (static) variables in minsyms.c.  */
> +// An RAII-based object that is used to record minimal symbols while
> +// they are being read.

GCC eschews using // comments, to avoid ending up with a mixbag of
styles.  Maybe we should follow suit?

> +class scoped_minimal_symbol_reader

Seems like this is growing to be _the_ reader in following patches?
I'm wondering whether the "scoped_" prefix will make sense going
forward.

Thanks,
Pedro Alves
  
Tom Tromey Oct. 6, 2016, 3:37 p.m. UTC | #2
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> GCC eschews using // comments, to avoid ending up with a mixbag of
Pedro> styles.  Maybe we should follow suit?

I think // comments are just better.
However, I don't really care.  I can adapt the patches.

Tom
  
Tom Tromey Oct. 10, 2016, 11:06 p.m. UTC | #3
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> GCC eschews using // comments, to avoid ending up with a mixbag
Pedro> of styles.  Maybe we should follow suit?

I don't remember - did you want me to change this or not?
I prefer "//" comments but it's up to you.

>> +class scoped_minimal_symbol_reader

Pedro> Seems like this is growing to be _the_ reader in following patches?
Pedro> I'm wondering whether the "scoped_" prefix will make sense going
Pedro> forward.

I made this change.  Thank goodness for git filter-branch.

Tom
  
Pedro Alves Oct. 10, 2016, 11:25 p.m. UTC | #4
On 10/11/2016 12:06 AM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> GCC eschews using // comments, to avoid ending up with a mixbag
> Pedro> of styles.  Maybe we should follow suit?
> 
> I don't remember - did you want me to change this or not?
> I prefer "//" comments but it's up to you.

I'd prefer to change it for the sake of consistency.  A single comment
in a different style in between other comments stands out as odd to me.

For tail-end single-line comments, like

  } // namespace gdb

it doesn't look that odd to me.  AFAICS, these are the cases
where gcc uses // too, for the most part.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b73b26e..81aeec9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,32 @@ 
 2016-09-26  Tom Tromey  <tom@tromey.com>
 
+	* xcoffread.c (xcoff_initial_scan): Use
+	scoped_minimal_symbol_reader.
+	* mipsread.c (mipscoff_symfile_read): Use
+	scoped_minimal_symbol_reader.
+	* minsyms.h (scoped_minimal_symbol_reader): New class.
+	(init_minimal_symbol_collection)
+	(make_cleanup_discard_minimal_symbols, install_minimal_symbols):
+	Don't declare.
+	* minsyms.c (scoped_minimal_symbol_reader): Renamed from
+	init_minimal_symbol_collection, turned into constructor.
+	(~scoped_minimal_symbol_reader): Renamed from
+	do_discard_minimal_symbols_cleanup, turned into destructor.
+	(make_cleanup_discard_minimal_symbols): Remove.
+	(scoped_minimal_symbol_reader::install): Rename form
+	install_minimal_symbols.
+	* mdebugread.c (elfmdebug_build_psymtabs): Use
+	scoped_minimal_symbol_reader.
+	* machoread.c (macho_symfile_read): Use
+	scoped_minimal_symbol_reader.
+	* elfread.c (elf_read_minimal_symbols): Use
+	scoped_minimal_symbol_reader.
+	* dbxread.c (dbx_symfile_read): Use scoped_minimal_symbol_reader.
+	* coffread.c (coff_symfile_read): Use
+	scoped_minimal_symbol_reader.
+
+2016-09-26  Tom Tromey  <tom@tromey.com>
+
 	* valprint.c (generic_emit_char, count_next_character)
 	(generic_printstr): Update.
 	* charset.c (struct wchar_iterator): Move to charset.h.
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 830deb5..e504cde 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -568,7 +568,7 @@  coff_symfile_read (struct objfile *objfile, int symfile_flags)
   unsigned int num_symbols;
   int symtab_offset;
   int stringtab_offset;
-  struct cleanup *back_to, *cleanup_minimal_symbols;
+  struct cleanup *back_to;
   int stabstrsize;
   
   info = (struct coff_symfile_info *) objfile_data (objfile,
@@ -645,8 +645,7 @@  coff_symfile_read (struct objfile *objfile, int symfile_flags)
   if (val < 0)
     error (_("\"%s\": can't get string table"), name);
 
-  init_minimal_symbol_collection ();
-  cleanup_minimal_symbols = make_cleanup_discard_minimal_symbols ();
+  scoped_minimal_symbol_reader reader;
 
   /* Now that the executable file is positioned at symbol table,
      process it and define symbols accordingly.  */
@@ -656,7 +655,7 @@  coff_symfile_read (struct objfile *objfile, int symfile_flags)
   /* Install any minimal symbols that have been collected as the
      current minimal symbols for this objfile.  */
 
-  install_minimal_symbols (objfile);
+  reader.install (objfile);
 
   if (pe_file)
     {
@@ -699,9 +698,6 @@  coff_symfile_read (struct objfile *objfile, int symfile_flags)
 	}
     }
 
-  /* Free the installed minimal symbol data.  */
-  do_cleanups (cleanup_minimal_symbols);
-
   bfd_map_over_sections (abfd, coff_locate_sections, (void *) info);
 
   if (info->stabsects)
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index d5a9587..a0e1cac 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -558,8 +558,7 @@  dbx_symfile_read (struct objfile *objfile, int symfile_flags)
   free_pending_blocks ();
   back_to = make_cleanup (really_free_pendings, 0);
 
-  init_minimal_symbol_collection ();
-  make_cleanup_discard_minimal_symbols ();
+  scoped_minimal_symbol_reader reader;
 
   /* Read stabs data from executable file and define symbols.  */
 
@@ -572,7 +571,7 @@  dbx_symfile_read (struct objfile *objfile, int symfile_flags)
   /* Install any minimal symbols that have been collected as the current
      minimal symbols for this objfile.  */
 
-  install_minimal_symbols (objfile);
+  reader.install (objfile);
 
   do_cleanups (back_to);
 }
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 84355cf..e1ae92a 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1049,8 +1049,8 @@  elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
       return;
     }
 
-  init_minimal_symbol_collection ();
-  back_to = make_cleanup_discard_minimal_symbols ();
+  scoped_minimal_symbol_reader reader;
+  back_to = make_cleanup (null_cleanup, NULL);
 
   /* Allocate struct to keep track of the symfile.  */
   dbx = XCNEW (struct dbx_symfile_info);
@@ -1149,7 +1149,7 @@  elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
      responsibility to install them.  "mdebug" appears to be the only one
      which will do this.  */
 
-  install_minimal_symbols (objfile);
+  reader.install (objfile);
   do_cleanups (back_to);
 
   if (symtab_create_debug)
diff --git a/gdb/machoread.c b/gdb/machoread.c
index 8abae47..19a2f42 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -861,8 +861,7 @@  macho_symfile_read (struct objfile *objfile, int symfile_flags)
 	  symbol_table = (asymbol **) xmalloc (storage_needed);
 	  make_cleanup (xfree, symbol_table);
 
-          init_minimal_symbol_collection ();
-          make_cleanup_discard_minimal_symbols ();
+          scoped_minimal_symbol_reader reader;
 
 	  symcount = bfd_canonicalize_symtab (objfile->obfd, symbol_table);
 
@@ -873,7 +872,7 @@  macho_symfile_read (struct objfile *objfile, int symfile_flags)
 
 	  macho_symtab_read (objfile, symcount, symbol_table, &oso_vector);
 
-          install_minimal_symbols (objfile);
+          reader.install (objfile);
 	}
 
       /* Try to read .eh_frame / .debug_frame.  */
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 157ce77..d07e683 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -4870,14 +4870,12 @@  elfmdebug_build_psymtabs (struct objfile *objfile,
 {
   bfd *abfd = objfile->obfd;
   struct ecoff_debug_info *info;
-  struct cleanup *back_to;
 
   /* FIXME: It's not clear whether we should be getting minimal symbol
      information from .mdebug in an ELF file, or whether we will.
      Re-initialize the minimal symbol reader in case we do.  */
 
-  init_minimal_symbol_collection ();
-  back_to = make_cleanup_discard_minimal_symbols ();
+  scoped_minimal_symbol_reader reader;
 
   info = ((struct ecoff_debug_info *)
 	  obstack_alloc (&objfile->objfile_obstack,
@@ -4889,8 +4887,7 @@  elfmdebug_build_psymtabs (struct objfile *objfile,
 
   mdebug_build_psymtabs (objfile, swap, info);
 
-  install_minimal_symbols (objfile);
-  do_cleanups (back_to);
+  reader.install (objfile);
 }
 
 void
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index b478402..7b316ce 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -921,8 +921,7 @@  get_symbol_leading_char (bfd *abfd)
 
 /* See minsyms.h.  */
 
-void
-init_minimal_symbol_collection (void)
+scoped_minimal_symbol_reader::scoped_minimal_symbol_reader ()
 {
   msym_count = 0;
   msym_bunch = NULL;
@@ -932,6 +931,26 @@  init_minimal_symbol_collection (void)
   msym_bunch_index = BUNCH_SIZE;
 }
 
+/* Discard the currently collected minimal symbols, if any.  If we wish
+   to save them for later use, we must have already copied them somewhere
+   else before calling this function.
+
+   FIXME:  We could allocate the minimal symbol bunches on their own
+   obstack and then simply blow the obstack away when we are done with
+   it.  Is it worth the extra trouble though?  */
+
+scoped_minimal_symbol_reader::~scoped_minimal_symbol_reader ()
+{
+  struct msym_bunch *next;
+
+  while (msym_bunch != NULL)
+    {
+      next = msym_bunch->next;
+      xfree (msym_bunch);
+      msym_bunch = next;
+    }
+}
+
 /* See minsyms.h.  */
 
 void
@@ -1087,37 +1106,6 @@  compare_minimal_symbols (const void *fn1p, const void *fn2p)
     }
 }
 
-/* Discard the currently collected minimal symbols, if any.  If we wish
-   to save them for later use, we must have already copied them somewhere
-   else before calling this function.
-
-   FIXME:  We could allocate the minimal symbol bunches on their own
-   obstack and then simply blow the obstack away when we are done with
-   it.  Is it worth the extra trouble though?  */
-
-static void
-do_discard_minimal_symbols_cleanup (void *arg)
-{
-  struct msym_bunch *next;
-
-  while (msym_bunch != NULL)
-    {
-      next = msym_bunch->next;
-      xfree (msym_bunch);
-      msym_bunch = next;
-    }
-}
-
-/* See minsyms.h.  */
-
-struct cleanup *
-make_cleanup_discard_minimal_symbols (void)
-{
-  return make_cleanup (do_discard_minimal_symbols_cleanup, 0);
-}
-
-
-
 /* Compact duplicate entries out of a minimal symbol table by walking
    through the table and compacting out entries with duplicate addresses
    and matching names.  Return the number of entries remaining.
@@ -1244,7 +1232,7 @@  build_minimal_symbol_hash_tables (struct objfile *objfile)
    attempts to demangle them if we later add more minimal symbols.  */
 
 void
-install_minimal_symbols (struct objfile *objfile)
+scoped_minimal_symbol_reader::install (struct objfile *objfile)
 {
   int bindex;
   int mcount;
diff --git a/gdb/minsyms.h b/gdb/minsyms.h
index 8857f1a..5ea3610 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -53,21 +53,33 @@  struct bound_minimal_symbol
    as opaque and use functions provided by minsyms.c to inspect them.
 */
 
-/* Prepare to start collecting minimal symbols.  This should be called
-   by a symbol reader to initialize the minimal symbol module.
-   Currently, minimal symbol table creation is not reentrant; it
-   relies on global (static) variables in minsyms.c.  */
+// An RAII-based object that is used to record minimal symbols while
+// they are being read.
+class scoped_minimal_symbol_reader
+{
+ public:
+
+  /* Prepare to start collecting minimal symbols.  This should be called
+     by a symbol reader to initialize the minimal symbol module.
+     Currently, minimal symbol table creation is not reentrant; it
+     relies on global (static) variables in minsyms.c.  */
+
+  explicit scoped_minimal_symbol_reader ();
+
+  ~scoped_minimal_symbol_reader ();
 
-void init_minimal_symbol_collection (void);
+  /* Install the minimal symbols that have been collected into the
+     given objfile.  */
 
-/* Return a cleanup which is used to clean up the global state left
-   over by minimal symbol creation.  After calling
-   init_minimal_symbol_collection, a symbol reader should call this
-   function.  Then, after all minimal symbols have been read,
-   regardless of whether they are installed or not, the cleanup
-   returned by this function should be run.  */
+  void install (struct objfile *);
 
-struct cleanup *make_cleanup_discard_minimal_symbols (void);
+ private:
+
+  // No need for these.  They are intentionally not defined anywhere.
+  scoped_minimal_symbol_reader &operator=
+    (const scoped_minimal_symbol_reader &);
+  scoped_minimal_symbol_reader (const scoped_minimal_symbol_reader &);
+};
 
 /* Record a new minimal symbol.  This is the "full" entry point;
    simpler convenience entry points are also provided below.
@@ -122,13 +134,6 @@  struct minimal_symbol *prim_record_minimal_symbol_and_info
      int section,
      struct objfile *);
 
-/* Install the minimal symbols that have been collected into the given
-   objfile.  After this is called, the cleanup returned by
-   make_cleanup_discard_minimal_symbols should be run in order to
-   clean up global state.  */
-
-void install_minimal_symbols (struct objfile *);
-
 /* Create the terminating entry of OBJFILE's minimal symbol table.
    If OBJFILE->msymbols is zero, allocate a single entry from
    OBJFILE->objfile_obstack; otherwise, just initialize
diff --git a/gdb/mipsread.c b/gdb/mipsread.c
index 27a59d2..da09c03 100644
--- a/gdb/mipsread.c
+++ b/gdb/mipsread.c
@@ -69,10 +69,8 @@  static void
 mipscoff_symfile_read (struct objfile *objfile, int symfile_flags)
 {
   bfd *abfd = objfile->obfd;
-  struct cleanup *back_to;
 
-  init_minimal_symbol_collection ();
-  back_to = make_cleanup_discard_minimal_symbols ();
+  scoped_minimal_symbol_reader reader;
 
   /* Now that the executable file is positioned at symbol table,
      process it and define symbols accordingly.  */
@@ -91,8 +89,7 @@  mipscoff_symfile_read (struct objfile *objfile, int symfile_flags)
   /* Install any minimal symbols that have been collected as the current
      minimal symbols for this objfile.  */
 
-  install_minimal_symbols (objfile);
-  do_cleanups (back_to);
+  reader.install (objfile);
 }
 
 /* Perform any local cleanups required when we are done with a
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 507baf2..81b323c 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -3006,8 +3006,7 @@  xcoff_initial_scan (struct objfile *objfile, int symfile_flags)
   free_pending_blocks ();
   back_to = make_cleanup (really_free_pendings, 0);
 
-  init_minimal_symbol_collection ();
-  make_cleanup_discard_minimal_symbols ();
+  scoped_minimal_symbol_reader reader;
 
   /* Now that the symbol table data of the executable file are all in core,
      process them and define symbols accordingly.  */
@@ -3017,7 +3016,7 @@  xcoff_initial_scan (struct objfile *objfile, int symfile_flags)
   /* Install any minimal symbols that have been collected as the current
      minimal symbols for this objfile.  */
 
-  install_minimal_symbols (objfile);
+  reader.install (objfile);
 
   /* DWARF2 sections.  */