This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: I think permanent breakpoints are fundamentally broken as is
- From: "Andrew Burgess" <aburgess at broadcom dot com>
- To: "Pedro Alves" <palves at redhat dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, lgustavo at codesourcery dot com
- Date: Tue, 29 Oct 2013 17:52:23 +0000
- Subject: Re: I think permanent breakpoints are fundamentally broken as is
- Authentication-results: sourceware.org; auth=none
- References: <52614A15 dot 7070301 at broadcom dot com> <52615F0B dot 4050008 at redhat dot 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);