Patchwork [RFA,05/15] Simplify calls to init_psymbol_list

login
register
mail settings
Submitter Tom Tromey
Date May 10, 2018, 10:23 p.m.
Message ID <20180510222357.27332-6-tom@tromey.com>
Download mbox | patch
Permalink /patch/27224/
State New
Headers show

Comments

Tom Tromey - May 10, 2018, 10:23 p.m.
Existing callers to init_psymbol_list were checking to see if psymbols
had already been initialized.  It seemed better to me to do this check
directly in init_psymbol_list, simplifying the callers.

2018-05-09  Tom Tromey  <tom@tromey.com>

	* xcoffread.c (xcoff_initial_scan): Unconditionally call
	init_psymbol_list.
	* psymtab.c (init_psymbol_list): Do nothing if already called.
	* psympriv.h (init_psymbol_list): Add comment.
	* dwarf2read.c (dwarf2_build_psymtabs): Unconditionally call
	init_psymbol_list.
	* dbxread.c (dbx_symfile_read): Unconditionally call
	init_psymbol_list.
---
 gdb/ChangeLog    | 11 +++++++++++
 gdb/dbxread.c    |  4 +---
 gdb/dwarf2read.c |  4 +---
 gdb/psympriv.h   |  6 +++++-
 gdb/psymtab.c    |  8 ++++----
 gdb/xcoffread.c  | 13 +++++--------
 6 files changed, 27 insertions(+), 19 deletions(-)
Simon Marchi - July 18, 2018, 2:50 a.m.
On 2018-05-10 06:23 PM, Tom Tromey wrote:
> diff --git a/gdb/psymtab.c b/gdb/psymtab.c
> index 4fd47bf92b..c1c8cba800 100644
> --- a/gdb/psymtab.c
> +++ b/gdb/psymtab.c
> @@ -1725,14 +1725,14 @@ add_psymbol_to_list (const char *name, int namelength, int copy_name,
>    append_psymbol_to_list (list, psym, objfile);
>  }
>  
> -/* Initialize storage for partial symbols.  */
> +/* See psympriv.h.  */
>  
>  void
>  init_psymbol_list (struct objfile *objfile, int total_symbols)
>  {
> -  /* Free any previously allocated psymbol lists.  */
> -  objfile->global_psymbols.clear ();
> -  objfile->static_psymbols.clear ();
> +  if (objfile->global_psymbols.capacity () == 0
> +      && objfile->static_psymbols.capacity () == 0)
> +    return;

Hmm, isn't the condition backwards?  If the capacity is zero, we want to
reserve some space.  But maybe we can skip the check completely... if we
have already reserved the same space previously, calling reserve again
does nothing (is harmless).

Would LGTM with the condition fixed (assuming I am not the one being confused)
or removed.

Simon
Tom Tromey - Sept. 23, 2018, 9:12 p.m.
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> void
>> init_psymbol_list (struct objfile *objfile, int total_symbols)
>> {
>> -  /* Free any previously allocated psymbol lists.  */
>> -  objfile->global_psymbols.clear ();
>> -  objfile->static_psymbols.clear ();
>> +  if (objfile->global_psymbols.capacity () == 0
>> +      && objfile->static_psymbols.capacity () == 0)
>> +    return;

Simon> Hmm, isn't the condition backwards?

Yep.

Simon> Would LGTM with the condition fixed (assuming I am not the one being confused)
Simon> or removed.

I've fixed it locally.

Tom

Patch

diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index be0483a38b..17d53331a4 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -529,9 +529,7 @@  dbx_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
     perror_with_name (objfile_name (objfile));
 
   /* Size the symbol table.  */
-  if (objfile->global_psymbols.capacity () == 0
-      && objfile->static_psymbols.capacity () == 0)
-    init_psymbol_list (objfile, DBX_SYMCOUNT (objfile));
+  init_psymbol_list (objfile, DBX_SYMCOUNT (objfile));
 
   symbol_size = DBX_SYMBOL_SIZE (objfile);
   symbol_table_offset = DBX_SYMTAB_OFFSET (objfile);
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 823f6bfd2a..9e846e75bb 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -6226,9 +6226,7 @@  dwarf2_build_psymtabs (struct objfile *objfile)
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
-  if (objfile->global_psymbols.capacity () == 0
-      && objfile->static_psymbols.capacity () == 0)
-    init_psymbol_list (objfile, 1024);
+  init_psymbol_list (objfile, 1024);
 
   TRY
     {
diff --git a/gdb/psympriv.h b/gdb/psympriv.h
index 8dcd82212b..32c9ee1199 100644
--- a/gdb/psympriv.h
+++ b/gdb/psympriv.h
@@ -295,7 +295,11 @@  extern void add_psymbol_to_list (const char *, int,
 				 CORE_ADDR,
 				 enum language, struct objfile *);
 
-extern void init_psymbol_list (struct objfile *, int);
+/* Initialize storage for partial symbols.  If partial symbol storage
+   has already been initialized, this does nothing.  TOTAL_SYMBOLS is
+   an estimate of how many symbols there will be.  */
+
+extern void init_psymbol_list (struct objfile *objfile, int total_symbols);
 
 extern struct partial_symtab *start_psymtab_common (struct objfile *,
 						    const char *, CORE_ADDR);
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 4fd47bf92b..c1c8cba800 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1725,14 +1725,14 @@  add_psymbol_to_list (const char *name, int namelength, int copy_name,
   append_psymbol_to_list (list, psym, objfile);
 }
 
-/* Initialize storage for partial symbols.  */
+/* See psympriv.h.  */
 
 void
 init_psymbol_list (struct objfile *objfile, int total_symbols)
 {
-  /* Free any previously allocated psymbol lists.  */
-  objfile->global_psymbols.clear ();
-  objfile->static_psymbols.clear ();
+  if (objfile->global_psymbols.capacity () == 0
+      && objfile->static_psymbols.capacity () == 0)
+    return;
 
   /* Current best guess is that approximately a twentieth
      of the total symbols (in a debugging file) are global or static
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 3cb1944554..7ec367a7ac 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -3000,14 +3000,11 @@  xcoff_initial_scan (struct objfile *objfile, symfile_add_flags symfile_flags)
   if (val != size)
     perror_with_name (_("reading symbol table"));
 
-  /* If we are reinitializing, or if we have never loaded syms yet, init.  */
-  if (objfile->global_psymbols.capacity () == 0
-      && objfile->static_psymbols.capacity () == 0)
-    /* I'm not sure how how good num_symbols is; the rule of thumb in
-       init_psymbol_list was developed for a.out.  On the one hand,
-       num_symbols includes auxents.  On the other hand, it doesn't
-       include N_SLINE.  */
-    init_psymbol_list (objfile, num_symbols);
+  /* I'm not sure how how good num_symbols is; the rule of thumb in
+     init_psymbol_list was developed for a.out.  On the one hand,
+     num_symbols includes auxents.  On the other hand, it doesn't
+     include N_SLINE.  */
+  init_psymbol_list (objfile, num_symbols);
 
   free_pending_blocks ();