[RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Jeff Johnston]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

The following is a patch to centralize the test for an active-enabled 
breakpoint.   This patch is in response to Daniel J's comments to split 
up the pending-breakpoint support patch.   With pending breakpoint 
support, the test will be modified to include a check for a pending 
breakpoint.  This will simplify reviewing the last of the patches 
required for pending-breakpoint support.

Ok to commit?

-- Jeff J.

2003-12-10  Jeff Johnston  <jjohnstn@redhat.com>

    * breakpoint.c (breakpoint_enabled): New function to test whether 
breakpoint is
    active and enabled.
    ( insert_bp_location, insert_breakpoints): Call new function to test
    for enabled breakpoint.
    (remove_breakpoint, breakpoint_here_p): Ditto.
    (breakpoint_thread_match): Ditto.
    (bpstat_should_step, bpstat_have_active_hw_watchpoints): Ditto.
    (disable_breakpoints_in_shlibs): Ditto.
    (hw_watchpoint_used_count): Ditto.
    (disable_watchpoints_before_interactive_call_start): Ditto.
    (breakpoint_re_set_one): Ditto.
    (bpstat_stop_status): Use new function and simplify test.


[-- Attachment #2: breakpoint.patch --]
[-- Type: text/plain, Size: 5843 bytes --]

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.145
diff -u -p -r1.145 breakpoint.c
--- breakpoint.c	17 Nov 2003 00:55:49 -0000	1.145
+++ breakpoint.c	10 Dec 2003 21:53:31 -0000
@@ -332,6 +332,13 @@ int exception_support_initialized = 0;
    error ("catch of library unloads not yet implemented on this platform")
 #endif
 
+/* Return whether a breakpoint is an active enabled breakpoint.  */
+static int
+breakpoint_enabled (struct breakpoint *b)
+{
+  return b->enable_state == bp_enabled;
+}
+
 /* Set breakpoint count to NUM.  */
 
 void
@@ -757,7 +764,7 @@ insert_bp_location (struct bp_location *
 
   /* Permanent breakpoints cannot be inserted or removed.  Disabled
      breakpoints should not be inserted.  */
-  if (bpt->owner->enable_state != bp_enabled)
+  if (!breakpoint_enabled (bpt->owner))
     return 0;
 
   if (bpt->inserted || bpt->duplicate)
@@ -1107,7 +1114,7 @@ insert_breakpoints (void)
     {
       /* Permanent breakpoints cannot be inserted or removed.  Disabled
 	 breakpoints should not be inserted.  */
-      if (b->owner->enable_state != bp_enabled)
+      if (!breakpoint_enabled (b->owner))
 	continue;
 
       /* FIXME drow/2003-10-07: This code should be pushed elsewhere when
@@ -1457,7 +1464,7 @@ remove_breakpoint (struct bp_location *b
       b->inserted = (is == mark_inserted);
     }
   else if (b->loc_type == bp_loc_hardware_watchpoint
-	   && b->owner->enable_state == bp_enabled
+	   && breakpoint_enabled (b->owner)
 	   && !b->duplicate)
     {
       struct value *v;
@@ -1513,7 +1520,7 @@ remove_breakpoint (struct bp_location *b
   else if ((b->owner->type == bp_catch_fork ||
 	    b->owner->type == bp_catch_vfork ||
 	    b->owner->type == bp_catch_exec)
-	   && b->owner->enable_state == bp_enabled
+	   && breakpoint_enabled (b->owner)
 	   && !b->duplicate)
     {
       val = -1;
@@ -1538,7 +1545,7 @@ remove_breakpoint (struct bp_location *b
     }
   else if ((b->owner->type == bp_catch_catch ||
 	    b->owner->type == bp_catch_throw)
-	   && b->owner->enable_state == bp_enabled
+	   && breakpoint_enabled (b->owner)
 	   && !b->duplicate)
     {
 
@@ -1549,7 +1556,7 @@ remove_breakpoint (struct bp_location *b
     }
   else if (ep_is_exception_catchpoint (b->owner)
 	   && b->inserted	/* sometimes previous insert doesn't happen */
-	   && b->owner->enable_state == bp_enabled
+	   && breakpoint_enabled (b->owner)
 	   && !b->duplicate)
     {
 
@@ -1675,7 +1682,7 @@ breakpoint_here_p (CORE_ADDR pc)
 	  && bpt->loc_type != bp_loc_hardware_breakpoint)
 	continue;
 
-      if ((bpt->owner->enable_state == bp_enabled
+      if ((breakpoint_enabled (bpt->owner)
 	   || bpt->owner->enable_state == bp_permanent)
 	  && bpt->address == pc)	/* bp is enabled and matches pc */
 	{
@@ -1772,7 +1779,7 @@ breakpoint_thread_match (CORE_ADDR pc, p
 	  && bpt->loc_type != bp_loc_hardware_breakpoint)
 	continue;
 
-      if ((bpt->owner->enable_state == bp_enabled
+      if ((breakpoint_enabled (bpt->owner)
 	   || bpt->owner->enable_state == bp_permanent)
 	  && bpt->address == pc
 	  && (bpt->owner->thread == -1 || bpt->owner->thread == thread))
@@ -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;
 
     if (b->type != bp_watchpoint
@@ -3179,7 +3184,7 @@ bpstat_should_step (void)
 {
   struct breakpoint *b;
   ALL_BREAKPOINTS (b)
-    if (b->enable_state == bp_enabled && b->type == bp_watchpoint)
+    if (breakpoint_enabled (b) && b->type == bp_watchpoint)
       return 1;
   return 0;
 }
@@ -3190,7 +3195,7 @@ bpstat_have_active_hw_watchpoints (void)
 {
   struct bp_location *bpt;
   ALL_BP_LOCATIONS (bpt)
-    if ((bpt->owner->enable_state == bp_enabled)
+    if (breakpoint_enabled (bpt->owner)
 	&& bpt->inserted
 	&& bpt->loc_type == bp_loc_hardware_watchpoint)
       return 1;
@@ -4268,7 +4273,7 @@ disable_breakpoints_in_shlibs (int silen
 #if defined (PC_SOLIB)
     if (((b->type == bp_breakpoint) ||
 	 (b->type == bp_hardware_breakpoint)) &&
-	b->enable_state == bp_enabled &&
+	breakpoint_enabled (b) &&
 	!b->loc->duplicate &&
 	PC_SOLIB (b->loc->address))
       {
@@ -4491,14 +4496,13 @@ hw_watchpoint_used_count (enum bptype ty
   *other_type_used = 0;
   ALL_BREAKPOINTS (b)
   {
-    if (b->enable_state == bp_enabled)
+    if (breakpoint_enabled (b))
       {
 	if (b->type == type)
 	  i++;
 	else if ((b->type == bp_hardware_watchpoint ||
 		  b->type == bp_read_watchpoint ||
-		  b->type == bp_access_watchpoint)
-		 && b->enable_state == bp_enabled)
+		  b->type == bp_access_watchpoint))
 	  *other_type_used = 1;
       }
   }
@@ -4540,7 +4544,7 @@ disable_watchpoints_before_interactive_c
 	 || (b->type == bp_read_watchpoint)
 	 || (b->type == bp_access_watchpoint)
 	 || ep_is_exception_catchpoint (b))
-	&& (b->enable_state == bp_enabled))
+	&& breakpoint_enabled (b))
       {
 	b->enable_state = bp_call_disabled;
 	check_duplicates (b);
@@ -7063,7 +7067,7 @@ breakpoint_re_set_one (void *bint)
 	value_free (b->val);
       b->val = evaluate_expression (b->exp);
       release_value (b->val);
-      if (VALUE_LAZY (b->val) && b->enable_state == bp_enabled)
+      if (VALUE_LAZY (b->val) && breakpoint_enabled (b))
 	value_fetch_lazy (b->val);
 
       if (b->cond_string != NULL)
@@ -7073,7 +7077,7 @@ breakpoint_re_set_one (void *bint)
 	    xfree (b->cond);
 	  b->cond = parse_exp_1 (&s, (struct block *) 0, 0);
 	}
-      if (b->enable_state == bp_enabled)
+      if (breakpoint_enabled (b))
 	mention (b);
       value_free_to_mark (mark);
       break;

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Daniel Jacobowitz]

On Wed, Dec 10, 2003 at 08:11:52PM -0500, Jeff Johnston wrote:
> The following is a patch to centralize the test for an active-enabled 
> breakpoint.   This patch is in response to Daniel J's comments to split 
> up the pending-breakpoint support patch.   With pending breakpoint 
> support, the test will be modified to include a check for a pending 
> breakpoint.  This will simplify reviewing the last of the patches 
> required for pending-breakpoint support.
> 
> Ok to commit?
> 
> -- Jeff J.
> 
> 2003-12-10  Jeff Johnston  <jjohnstn@redhat.com>
> 
>    * breakpoint.c (breakpoint_enabled): New function to test whether 
> breakpoint is
>    active and enabled.
>    ( insert_bp_location, insert_breakpoints): Call new function to test
>    for enabled breakpoint.
>    (remove_breakpoint, breakpoint_here_p): Ditto.
>    (breakpoint_thread_match): Ditto.
>    (bpstat_should_step, bpstat_have_active_hw_watchpoints): Ditto.
>    (disable_breakpoints_in_shlibs): Ditto.
>    (hw_watchpoint_used_count): Ditto.
>    (disable_watchpoints_before_interactive_call_start): Ditto.
>    (breakpoint_re_set_one): Ditto.
>    (bpstat_stop_status): Use new function and simplify test.

This is fine!  Thanks for splitting it up.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Eli Zaretskii]

> Date: Wed, 10 Dec 2003 20:11:52 -0500
> From: Jeff Johnston <jjohnstn@redhat.com>
> 
> Ok to commit?

I have 2 very minor comments.  The first one is about the ChangeLog
entries:

> 2003-12-10  Jeff Johnston  <jjohnstn@redhat.com>
> 
>     * breakpoint.c (breakpoint_enabled): New function to test whether breakpoint is
>     active and enabled.

Is this line really that long, or did your mailer mess it up?  If the
former, it needs to be reformatted.

>     ( insert_bp_location, insert_breakpoints): Call new function to test
>     for enabled breakpoint.
>     (remove_breakpoint, breakpoint_here_p): Ditto.
>     (breakpoint_thread_match): Ditto.
>     (bpstat_should_step, bpstat_have_active_hw_watchpoints): Ditto.
>     (disable_breakpoints_in_shlibs): Ditto.
>     (hw_watchpoint_used_count): Ditto.
>     (disable_watchpoints_before_interactive_call_start): Ditto.
>     (breakpoint_re_set_one): Ditto.

Instead of the long series of "(func): Ditto." kind of entries, it's
better to make a single multi-line entry, like this:

    (remove_breakpoint, breakpoint_here_p, breakpoint_thread_match)
    (bpstat_should_step, bpstat_have_active_hw_watchpoints)
    (disable_breakpoints_in_shlibs, hw_watchpoint_used_count)
    (disable_watchpoints_before_interactive_call_start)
    (breakpoint_re_set_one): Ditto.

(Note how every line ends with a right paren: it's important for
Emacs to highlight the function names correctly.)

Also, please make sure each line of the ChangeLog entry begins with a
literal TAB character.

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?

Thanks.

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Daniel Jacobowitz]

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

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Eli Zaretskii]

> Date: Thu, 11 Dec 2003 09:21:19 -0500
> From: Daniel Jacobowitz <drow@mvista.com>
> 
> I asked Jeff to do that, so I'll step in here :)

Thanks, I don't bother anymore ;-)

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[J. Johnston]

Eli Zaretskii wrote:
>>Date: Wed, 10 Dec 2003 20:11:52 -0500
>>From: Jeff Johnston <jjohnstn@redhat.com>
>>
>>Ok to commit?
> 
> 
> I have 2 very minor comments.  The first one is about the ChangeLog
> entries:
> 
> 
>>2003-12-10  Jeff Johnston  <jjohnstn@redhat.com>
>>
>>    * breakpoint.c (breakpoint_enabled): New function to test whether breakpoint is
>>    active and enabled.
> 
> 
> Is this line really that long, or did your mailer mess it up?  If the
> former, it needs to be reformatted.
>

Eli, I realize you are just making a minor comment, but can I ask that gdb 
maintainers please start trusting me on this.  My ChangeLog entries are just 
typed into my note (i.e. I do not cut and paste from the actual ChangeLog).  I 
"always" retype the ChangeLog entry when and if the patch is accepted so the 
line length and white-spacing you see in the note is completely moot.  If 
anybody is unhappy with my previous ChangeLog entries, feel free to let me know.

>>    ( insert_bp_location, insert_breakpoints): Call new function to test
>>    for enabled breakpoint.
>>    (remove_breakpoint, breakpoint_here_p): Ditto.
>>    (breakpoint_thread_match): Ditto.
>>    (bpstat_should_step, bpstat_have_active_hw_watchpoints): Ditto.
>>    (disable_breakpoints_in_shlibs): Ditto.
>>    (hw_watchpoint_used_count): Ditto.
>>    (disable_watchpoints_before_interactive_call_start): Ditto.
>>    (breakpoint_re_set_one): Ditto.
> 
> 
> Instead of the long series of "(func): Ditto." kind of entries, it's
> better to make a single multi-line entry, like this:
> 
>     (remove_breakpoint, breakpoint_here_p, breakpoint_thread_match)
>     (bpstat_should_step, bpstat_have_active_hw_watchpoints)
>     (disable_breakpoints_in_shlibs, hw_watchpoint_used_count)
>     (disable_watchpoints_before_interactive_call_start)
>     (breakpoint_re_set_one): Ditto.
> 

Ok, will do.

> (Note how every line ends with a right paren: it's important for
> Emacs to highlight the function names correctly.)
> 
> Also, please make sure each line of the ChangeLog entry begins with a
> literal TAB character.
> 
> 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?
> 

No I can't predict possible future enable states.  However, the change was 
suggested by Daniel and he is much closer to the code than I am.  I would think 
that whatever new value was added, all tests of the enable_state would have to 
be analyzed and dealt with; this one included.  I have no problems with changing 
it to back to a simple test if people are uncomfortable with it.

-- Jeff J.

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Eli Zaretskii]

> Date: Thu, 11 Dec 2003 11:32:46 -0500
> From: "J. Johnston" <jjohnstn@redhat.com>
> > 
> > Is this line really that long, or did your mailer mess it up?  If the
> > former, it needs to be reformatted.
> 
> Eli, I realize you are just making a minor comment, but can I ask that gdb 
> maintainers please start trusting me on this.

Sorry, I didn't in any way mean to say that I didn't trust you.  I
couldn't possibly know whether what I saw was a real mistake on your
part or not, and I think it isn't reasonable to request me to check
your previous ChangeLog entries in order to decide one way or the
other.  One popular reason for messed-up formatting is your mailer,
so I took care to inquire about that before assuming that what I see
is what you meant.

Could we please assume that the comments you get are meant to make
sure the code is up to the standards, not to diminish your abilities
or show signs of mistrust?

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[J. Johnston]

Eli Zaretskii wrote:
>>Date: Thu, 11 Dec 2003 11:32:46 -0500
>>From: "J. Johnston" <jjohnstn@redhat.com>
>>
>>>Is this line really that long, or did your mailer mess it up?  If the
>>>former, it needs to be reformatted.
>>
>>Eli, I realize you are just making a minor comment, but can I ask that gdb 
>>maintainers please start trusting me on this.
> 
> 
> Sorry, I didn't in any way mean to say that I didn't trust you.  I
> couldn't possibly know whether what I saw was a real mistake on your
> part or not, and I think it isn't reasonable to request me to check
> your previous ChangeLog entries in order to decide one way or the
> other.  One popular reason for messed-up formatting is your mailer,
> so I took care to inquire about that before assuming that what I see
> is what you meant.
> 
> Could we please assume that the comments you get are meant to make
> sure the code is up to the standards, not to diminish your abilities
> or show signs of mistrust?
> 
> 

No need to apologize.  I wasn't trying to imply that you didn't trust me nor was 
I offended in any way.  The request was to all gdb global maintainers out there. 
  The comment about the line length for my ChangeLog entries comes up 
continously and I thought it was time I just pointed out that I am on the same 
page as everyone else with regards to this.

-- Jeff J.

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Daniel Jacobowitz]

On Thu, Dec 11, 2003 at 02:33:41PM -0500, J. Johnston wrote:
> Eli Zaretskii wrote:
> >>Date: Thu, 11 Dec 2003 11:32:46 -0500
> >>From: "J. Johnston" <jjohnstn@redhat.com>
> >>
> >>>Is this line really that long, or did your mailer mess it up?  If the
> >>>former, it needs to be reformatted.
> >>
> >>Eli, I realize you are just making a minor comment, but can I ask that 
> >>gdb maintainers please start trusting me on this.
> >
> >
> >Sorry, I didn't in any way mean to say that I didn't trust you.  I
> >couldn't possibly know whether what I saw was a real mistake on your
> >part or not, and I think it isn't reasonable to request me to check
> >your previous ChangeLog entries in order to decide one way or the
> >other.  One popular reason for messed-up formatting is your mailer,
> >so I took care to inquire about that before assuming that what I see
> >is what you meant.
> >
> >Could we please assume that the comments you get are meant to make
> >sure the code is up to the standards, not to diminish your abilities
> >or show signs of mistrust?
> >
> >
> 
> No need to apologize.  I wasn't trying to imply that you didn't trust me 
> nor was I offended in any way.  The request was to all gdb global 
> maintainers out there. The comment about the line length for my ChangeLog 
>  entries comes up continously and I thought it was time I just pointed out 
> that I am on the same page as everyone else with regards to this.

We can't stop complaining about misformated entries for everybody,
since a lot of people get them wrong - I do this routinely myself. 
It's a common problem.

I think that the same rules for posting patches should apply to posting
changelog entries - it's not acceptable to post a different patch than
you'll check in, after all.

Just my two cents.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> 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).

I'm responsible for adding the permanent breakpoint kludge.

Permanent breakpoints were added for HP-UX, where the dynamic linker
lives in a special region of memory that the debugger cannot modify.
The breakpoint instruction is hard-coded into the function; and GDB
cannot write to that address to remove it.  pa64solib.c seems to be
the only code that creates them.

In an earlier message, you said:
> It's not clear what to do with permanent breakpoints (I don't think
> that should be an enable state, long term!) so I chose the version
> with minimal textual changes.

Do you mean, here, that having breakpoint_enabled return false for
permanent breakpoints results in fewer changes overall?  I would
expect the opposite --- permanent breakpoints really are enabled.

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Daniel Jacobowitz]

On Thu, Dec 11, 2003 at 03:33:34PM -0500, Jim Blandy wrote:
> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > 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).
> 
> I'm responsible for adding the permanent breakpoint kludge.
> 
> Permanent breakpoints were added for HP-UX, where the dynamic linker
> lives in a special region of memory that the debugger cannot modify.
> The breakpoint instruction is hard-coded into the function; and GDB
> cannot write to that address to remove it.  pa64solib.c seems to be
> the only code that creates them.
> 
> In an earlier message, you said:
> > It's not clear what to do with permanent breakpoints (I don't think
> > that should be an enable state, long term!) so I chose the version
> > with minimal textual changes.
> 
> Do you mean, here, that having breakpoint_enabled return false for
> permanent breakpoints results in fewer changes overall?  I would
> expect the opposite --- permanent breakpoints really are enabled.

Yes, but most code either:
  - Doesn't know about permanent breakpoints
or
  - Doesn't want to deal with permanent breakpoints

Take a look at the patch Jeff posted yesterday for breakpoint.c to see
how this happens.  I suspect it's mostly the former; if you wanted to
add permanent breakpoints to breakpoint_enabled() you'd have to review
all its callers to see the effect (which would be easier now).

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Jim Blandy]

Jim Blandy <jimb@redhat.com> writes:
> Do you mean, here, that having breakpoint_enabled return false for
> permanent breakpoints results in fewer changes overall?  I would
> expect the opposite --- permanent breakpoints really are enabled.

(Just to be clear --- I don't object to the patch.)

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Eli Zaretskii]

> Date: Thu, 11 Dec 2003 14:33:41 -0500
> From: "J. Johnston" <jjohnstn@redhat.com>
> 
>   The comment about the line length for my ChangeLog entries comes
> up continously and I thought it was time I just pointed out that I
> am on the same page as everyone else with regards to this.

In that case, it is IMHO unreasonable to ask the maintainers to
remember such miniscule details about specific contributors.  I
suggest, like Daniel did, that you simply use the exact ChangeLog
entry in your message.  It is, after all, very easy to do: simply
always make an entry when you produce the patch to send to the list,
don't postpone that until the patch is approved.  As a side-effect,
you will not need to remember all the changes you did back then with
their reasons.

Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[J. Johnston]

Patch checked in.


Eli Zaretskii wrote:
>>Date: Thu, 11 Dec 2003 09:21:19 -0500
>>From: Daniel Jacobowitz <drow@mvista.com>
>>
>>I asked Jeff to do that, so I'll step in here :)
> 
> 
> Thanks, I don't bother anymore ;-)
> 
>