Re: [PATCH][gdb/python] Add interface to access minimal_symbols

[Tom de Vries <tdevries@suse.de>]

On 10/5/18 8:45 AM, Eli Zaretskii wrote:
>> Date: Thu, 4 Oct 2018 23:11:17 +0200
>> From: Tom de Vries <tdevries@suse.de>
>>
>> [ Submitted earlier here (
>> https://sourceware.org/ml/gdb-patches/2016-02/msg00124.html ). ]
>>
>> This patch adds a new gdb.MinSymbol object to export the minimal_symbol
>> interface.
> 
> Thanks.
> 
>> Build and reg-tested on x86_64-linux.
>>
>> OK for trunk?
> 
> I have a couple of comments for the documentation part.
> 
> I think this warrants a NEWS entry.
> 

Added.

>> +@value{GDBN} represents every variable, function and type as an
>> +entry in a symbol table.  @xref{Symbols, ,Examining the Symbol Table}.
>> +Typical symbols like functions, variables, etc are represented by
>> +gdb.Symbol objects in Python.  Some symbols are defined with less
> 
> "gdb.Symbol" should be in @code.
> 

Done.

>> +@findex gdb.lookup_minimal_symbol
>> +@defun gdb.lookup_minimal_symbol (name @r{[}, sfile@r{]}, objfile@r{[})
> 
> There's no need to use @findex for methods you introduce with @defun,
> since the latter automatically creates an @findex entry.
> 

Removed.

>> +This function searches for a minimal symbol by name.
>> +The search scope can be restricted by the sfile and objfile arguments.
>                                              ^^^^^     ^^^^^^^
> Argument names in text should have the @var markup, like you did below:
> 

Done.

>> +The optional @var{sfile} argument restricts the search to the source file
>> +in which the minimal symbol was defined.
>> +The @var{sfile} argument must be a string.  The optional @var{objfile}
>> +restricts the search to the objfile that contains the minimal symbol.
> 
>> +@defvar MinSymbol.print_name
>> +The name of the symbol in a form suitable for output.  This is either
>> +@code{name} or @code{linkage_name}, depending on whether the user
>> +asked @value{GDBN} to display demangled or mangled names.
> 
> When you refer to other GDB features, it is generally a good idea to
> have a hyperlink there.  In this case, I believe you refer to the
> command "set print demangle", described in "Print Settings", so please
> use @pxref or @xref to add a cross-reference there.
> 

Done.

>> +@defvar MinSymbol.filename
>> +The file name of the source file where the minimal symbol is defined.  This
>> +value may represent filenames used internally by the compiler rather
>> +than a viewable/editable source file.
> 
> The last sentence sounds a bit mysterious to me: what are "filenames
> used internally by the compiler"?  Maybe an example will clarify that.
> 

I've removed that bit.

>> +@defvar MinSymbol.section
>> +The name of the binary section containing this minimal symbol.
> 
> I would suggest to use "section in the object file"; "binary" might be
> misinterpreted to mean that the section itself is binary.
> 

Done.

>> +@vindex MINSYMBOL_TYPE_SLOT_GOT_PLT
>> +@item gdb.MINSYMBOL_TYPE_SLOT_GOT_PLT
>> +This type represents GOT for .plt sections.
> 
> I don't believe we have "GOT" mentioned anywhere else in the manual,
> so (1) please use @acronym{GOT}, and (2) please tell in the text what
> this acronym stands for.
> 

Done.

>> +@vindex MINSYMBOL_TYPE_FILE_DATA
>> +@item gdb.MINSYMBOL_TYPE_FILE_DATA
>> +This type represents the static version of gdb.MINSYMBOL_TYPE_DATA.
>> +
>> +@vindex MINSYMBOL_TYPE_FILE_BSS
>> +@item gdb.MINSYMBOL_TYPE_FILE_BSS
>> +This type represents the static version of gdb.MINSYMBOL_TYPE_BSS.
>> +@end vtable
> 
> The 2 "gdb.*" symbols above should be in @code.
> 

Done.

> The documentation part is approved with the above gotchas fixed.
> 

Retested and reposted at
https://sourceware.org/ml/gdb-patches/2018-10/msg00756.html .

Thanks,
- Tom