Re: [PATCH v2 09/11] gdb: add matching helpers to global_symbol_searcher

[Simon Marchi <simon.marchi@efficios.com>]

On 10/1/26 11:39 AM, Andrew Burgess wrote:
> simon.marchi@polymtl.ca writes:
> 
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> The methods of global_symbol_searcher repeat the same checks in several
>> places.  Add some helpers to make the code easier to follow:
>>
>>  - name_matches: check a name against the (optional) name regexp
>>
>>  - symtab_matches_filenames: check a symtab against the searched file
>>    names
>>
>>  - symbol_matches: all the filtering done on full symbols in
>>    add_matching_symbols
>>
>>  - msymbol_matches: the filtering done on minimal symbols, shared by
>>    expand_symtabs and add_matching_msymbols
>>
>> Switch expand_symtabs, add_matching_symbols and add_matching_msymbols to
>> use early continues to avoid deeply nested ifs.
>>
>> Change-Id: I553f70fb8ea9c7e55b89bb1204a402938c116c3b
>> ---
>>  gdb/symtab.c | 205 ++++++++++++++++++++++++++++-----------------------
>>  gdb/symtab.h |  16 ++++
>>  2 files changed, 130 insertions(+), 91 deletions(-)
>>
>> diff --git a/gdb/symtab.c b/gdb/symtab.c
>> index fc0ff22448ee..ef21f285abcd 100644
>> --- a/gdb/symtab.c
>> +++ b/gdb/symtab.c
> 
> 
>> @@ -4906,34 +4938,25 @@ global_symbol_searcher::add_matching_msymbols
>>      {
>>        QUIT;
>>  
>> -      if (msymbol->created_by_gdb)
>> +      if (!msymbol_matches (msymbol, name_regex))
>>  	continue;
>>  
>> -      if (is_suitable_msymbol (msymbol))
>> -	{
>> -	  if (!name_regex.has_value ()
>> -	      || name_regex->exec (msymbol->natural_name (), 0,
>> -				   NULL, 0) == 0)
>> -	    {
>> -	      /* For functions we can do a quick check of whether the
>> -		 symbol might be found via find_pc_symtab.  */
>> -	      if (m_kind != symbol_search_kind::FUNCTION
>> -		  || (find_compunit_symtab_for_pc
>> -		      (msymbol->value_address (objfile)) == NULL))
>> -		{
>> -		  if (lookup_symbol_in_objfile_from_linkage_name
>> -		      (objfile, msymbol->linkage_name (),
>> -		       SEARCH_VFT).symbol == NULL)
>> -		    {
>> -		      /* Matching msymbol, add it to the results list.  */
>> -		      if (results->size () < m_max_search_results)
>> -			results->emplace_back (GLOBAL_BLOCK, msymbol, objfile);
>> -		      else
>> -			return false;
>> -		    }
>> -		}
>> -	    }
>> -	}
>> +      /* For functions we can do a quick check of whether the
>> +	 symbol might be found via find_pc_symtab.  */
>> +      if (m_kind == symbol_search_kind::FUNCTION
>> +	  && (find_compunit_symtab_for_pc
>> +	      (msymbol->value_address (objfile)) != NULL))
>> +	continue;
>> +
>> +      if (lookup_symbol_in_objfile_from_linkage_name
>> +	    (objfile, msymbol->linkage_name (), SEARCH_VFT).symbol != NULL)
> 
> Two lines here where NULL can be updated to nullptr.

Yes, I had already fixed them following your previous review.

> With that fix:
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,

Simon