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

[Simon Marchi <simark@simark.ca>]

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