diff mbox series

[8/9] gdb: pass result containers by reference in global_symbol_searcher

Message ID 20260929194119.155169-9-simon.marchi@efficios.com
State New
Headers
Series Cleanups in global_symbol_searcher |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed

Commit Message

Simon Marchi Sept. 29, 2026, 7:39 p.m. UTC 
  add_matching_symbols and add_matching_msymbols take the container to
add results to as a pointer, but it can never be nullptr.  Pass it by
reference instead.

Change-Id: I65d8e14f2f3152d83afbe3b348fc91a8817b87fa
---
 gdb/symtab.c | 17 ++++++++---------
 gdb/symtab.h |  4 ++--
 2 files changed, 10 insertions(+), 11 deletions(-)

Comments

Rohr, Stephan Sept. 30, 2026, 1:58 p.m. UTC | #1 
> -----Original Message-----
> From: Simon Marchi <simon.marchi@efficios.com>
> Sent: Tuesday, 29 September 2026 21:39
> To: gdb-patches@sourceware.org
> Cc: Simon Marchi <simon.marchi@efficios.com>
> Subject: [PATCH 8/9] gdb: pass result containers by reference in
> global_symbol_searcher
> 
> add_matching_symbols and add_matching_msymbols take the container to
> add results to as a pointer, but it can never be nullptr.  Pass it by
> reference instead.
> 
> Change-Id: I65d8e14f2f3152d83afbe3b348fc91a8817b87fa
> ---
>  gdb/symtab.c | 17 ++++++++---------
>  gdb/symtab.h |  4 ++--
>  2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 79e43eafde18..10a794004d33 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -4899,7 +4899,7 @@ global_symbol_searcher::add_matching_symbols
>  	(objfile *objfile,
>  	 const std::optional<compiled_regex> &name_regex,
>  	 const std::optional<compiled_regex> &type_regex,
> -	 std::set<symbol_search> *result_set) const
> +	 std::set<symbol_search> &result_set) const

Just a general question: does it make sense to keep "objfile" as pointer
or could this also be converted to reference for consistency?

>  {
>    /* Add matching symbols (if not already present).  */
>    for (compunit_symtab &cust : objfile->compunits ())
> @@ -4918,8 +4918,8 @@ global_symbol_searcher::add_matching_symbols
>  		continue;
> 
>  	      /* Match, insert if not already in the results.  */
> -	      if (result_set->size () < m_max_search_results)
> -		result_set->emplace (block, sym);
> +	      if (result_set.size () < m_max_search_results)
> +		result_set.emplace (block, sym);
>  	      else
>  		return false;
>  	    }
> @@ -4934,7 +4934,7 @@ global_symbol_searcher::add_matching_symbols
>  bool
>  global_symbol_searcher::add_matching_msymbols
>  	(objfile *objfile, const std::optional<compiled_regex> &name_regex,
> -	 std::vector<symbol_search> *results) const
> +	 std::vector<symbol_search> &results) const
>  {
>    for (minimal_symbol *msymbol : objfile->msymbols ())
>      {
> @@ -4955,8 +4955,8 @@
> global_symbol_searcher::add_matching_msymbols
>  	continue;
> 
>        /* Matching msymbol, add it to the results list.  */
> -      if (results->size () < m_max_search_results)
> -	results->emplace_back (GLOBAL_BLOCK, msymbol, objfile);
> +      if (results.size () < m_max_search_results)
> +	results.emplace_back (GLOBAL_BLOCK, msymbol, objfile);
>        else
>  	return false;
>      }
> @@ -5056,8 +5056,7 @@ global_symbol_searcher::search () const
>  	 RESULT_SET set.  Use a set here so that we can easily detect
>  	 duplicates as we go, and can therefore track how many unique
>  	 matches we have found so far.  */
> -      if (!add_matching_symbols (&objfile, name_regex, type_regex,
> -				 &result_set))
> +      if (!add_matching_symbols (&objfile, name_regex, type_regex,
> result_set))
>  	break;
>      }
> 
> @@ -5077,7 +5076,7 @@ global_symbol_searcher::search () const
>        gdb_assert (m_kind == symbol_search_kind::VARIABLE
>  		  || m_kind == symbol_search_kind::FUNCTION);
>        for (objfile &objfile : current_program_space->objfiles ())
> -	if (!add_matching_msymbols (&objfile, name_regex, &result))
> +	if (!add_matching_msymbols (&objfile, name_regex, result))
>  	  break;
>      }
> 
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 5dd0475e4491..e382b64b427c 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -2705,7 +2705,7 @@ class global_symbol_searcher
>    bool add_matching_symbols (objfile *objfile,
>  			     const std::optional<compiled_regex> &name_regex,
>  			     const std::optional<compiled_regex> &type_regex,
> -			     std::set<symbol_search> *result_set) const;
> +			     std::set<symbol_search> &result_set) const;
> 

Nit: The comment in symtab.h mentions "RESULTS_SET", but it should be
"RESULT_SET".  This is not the correct patch to address this, I just noticed now.

>    /* Add msymbols from OBJFILE that match NAME_REGEX and M_KIND, to
> the
>       results vector RESULTS.  Return false if we stop adding results early
> @@ -2715,7 +2715,7 @@ class global_symbol_searcher
>       didn't _not_ add a result due to reaching MAX_SEARCH_RESULTS.  */
>    bool add_matching_msymbols (objfile *objfile,
>  			      const std::optional<compiled_regex>
> &name_regex,
> -			      std::vector<symbol_search> *results) const;
> +			      std::vector<symbol_search> &results) const;
> 
>    /* Return true if MSYMBOL is of type M_KIND.  */
>    bool is_suitable_msymbol (const minimal_symbol *msymbol) const;
> --
> 2.55.0

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Simon Marchi Oct. 1, 2026, 1:33 a.m. UTC | #2 
On 2026-09-30 09:58, Rohr, Stephan wrote:
> 
> 
>> -----Original Message-----
>> From: Simon Marchi <simon.marchi@efficios.com>
>> Sent: Tuesday, 29 September 2026 21:39
>> To: gdb-patches@sourceware.org
>> Cc: Simon Marchi <simon.marchi@efficios.com>
>> Subject: [PATCH 8/9] gdb: pass result containers by reference in
>> global_symbol_searcher
>>
>> add_matching_symbols and add_matching_msymbols take the container to
>> add results to as a pointer, but it can never be nullptr.  Pass it by
>> reference instead.
>>
>> Change-Id: I65d8e14f2f3152d83afbe3b348fc91a8817b87fa
>> ---
>>  gdb/symtab.c | 17 ++++++++---------
>>  gdb/symtab.h |  4 ++--
>>  2 files changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/gdb/symtab.c b/gdb/symtab.c
>> index 79e43eafde18..10a794004d33 100644
>> --- a/gdb/symtab.c
>> +++ b/gdb/symtab.c
>> @@ -4899,7 +4899,7 @@ global_symbol_searcher::add_matching_symbols
>>  	(objfile *objfile,
>>  	 const std::optional<compiled_regex> &name_regex,
>>  	 const std::optional<compiled_regex> &type_regex,
>> -	 std::set<symbol_search> *result_set) const
>> +	 std::set<symbol_search> &result_set) const
> 
> Just a general question: does it make sense to keep "objfile" as pointer
> or could this also be converted to reference for consistency?

It could be converted to a reference as well, I'll do it.

>> diff --git a/gdb/symtab.h b/gdb/symtab.h
>> index 5dd0475e4491..e382b64b427c 100644
>> --- a/gdb/symtab.h
>> +++ b/gdb/symtab.h
>> @@ -2705,7 +2705,7 @@ class global_symbol_searcher
>>    bool add_matching_symbols (objfile *objfile,
>>  			     const std::optional<compiled_regex> &name_regex,
>>  			     const std::optional<compiled_regex> &type_regex,
>> -			     std::set<symbol_search> *result_set) const;
>> +			     std::set<symbol_search> &result_set) const;
>>
> 
> Nit: The comment in symtab.h mentions "RESULTS_SET", but it should be
> "RESULT_SET".  This is not the correct patch to address this, I just noticed now.

Fixed that too.

Thanks,

Simon
diff mbox series

Patch

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 79e43eafde18..10a794004d33 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4899,7 +4899,7 @@  global_symbol_searcher::add_matching_symbols
 	(objfile *objfile,
 	 const std::optional<compiled_regex> &name_regex,
 	 const std::optional<compiled_regex> &type_regex,
-	 std::set<symbol_search> *result_set) const
+	 std::set<symbol_search> &result_set) const
 {
   /* Add matching symbols (if not already present).  */
   for (compunit_symtab &cust : objfile->compunits ())
@@ -4918,8 +4918,8 @@  global_symbol_searcher::add_matching_symbols
 		continue;
 
 	      /* Match, insert if not already in the results.  */
-	      if (result_set->size () < m_max_search_results)
-		result_set->emplace (block, sym);
+	      if (result_set.size () < m_max_search_results)
+		result_set.emplace (block, sym);
 	      else
 		return false;
 	    }
@@ -4934,7 +4934,7 @@  global_symbol_searcher::add_matching_symbols
 bool
 global_symbol_searcher::add_matching_msymbols
 	(objfile *objfile, const std::optional<compiled_regex> &name_regex,
-	 std::vector<symbol_search> *results) const
+	 std::vector<symbol_search> &results) const
 {
   for (minimal_symbol *msymbol : objfile->msymbols ())
     {
@@ -4955,8 +4955,8 @@  global_symbol_searcher::add_matching_msymbols
 	continue;
 
       /* Matching msymbol, add it to the results list.  */
-      if (results->size () < m_max_search_results)
-	results->emplace_back (GLOBAL_BLOCK, msymbol, objfile);
+      if (results.size () < m_max_search_results)
+	results.emplace_back (GLOBAL_BLOCK, msymbol, objfile);
       else
 	return false;
     }
@@ -5056,8 +5056,7 @@  global_symbol_searcher::search () const
 	 RESULT_SET set.  Use a set here so that we can easily detect
 	 duplicates as we go, and can therefore track how many unique
 	 matches we have found so far.  */
-      if (!add_matching_symbols (&objfile, name_regex, type_regex,
-				 &result_set))
+      if (!add_matching_symbols (&objfile, name_regex, type_regex, result_set))
 	break;
     }
 
@@ -5077,7 +5076,7 @@  global_symbol_searcher::search () const
       gdb_assert (m_kind == symbol_search_kind::VARIABLE
 		  || m_kind == symbol_search_kind::FUNCTION);
       for (objfile &objfile : current_program_space->objfiles ())
-	if (!add_matching_msymbols (&objfile, name_regex, &result))
+	if (!add_matching_msymbols (&objfile, name_regex, result))
 	  break;
     }
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 5dd0475e4491..e382b64b427c 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2705,7 +2705,7 @@  class global_symbol_searcher
   bool add_matching_symbols (objfile *objfile,
 			     const std::optional<compiled_regex> &name_regex,
 			     const std::optional<compiled_regex> &type_regex,
-			     std::set<symbol_search> *result_set) const;
+			     std::set<symbol_search> &result_set) const;
 
   /* Add msymbols from OBJFILE that match NAME_REGEX and M_KIND, to the
      results vector RESULTS.  Return false if we stop adding results early
@@ -2715,7 +2715,7 @@  class global_symbol_searcher
      didn't _not_ add a result due to reaching MAX_SEARCH_RESULTS.  */
   bool add_matching_msymbols (objfile *objfile,
 			      const std::optional<compiled_regex> &name_regex,
-			      std::vector<symbol_search> *results) const;
+			      std::vector<symbol_search> &results) const;
 
   /* Return true if MSYMBOL is of type M_KIND.  */
   bool is_suitable_msymbol (const minimal_symbol *msymbol) const;