Re: I think permanent breakpoints are fundamentally broken as is

["Andrew Burgess" <aburgess@broadcom.com>]

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);