From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15109 invoked by alias); 11 Dec 2003 14:21:22 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 15102 invoked from network); 11 Dec 2003 14:21:21 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 11 Dec 2003 14:21:21 -0000 Received: from drow by nevyn.them.org with local (Exim 4.24 #1 (Debian)) id 1AURgt-0006ux-N8; Thu, 11 Dec 2003 09:21:19 -0500 Date: Thu, 11 Dec 2003 14:21:00 -0000 From: Daniel Jacobowitz To: Eli Zaretskii Cc: Jeff Johnston , gdb-patches@sources.redhat.com Subject: Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support) Message-ID: <20031211142119.GA26428@nevyn.them.org> Mail-Followup-To: Eli Zaretskii , Jeff Johnston , gdb-patches@sources.redhat.com References: <3FD7C458.2080404@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.1i X-SW-Source: 2003-12/txt/msg00317.txt.bz2 On Thu, Dec 11, 2003 at 08:01:58AM +0200, Eli Zaretskii wrote: > The second comment is about this hunk of changes: > > > @@ -2574,9 +2581,7 @@ bpstat_stop_status (CORE_ADDR *pc, int n > > > > ALL_BREAKPOINTS_SAFE (b, temp) > > { > > - if (b->enable_state == bp_disabled > > - || b->enable_state == bp_shlib_disabled > > - || b->enable_state == bp_call_disabled) > > + if (!breakpoint_enabled (b) && b->enable_state != bp_permanent) > > continue; > > Bother. Is it really wise to replace an explicit check of equality to > several bp_* constants with "!= bp_permanent"? Are we sure that any > non-bp_permanent breakpoint should pass this test, even if in the > future additional bp_* constants will be introduced that aren't there > now? I asked Jeff to do that, so I'll step in here :) Right now, there are five possible enable states: enabled disabled permanent call_disabled shlib_disabled I'm not convinced that permanent should even be on the list. It's a real oddball; and there's no reason that GDB couldn't virtually "disable" a permanent breakpoint (step over it automatically when hitting it; give it an always-false condition, in effect). So the others boil down to a group of enabled breakpoint states and a group of disabled breakpoint states. The body of the bpstat_stop_status loop only cares about enabled breakpoints, and for its purposes permanent breakpoints are enabled (because they might be the reason that we stopped). So I think the new test is logically correct. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer