From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6146 invoked by alias); 8 Dec 2017 12:05:57 -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 6132 invoked by uid 89); 8 Dec 2017 12:05:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 08 Dec 2017 12:05:55 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6E8A9C058ECA; Fri, 8 Dec 2017 12:05:54 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id A52C662660; Fri, 8 Dec 2017 12:05:53 +0000 (UTC) Subject: Re: [PATCH] python: Add qualified parameter to gdb.Breakpoint To: Simon Marchi , gdb-patches@sourceware.org References: <1512686013-24658-1-git-send-email-simon.marchi@ericsson.com> From: Pedro Alves Message-ID: <18e86b00-7659-e57f-e650-fae62ebf1d2c@redhat.com> Date: Fri, 08 Dec 2017 12:05:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1512686013-24658-1-git-send-email-simon.marchi@ericsson.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-12/txt/msg00178.txt.bz2 That was fast. :-) Thanks! See comments about the docs below. The code looks fine to me, except a formatting nit. > --- a/gdb/doc/python.texi > +++ b/gdb/doc/python.texi > @@ -4885,7 +4885,7 @@ create both breakpoints and watchpoints. The second accepts separate Python > arguments similar to @ref{Explicit Locations} and can only be used to create > breakpoints. > > -@defun Breakpoint.__init__ (spec @r{[}, type @r{][}, wp_class @r{][}, internal @r{][}, temporary @r{]}) > +@defun Breakpoint.__init__ (spec @r{[}, type @r{][}, wp_class @r{][}, internal @r{][}, temporary @r{][}, qualified @r{]}) > Create a new breakpoint according to @var{spec}, which is a string naming the > location of a breakpoint, or an expression that defines a watchpoint. The > contents can be any location recognized by the @code{break} command or, in the > @@ -4909,14 +4909,20 @@ The optional @var{temporary} argument makes the breakpoint a temporary > breakpoint. Temporary breakpoints are deleted after they have been hit. Any > further access to the Python breakpoint after it has been hit will result in a > runtime error (as that breakpoint has now been automatically deleted). > + > +The optional @var{qualified} argument is a boolean that allows restricting the > +breakpoint to free-functions. "free-functions" is incorrect. With: struct A { void func (); } // #1 namespace B { struct A { void func (); } } // #2 "b -q A::func()" only matches #1 while "b A::func()" matches both #1 and #2. and A::func() above is not a free function. Here's what we say for -qualified in the explicit locations part of the manual: ~~~ @item -qualified This flag makes @value{GDBN} interpret a function name specified with @kbd{-function} as a complete fully-qualified name. For example, assuming a C@t{++} program with symbols named @code{A::B::func} and @code{B::func}, the @w{@kbd{break -qualified -function B::func}} command sets a breakpoint on @code{B::func}, only. (Note: the @kbd{-qualified} option can precede a linespec as well (@pxref{Linespec Locations}), so the particular example above could be simplified as @w{@kbd{break -qualified B::func}}.) ~~~ There's similar text in the linespecs part of the manual and also in "help break". See: $ git show a20714ff39f6 -- doc/gdb.texinfo NEWS break.c So we either need to clarify that in the Python bits too, or some do some xref'ing. It is equivalent to @code{break}'s > +@code{-qualified} flag (@pxref{Linespec Locations}). > + > @end defun > > -@defun Breakpoint.__init__ (@r{[} source @r{][}, function @r{][}, label @r{][}, line @r{]}, @r{][} internal @r{][}, temporary @r{][}) > +@defun Breakpoint.__init__ (@r{[} source @r{][}, function @r{][}, label @r{][}, line @r{]}, @r{][} internal @r{][}, temporary @r{][}, qualified @r{]}) > Create a new explicit location breakpoint (@pxref{Explicit Locations}) > according to the specifications contained in the key words @var{source}, > @var{function}, @var{label} and @var{line}. Should @var{qualified} be added to this list too? (BTW, noticed a typo in this paragraph: "create a new a explicit location" the second "a" is spurious.) > > -@var{internal} and @var{temporary} have the same usage as explained previously. > +@var{internal}, @var{temporary} and @var{qualified} have the same usage as > +explained previously. > @end defun > > The available types are represented by constants defined in the @code{gdb} > @@ -759,6 +761,9 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) > case bp_breakpoint: > { > event_location_up location; > + symbol_name_match_type func_name_match_type > + = qualified ? symbol_name_match_type::FULL > + : symbol_name_match_type::WILD; Should wrap the multi-line expression in ()s: symbol_name_match_type func_name_match_type = (qualified ? symbol_name_match_type::FULL : symbol_name_match_type::WILD); Though I prefer breaking ternary operators like this: symbol_name_match_type func_name_match_type = (qualified ? symbol_name_match_type::FULL : symbol_name_match_type::WILD); because it makes it look more obviously like if COND THEN ELSE Thanks, Pedro Alves