From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7216 invoked by alias); 7 May 2013 03:39:41 -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 7205 invoked by uid 89); 7 May 2013 03:39:41 -0000 X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,TW_XS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 07 May 2013 03:39:40 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r473dcR7012379 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 6 May 2013 23:39:38 -0400 Received: from psique (ovpn-113-115.phx2.redhat.com [10.3.113.115]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r473dYNi011313 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 6 May 2013 23:39:37 -0400 From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches , "Dr. David Alan Gilbert" Subject: Re: [PATCH] Fix for PR 15413 (segfault when completing "condition" for pending bp) References: <5187ED3A.8000108@redhat.com> <51881BCD.7070009@redhat.com> X-URL: http://www.redhat.com Date: Tue, 07 May 2013 03:39:00 -0000 In-Reply-To: <51881BCD.7070009@redhat.com> (Pedro Alves's message of "Mon, 06 May 2013 22:08:29 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-05/txt/msg00169.txt.bz2 On Monday, May 06 2013, Pedro Alves wrote: > On 05/06/2013 09:57 PM, Sergio Durigan Junior wrote: > >> So, if I understood your brain dump correctly, you're suggesting that >> the "condition" command shouldn't complete multiple locations at all, >> since the condition is inherent to the breakpoint, not to the >> location(s). Is that right? I will submit a patch soon. > > Yes. The patch to do that should be quite trivial, > mostly just removing/simplifying code, and it should fix the > segfault too as consequence, so I'd prefer focusing on a patch > that did that first (over trying to fix the existing bogus > multi-location code), > >> [OTOH, I guess it would make more sense if the condition were a location >> property.] > > and leave this for a separate discussion. The patch should then > be quite safe for 7.6 backporting too. Sure enough, I wasn't planning to address this whole issue now. Here is the updated patch. I created a new test on gdb.linespec/linespec.exp because it seemed like the best place to test the "condition" for multiple locations. OK? -- Sergio gdb/ 2013-05-07 Sergio Durigan Junior PR breakpoints/15413: * breakpoint.c (condition_completer): Simplify the code to disconsider multiple locations of breakpoints when completing the "condition" command. gdb/testsuite 2013-05-07 Sergio Durigan Junior PR breakpoints/15413: * gdb.base/pending.exp: Add test for completion of the "condition" command for pending breakpoints. * gdb.linespec/linespec.ex: Add test for completion of the "condition" command when dealing with multiple locations. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index ef9c23c..b1488dc 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1012,27 +1012,14 @@ condition_completer (struct cmd_list_element *cmd, len = strlen (text); ALL_BREAKPOINTS (b) - { - int single = b->loc->next == NULL; - struct bp_location *loc; - int count = 1; - - for (loc = b->loc; loc; loc = loc->next) - { - char location[50]; - - if (single) - xsnprintf (location, sizeof (location), "%d", b->number); - else - xsnprintf (location, sizeof (location), "%d.%d", b->number, - count); + { + char location[50]; - if (strncmp (location, text, len) == 0) - VEC_safe_push (char_ptr, result, xstrdup (location)); + xsnprintf (location, sizeof (location), "%d", b->number); - ++count; - } - } + if (strncmp (location, text, len) == 0) + VEC_safe_push (char_ptr, result, xstrdup (location)); + } return result; } diff --git a/gdb/testsuite/gdb.base/pending.exp b/gdb/testsuite/gdb.base/pending.exp index 68322f5..a8dbef5 100644 --- a/gdb/testsuite/gdb.base/pending.exp +++ b/gdb/testsuite/gdb.base/pending.exp @@ -55,6 +55,13 @@ gdb_test_multiple "break pendfunc1" "set pending breakpoint" { } } +# Complete the condition (PR 15413). +# This test must come right after we set the first pending breakpoint, and +# before we set any other breakpoint, since we are testing if the "condition" +# command can properly complete its argument. The PR only fails if there +# is only one pending breakpoint set (without anything else). +gdb_test "complete condition " "condition 1" + gdb_test "info break" \ "Num Type\[ \]+Disp Enb Address\[ \]+What.* \[0-9\]+\[\t \]+breakpoint keep y.*PENDING.*pendfunc1.*" \ diff --git a/gdb/testsuite/gdb.linespec/linespec.exp b/gdb/testsuite/gdb.linespec/linespec.exp index 741ada0..fe02365 100644 --- a/gdb/testsuite/gdb.linespec/linespec.exp +++ b/gdb/testsuite/gdb.linespec/linespec.exp @@ -69,6 +69,10 @@ gdb_test "break dupname:label" \ "Breakpoint $decimal at $hex: dupname:label. \[(\]2 locations\[)\]" \ "multi-location break using duplicate function name and label" +# Testing if the "condition" command completes only the breakpoints, +# not the locations. +gdb_test "complete condition " "condition $decimal\r\ncondition $decimal\r\ncondition $decimal" + gdb_test_no_output "set breakpoint pending off" \ "disable pending breakpoints for linespec tests"