Re: combine bfd_lookup_symbol in solib-*.c

[Pedro Alves <pedro at codesourcery dot com>]

Hi Yao, thanks for doing this.  Looks mostly good.
A couple cosmetic comments below.

On Thursday 18 August 2011 16:36:46, Yao Qi wrote:

> +
> +CORE_ADDR
> +bfd_lookup_symbol (bfd *abfd, const char *symname,
> +                  int (*is_right_sym) (asymbol *, const char *))

All the function does with the passed in SYMNAME is passing
it down to the callback.  If the interface has been generalized to
pass in a "match" callback, then make the symname parameter
generic too, like:

  CORE_ADDR
  bfd_lookup_symbol (bfd *abfd,
                  int (*is_right_sym) (asymbol *, void *),
                  void *data);

Please document the parameters.

I think it's best to reserve extern symbols that start with
"bfd_" to libbfd proper.  Maybe gdb_bfd_lookup_symbol would
be a good enough alternative.  (Or maybe propose the function
for bfd proper, though I'd suggest fixing its interface
to not assume 0 return is error then)

> +/* Lookup the value for a specific symbol.
> +
> +   An expensive way to lookup the value of a single symbol for
> +   bfd's that are only temporary anyway.  This is used by the
> +   shared library support to find the address of the debugger
> +   notification routine in the shared library.
> +
> +   The returned symbol may be in a code or data section; functions
> +   will normally be in a code section, but may be in a data section
> +   if this architecture uses function descriptors.

The way you've generalized the function, since it's the
callback's responsibility to pick the the symbol, this
comment no longer appears to be in the right place.

-- 
Pedro Alves