From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 96970 invoked by alias); 31 Oct 2018 17:02:50 -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 96958 invoked by uid 89); 31 Oct 2018 17:02:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.4 required=5.0 tests=BAYES_00,GIT_PATCH_1,KAM_STOCKGEN,SPF_PASS autolearn=ham version=3.3.2 spammy=crossreference, cross-reference X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 31 Oct 2018 17:02:43 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 344C8B11A; Wed, 31 Oct 2018 17:02:41 +0000 (UTC) Subject: Re: [PATCH][gdb/python] Add interface to access minimal_symbols To: Eli Zaretskii Cc: gdb-patches@sourceware.org References: <20181004211115.GA31056@delia> <834le03n8u.fsf@gnu.org> From: Tom de Vries Message-ID: <3f88a394-6fda-12b4-5eae-ce07be95f91f@suse.de> Date: Wed, 31 Oct 2018 17:02:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <834le03n8u.fsf@gnu.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-10/txt/msg00757.txt.bz2 On 10/5/18 8:45 AM, Eli Zaretskii wrote: >> 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. > 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