From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40247 invoked by alias); 5 Oct 2018 06:46:09 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 40235 invoked by uid 89); 5 Oct 2018 06:46:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_1,KAM_STOCKGEN,SPF_PASS autolearn=ham version=3.3.2 spammy=mysterious, hyperlink X-HELO: eggs.gnu.org Received: from eggs.gnu.org (HELO eggs.gnu.org) (208.118.235.92) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 05 Oct 2018 06:46:05 +0000 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g8Js4-0003F7-9r for gdb-patches@sourceware.org; Fri, 05 Oct 2018 02:46:01 -0400 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:36686) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g8Js2-0003C2-56; Fri, 05 Oct 2018 02:45:56 -0400 Received: from [176.228.60.248] (port=3270 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1g8Jrz-000091-Vy; Fri, 05 Oct 2018 02:45:54 -0400 Date: Fri, 05 Oct 2018 06:46:00 -0000 Message-Id: <834le03n8u.fsf@gnu.org> From: Eli Zaretskii To: Tom de Vries CC: gdb-patches@sourceware.org In-reply-to: <20181004211115.GA31056@delia> (message from Tom de Vries on Thu, 4 Oct 2018 23:11:17 +0200) Subject: Re: [PATCH][gdb/python] Add interface to access minimal_symbols References: <20181004211115.GA31056@delia> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-IsSubscribed: yes X-SW-Source: 2018-10/txt/msg00123.txt.bz2 > Date: Thu, 4 Oct 2018 23:11:17 +0200 > From: Tom de Vries > > [ 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. > +@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. > +@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. > +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: > +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. > +@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. > +@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. > +@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. > +@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. The documentation part is approved with the above gotchas fixed.