From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14768 invoked by alias); 3 Jul 2008 08:48:44 -0000 Received: (qmail 14757 invoked by uid 22791); 3 Jul 2008 08:48:43 -0000 X-Spam-Check-By: sourceware.org Received: from main.gmane.org (HELO ciao.gmane.org) (80.91.229.2) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 03 Jul 2008 08:48:20 +0000 Received: from list by ciao.gmane.org with local (Exim 4.43) id 1KEKUC-0006se-US for gdb-patches@sources.redhat.com; Thu, 03 Jul 2008 08:48:17 +0000 Received: from 78.158.192.230 ([78.158.192.230]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 03 Jul 2008 08:48:16 +0000 Received: from vladimir by 78.158.192.230 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 03 Jul 2008 08:48:16 +0000 To: gdb-patches@sources.redhat.com From: Vladimir Prus Subject: Re: functions that delete breakpoints should not insert bp locations (was: Fix execl.exp sporadic failures) Date: Thu, 03 Jul 2008 08:48:00 -0000 Message-ID: References: <200807030226.54896.pedro@codesourcery.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8Bit User-Agent: KNode/0.10.9 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: 2008-07/txt/msg00029.txt.bz2 Pedro Alves wrote: > static void > -update_global_location_list (void) > +update_global_location_list (int inserting) There's no comment for the new parameter. Also, the parameter tells the function to do, or not do something, so "-ing" sounds awkward. How about 'should_insert' or 'suppress_insert'? > { > struct breakpoint *b; > struct bp_location **next = &bp_location_chain; > @@ -7132,7 +7123,11 @@ update_global_location_list (void) > check_duplicates (b); > } > > -  if (always_inserted_mode && target_has_execution) > +  /* If we get here due to deleting a breakpoint, don't try to insert > +     locations.   This does not tell how 'inserting' is related to 'deleting a breakpoint'. > This helps cases like: target_detach deleting a > +     breakpoint before detaching causing all other breakpoints to be > +     inserted.  */ I'd suggest to remove this comment, and add a comment before function saying this: /* If SUPPRESS_INSERT is true, do not insert any breakpoint locations into target, only remove already-inserted locations that no longer should be inserted. This behaviour is useful during tear-down -- when the target no longer is capable of inserting breakpoints, and we're removing internal GDB breakpoints, we don't want to try inserting other breakpoints into already-gone target. */ As we've already discussed, I think the ideal solution is to mark the target is 'going down' and suppress insertion of breakpoints if the target is in that state. Your patch, however, should be almost as good, so I don't have any objections. Thanks for addressing this issue! - Volodya