[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gdb-patches
Subject:    I think permanent breakpoints are fundamentally broken as is
From:       aburgess () broadcom ! com (Andrew Burgess)
Date:       2013-10-29 17:52:00
Message-ID: 526FF5D7.7000909 () broadcom ! com
[Download RAW message or body]

On 18/10/2013 5:17 PM, Pedro Alves wrote:
> On 10/18/2013 03:47 PM, Andrew Burgess wrote:
> > This patch:
> > https://sourceware.org/ml/gdb-patches/2012-01/msg00964.html
> > 
> > introduced what I believe is a stray line that causes permanent
> > breakpoints to become normal breakpoints if the user ever tries
> > to "enable" the permanent breakpoint.
> 
> I actually think "permanent breakpoints" are quite weird beasts,
> both from a user interface, and implementation perspectives.

<snip: lots of good points about permanent breakpoints>

OK, given all you've said I'd like to just commit the patch below.  This is basically \
removing the stray line I mention above but without adding any new tests.

I'd never even heard about "permanent breakpoints" before I spotted the odd looking \
extra line, so only added the tests as "good practice" to  ensure the same bug was \
not added again.

Given that we're not really sure exactly how permanent breakpoints should operate I \
think just removing the stray line for now would be best, then if anyone re-works \
permanent breakpoints they'll not have to find/consider this tiny "ooops".

OK to apply?

Thanks,
Andrew

gdb/ChangeLog

2013-10-29  Andrew Burgess  <aburgess@broadcom.com>

	* breakpoint.c (enable_breakpoint_disp): Remove setting of
	enabled_state for permanent breakpoints.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 608463d..b5bb3da 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14725,8 +14725,6 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp \
disposition,  if (bpt->enable_state != bp_permanent)
     bpt->enable_state = bp_enabled;
 
-  bpt->enable_state = bp_enabled;
-
   /* Mark breakpoint locations modified.  */
   mark_breakpoint_modified (bpt);
 


[prev in list] [next in list] [prev in thread] [next in thread]