combine bfd_lookup_symbol in solib-*.c

Yao Qi yao@codesourcery.com
Mon Aug 29 16:23:00 GMT 2011


On 08/29/2011 07:41 PM, Pedro Alves wrote:

> 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.

OK, parameters are documented.

>
> 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)
>

I agree.  Renamed bfd_* functions to gdb_bfd_*.

>> +/* 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.
>

OK, removed this comment here.

-- 
Yao (齐尧)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-combine-bfd_lookup_symbol.patch
Type: text/x-patch
Size: 16798 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20110829/9a7b7c16/attachment.bin>


More information about the Gdb-patches mailing list