Re: [PATCH 09/11] gdb: change objfile::map_symtabs_matching_filename to find_symtab_matching_filename

[Andrew Burgess <aburgess@redhat.com>]

Simon Marchi <simon.marchi@efficios.com> writes:

> The only user of objfile::map_symtabs_matching_filename uses that method
> to find the first matching symtab.  It would therefore be more natural
> for that method to be a "find" method, returning the first symtab
> matching the predicate.
>
> Change map_symtabs_matching_filename to be
> find_symtab_in_compunit_symtab, and the internal
> iterate_over_one_compunit_symtab to be find_symtab_in_compunit_symtab.

Typo:

  Change map_symtabs_matching_filename to be find_symtab_in_compunit_symtab, and ..." 

Should be:

  Change map_symtabs_matching_filename to be find_symtab_matching_filename, and ..." 

>
> This makes function find_symtab simpler.
>
> Change-Id: Id14a95498fad243495d6eab18810d0c4ab8dbf90
> ---
>  gdb/objfiles.h      | 16 +++-----
>  gdb/symfile-debug.c | 89 +++++++++++++++++++++------------------------
>  gdb/symtab.c        | 20 ++--------
>  3 files changed, 52 insertions(+), 73 deletions(-)
>

> diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
> index 8e4892649508..d90f400f379c 100644
> --- a/gdb/symfile-debug.c
> +++ b/gdb/symfile-debug.c
> @@ -159,29 +159,30 @@ objfile::forget_cached_source_info ()
>      iter->forget_cached_source_info (this);
>  }
>  
> -/* Check for a symtab of a specific name by searching some symtabs.
> +/* Find the first symtab of CUST matching BASE_NAME, NAME and REAL_PATH, for
> +   which CALLBACK returns true.
>  
>     If NAME is not absolute, then REAL_PATH is NULL
>     If NAME is absolute, then REAL_PATH is the gdb_realpath form of NAME.
>  
>     The return value, NAME, REAL_PATH and CALLBACK are identical to the
> -   `map_symtabs_matching_filename' method of quick_symbol_functions.
> +   `find_symtab_matching_filename' method of quick_symbol_functions.

This comment doesn't make sense.  The find_symtab_matching_filename
function is a member function of `objfile` not
`quick_symbol_functions`.  You didn't write that, but it looks like a
pre-existing mistake in the comment.  Can you fix that please?  Maybe:

   The return value, NAME, REAL_PATH and CALLBACK are identical to the
   `objfile::find_symtab_matching_filename' method.

Would do?

> @@ -254,6 +258,7 @@ objfile::map_symtabs_matching_filename
>      return false;
>    };
>  
> +  symtab *result = nullptr;
>    auto compunit_callback = [&] (compunit_symtab *symtab)
>    {
>      /* Skip included compunits, as they are searched by

The next line of this comment references
iterate_over_one_compunit_symtab, and should be updated to
find_symtab_in_compunit_symtab.

> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 6a5c8c7058e4..fd841318387e 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -637,8 +637,6 @@ static symtab *
>  find_symtab (program_space *pspace, const char *name,
>  	     find_symtab_callback_ftype callback)
>  {
> -  struct symtab *result = NULL;
> -

Ahh, I should have looked ahead!  I complained about this in the last
patch, but I see it's now gone.  No need to change the last patch I
guess.

>    gdb::unique_xmalloc_ptr<char> real_path;
>  
>    /* Here we are interested in canonicalizing an absolute path, not

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

thanks,
Andrew