From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8870 invoked by alias); 21 Mar 2011 21:15:20 -0000 Received: (qmail 8776 invoked by uid 22791); 21 Mar 2011 21:15:18 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from e24smtp04.br.ibm.com (HELO e24smtp04.br.ibm.com) (32.104.18.25) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 21 Mar 2011 21:15:11 +0000 Received: from /spool/local by e24smtp04.br.ibm.com with XMail ESMTP for from ; Mon, 21 Mar 2011 18:15:05 -0300 Received: from mailhub1.br.ibm.com ([9.18.232.109]) by e24smtp04.br.ibm.com ([10.172.0.140]) with XMail ESMTP; Mon, 21 Mar 2011 18:15:03 -0300 Received: from d24av01.br.ibm.com (d24av01.br.ibm.com [9.8.31.91]) by mailhub1.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p2LLFULd360896 for ; Mon, 21 Mar 2011 18:15:30 -0300 Received: from d24av01.br.ibm.com (loopback [127.0.0.1]) by d24av01.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p2LJF14b017098 for ; Mon, 21 Mar 2011 16:15:01 -0300 Received: from [9.12.227.211] ([9.12.227.211]) by d24av01.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id p2LJEwu9016991 for ; Mon, 21 Mar 2011 16:14:59 -0300 Subject: [RFC] Cleanup breakpoint_re_set_one From: Thiago Jung Bauermann To: gdb-patches ml Content-Type: text/plain; charset="UTF-8" Date: Tue, 22 Mar 2011 00:08:00 -0000 Message-ID: <1300742094.2557.10.camel@hactar> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit x-cbid: 11032121-8936-0000-0000-00000162728B X-IsSubscribed: yes 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 X-SW-Source: 2011-03/txt/msg00980.txt.bz2 Hi, I was studying breakpoint_re_set_one and had to scratch my head for a while before I could understand the error handling logic in the breakpoints case. There's an unusually high incidence of double negatives and booleans which when true mean that something is _not_ going on... I considered changing b->condition_not_parsed to b->condition_parsed, but that would be a very big and error-prone change. IMHO this rearrangement of the expression is a good first step. Changing decode_line_1's not_found to found would probably be easier, but since I don't have a pressing need for that I didn't attempt to do so here. I can provide a separate patch if you think it's worthwhile. Tested with no regressions on ppc-linux and ppc64-linux. WDYT? -- []'s Thiago Jung Bauermann IBM Linux Technology Center 2011-03-21 Thiago Jung Bauermann * breakpoint.c (breakpoint_re_set_one): Cleanup logic of the conditon expressions in the error handling code for breakpoints. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index c55e5c0..16fc508 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -10545,33 +10545,26 @@ breakpoint_re_set_one (void *bint) sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, (char ***) NULL, not_found_ptr); } - if (e.reason < 0) + + /* For pending breakpoints, it's expected that parsing will + fail until the right shared library is loaded. User has + already told to create pending breakpoints and don't need + extra messages. If breakpoint is in bp_shlib_disabled + state, then user already saw the message about that + breakpoint being disabled, and don't want to see more + errors. */ + if (e.reason < 0 && (!not_found || (!b->condition_not_parsed + && !(b->loc && b->loc->shlib_disabled) + && b->enable_state != bp_disabled))) { - int not_found_and_ok = 0; - /* For pending breakpoints, it's expected that parsing will - fail until the right shared library is loaded. User has - already told to create pending breakpoints and don't need - extra messages. If breakpoint is in bp_shlib_disabled - state, then user already saw the message about that - breakpoint being disabled, and don't want to see more - errors. */ - if (not_found - && (b->condition_not_parsed - || (b->loc && b->loc->shlib_disabled) - || b->enable_state == bp_disabled)) - not_found_and_ok = 1; - - if (!not_found_and_ok) - { - /* We surely don't want to warn about the same breakpoint - 10 times. One solution, implemented here, is disable - the breakpoint on error. Another solution would be to - have separate 'warning emitted' flag. Since this - happens only when a binary has changed, I don't know - which approach is better. */ - b->enable_state = bp_disabled; - throw_exception (e); - } + /* We surely don't want to warn about the same breakpoint + 10 times. One solution, implemented here, is disable + the breakpoint on error. Another solution would be to + have separate 'warning emitted' flag. Since this + happens only when a binary has changed, I don't know + which approach is better. */ + b->enable_state = bp_disabled; + throw_exception (e); } if (!not_found)