Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
       [not found] <Pine.GSO.4.33.0105102023220.29679-100000@ryobi.cygnus.com>
@ 2001-05-10 22:26 ` Fernando Nasser
  2001-05-10 23:56   ` Eli Zaretskii
  2001-05-11  7:05   ` Keith Seitz
  2001-05-11 18:56 ` Andrew Cagney
  1 sibling, 2 replies; 12+ messages in thread
From: Fernando Nasser @ 2001-05-10 22:26 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

Keith Seitz wrote:
> 
> Hi,
> 
> Currently ALL breakpoint modifications, deletions, creations generate an
> event. I don't think a user interface really cares about the "internal"
> breakpoints.
> 
> This patch adds a new macro which assesses whether a given breakpoint
> should generate an event.
> 
> Michael,JimB: your input on whether I've gotten all the "interesting" ones
> is especially welcome.
> 
> Keith
> 

It is OK to only generate events for visible breakpoints, but the hook
must run for all breakpoints.  Whatever is using the hook may need to
know about the internal ones as well.

F.


> ChangeLog:
> 2001-05-10  Keith Seitz  <keiths@cygnus.com>
> 
>         * breakpoint.c  (REPORT_BREAKPOINT_EVENT): New macro:
>         determines whether a given breakpoint event should be
>         generated.
>         (mention, delete_breakpoint, disable_breakpoint,
>         do_enable_breakpoint): Use REPORT_BREAKPOINT_EVENT to
>         determine if a breakpoint event should be generated for
>         a given breakpoint.
> 
> Patch:
> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 breakpoint.c
> --- breakpoint.c        2001/05/06 22:22:02     1.35
> +++ breakpoint.c        2001/05/11 03:23:55
> @@ -318,6 +318,15 @@ int exception_support_initialized = 0;
>     error ("catch of library unloads not yet implemented on this platform")
>  #endif
> 
> +/* Is this breakpoint interesting to a user interface? */
> +#define REPORT_BREAKPOINT_EVENT(bp) \
> +((bp)->type == bp_breakpoint             \
> + || (bp)->type == bp_hardware_breakpoint \
> + || (bp)->type == bp_watchpoint          \
> + || (bp)->type == bp_hardware_watchpoint \
> + || (bp)->type == bp_read_watchpoint     \
> + || (bp)->type == bp_access_watchpoint)
> +
>  /* Set breakpoint count to NUM.  */
> 
>  void
> @@ -4373,9 +4382,12 @@ mention (struct breakpoint *b)
>       clean this up and at the same time replace the random calls to
>       breakpoint_changed with this hook, as has already been done for
>       delete_breakpoint_hook and so on.  */
> -  if (create_breakpoint_hook)
> -    create_breakpoint_hook (b);
> -  breakpoint_create_event (b->number);
> +  if (REPORT_BREAKPOINT_EVENT (b))
> +    {
> +      if (create_breakpoint_hook)
> +       create_breakpoint_hook (b);
> +      breakpoint_create_event (b->number);
> +    }
> 
>    switch (b->type)
>      {
> @@ -6733,9 +6745,12 @@ delete_breakpoint (struct breakpoint *bp
>    if (bpt->type == bp_none)
>      return;
> 
> -  if (delete_breakpoint_hook)
> -    delete_breakpoint_hook (bpt);
> -  breakpoint_delete_event (bpt->number);
> +  if (REPORT_BREAKPOINT_EVENT (bpt))
> +    {
> +      if (delete_breakpoint_hook)
> +       delete_breakpoint_hook (bpt);
> +      breakpoint_delete_event (bpt->number);
> +    }
> 
>    if (bpt->inserted)
>      remove_breakpoint (bpt, mark_uninserted);
> @@ -7302,9 +7317,12 @@ disable_breakpoint (struct breakpoint *b
> 
>    check_duplicates (bpt);
> 
> -  if (modify_breakpoint_hook)
> -    modify_breakpoint_hook (bpt);
> -  breakpoint_modify_event (bpt->number);
> +  if (REPORT_BREAKPOINT_EVENT (bpt))
> +    {
> +      if (modify_breakpoint_hook)
> +       modify_breakpoint_hook (bpt);
> +      breakpoint_modify_event (bpt->number);
> +    }
>  }
> 
>  /* ARGSUSED */
> @@ -7430,10 +7448,14 @@ have been allocated for other watchpoint
>        if (save_selected_frame_level >= 0)
>         select_frame (save_selected_frame, save_selected_frame_level);
>        value_free_to_mark (mark);
> +    }
> +
> +  if (REPORT_BREAKPOINT_EVENT (bpt))
> +    {
> +      if (modify_breakpoint_hook)
> +       modify_breakpoint_hook (bpt);
> +      breakpoint_modify_event (bpt->number);
>      }
> -  if (modify_breakpoint_hook)
> -    modify_breakpoint_hook (bpt);
> -  breakpoint_modify_event (bpt->number);
>  }
> 
>  void

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
  2001-05-10 22:26 ` [RFA] breakpoint.c: don't generate bp events for internal bps Fernando Nasser
@ 2001-05-10 23:56   ` Eli Zaretskii
  2001-05-11  7:09     ` Keith Seitz
  2001-05-11  7:05   ` Keith Seitz
  1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2001-05-10 23:56 UTC (permalink / raw)
  To: fnasser; +Cc: keiths, gdb-patches

> Date: Fri, 11 May 2001 01:24:45 -0400
> From: Fernando Nasser <fnasser@redhat.com>
> 
> It is OK to only generate events for visible breakpoints, but the hook
> must run for all breakpoints.  Whatever is using the hook may need to
> know about the internal ones as well.

I agree.  But I would go even farther: is it possible to explain why
is it a good idea to make this change?  What exactly is wrong with
generating breakpoint events as we do now?

At the very least, I think we should consider all the uses of the
breakpoint events, to judge whether some of them do indeed want to see
internal breakpoints.  Is such a list available somewhere?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
  2001-05-10 22:26 ` [RFA] breakpoint.c: don't generate bp events for internal bps Fernando Nasser
  2001-05-10 23:56   ` Eli Zaretskii
@ 2001-05-11  7:05   ` Keith Seitz
  2001-05-11  8:54     ` Fernando Nasser
  1 sibling, 1 reply; 12+ messages in thread
From: Keith Seitz @ 2001-05-11  7:05 UTC (permalink / raw)
  To: Fernando Nasser; +Cc: gdb-patches

On Fri, 11 May 2001, Fernando Nasser wrote:

> It is OK to only generate events for visible breakpoints, but the hook
> must run for all breakpoints.  Whatever is using the hook may need to
> know about the internal ones as well.

Despite the fact that NOTHING uses the hook, here is a revised patch:

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.35
diff -u -p -r1.35 breakpoint.c
--- breakpoint.c	2001/05/06 22:22:02	1.35
+++ breakpoint.c	2001/05/11 03:23:55
@@ -318,6 +318,15 @@ int exception_support_initialized = 0;
    error ("catch of library unloads not yet implemented on this platform")
 #endif

+/* Is this breakpoint interesting to a user interface? */
+#define REPORT_BREAKPOINT_EVENT(bp) \
+((bp)->type == bp_breakpoint             \
+ || (bp)->type == bp_hardware_breakpoint \
+ || (bp)->type == bp_watchpoint          \
+ || (bp)->type == bp_hardware_watchpoint \
+ || (bp)->type == bp_read_watchpoint     \
+ || (bp)->type == bp_access_watchpoint)
+
 /* Set breakpoint count to NUM.  */

 void
@@ -4373,9 +4382,12 @@ mention (struct breakpoint *b)
      clean this up and at the same time replace the random calls to
      breakpoint_changed with this hook, as has already been done for
      delete_breakpoint_hook and so on.  */
-  if (create_breakpoint_hook)
-    create_breakpoint_hook (b);
-  breakpoint_create_event (b->number);
+  if (REPORT_BREAKPOINT_EVENT (b))
+    {
+      if (create_breakpoint_hook)
+	create_breakpoint_hook (b);
+      breakpoint_create_event (b->number);
+    }

   switch (b->type)
     {
@@ -6733,9 +6745,12 @@ delete_breakpoint (struct breakpoint *bp
   if (bpt->type == bp_none)
     return;

-  if (delete_breakpoint_hook)
-    delete_breakpoint_hook (bpt);
-  breakpoint_delete_event (bpt->number);
+  if (REPORT_BREAKPOINT_EVENT (bpt))
+    {
+      if (delete_breakpoint_hook)
+	delete_breakpoint_hook (bpt);
+      breakpoint_delete_event (bpt->number);
+    }

   if (bpt->inserted)
     remove_breakpoint (bpt, mark_uninserted);
@@ -7302,9 +7317,12 @@ disable_breakpoint (struct breakpoint *b

   check_duplicates (bpt);

-  if (modify_breakpoint_hook)
-    modify_breakpoint_hook (bpt);
-  breakpoint_modify_event (bpt->number);
+  if (REPORT_BREAKPOINT_EVENT (bpt))
+    {
+      if (modify_breakpoint_hook)
+	modify_breakpoint_hook (bpt);
+      breakpoint_modify_event (bpt->number);
+    }
 }

 /* ARGSUSED */
@@ -7430,10 +7448,14 @@ have been allocated for other watchpoint
       if (save_selected_frame_level >= 0)
 	select_frame (save_selected_frame, save_selected_frame_level);
       value_free_to_mark (mark);
+    }
+
+  if (REPORT_BREAKPOINT_EVENT (bpt))
+    {
+      if (modify_breakpoint_hook)
+	modify_breakpoint_hook (bpt);
+      breakpoint_modify_event (bpt->number);
     }
-  if (modify_breakpoint_hook)
-    modify_breakpoint_hook (bpt);
-  breakpoint_modify_event (bpt->number);
 }

 void


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
  2001-05-10 23:56   ` Eli Zaretskii
@ 2001-05-11  7:09     ` Keith Seitz
  2001-05-11  8:04       ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Keith Seitz @ 2001-05-11  7:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: fnasser, gdb-patches

On Fri, 11 May 2001, Eli Zaretskii wrote:

> > Date: Fri, 11 May 2001 01:24:45 -0400
> > From: Fernando Nasser <fnasser@redhat.com>
> >
> > It is OK to only generate events for visible breakpoints, but the hook
> > must run for all breakpoints.  Whatever is using the hook may need to
> > know about the internal ones as well.
>
> I agree.  But I would go even farther: is it possible to explain why
> is it a good idea to make this change?  What exactly is wrong with
> generating breakpoint events as we do now?

Do the words "it's stupid" mean anything to you? From gdb-events.h:

/* User Interface Events.
   Copyright 1999, 2000, 2001 Free Software Foundation, Inc.

"User Interface Events". User interfaces don't care about internal
breakpoints -- only user-set breakpoints. IMO all of the breakpoints
excluded from the event trigger are BACKEND breakpoints, and they should
not ever be exposed to users (or non-backend code).

If someone writing ui code needs to know about internal breakpoints, he
doing something wrong.

Keith



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
  2001-05-11  7:09     ` Keith Seitz
@ 2001-05-11  8:04       ` Eli Zaretskii
  2001-05-11  8:14         ` Keith Seitz
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2001-05-11  8:04 UTC (permalink / raw)
  To: keiths; +Cc: fnasser, gdb-patches

> Date: Fri, 11 May 2001 07:08:31 -0700 (PDT)
> From: Keith Seitz <keiths@cygnus.com>
> 
> On Fri, 11 May 2001, Eli Zaretskii wrote:
> 
> > > Date: Fri, 11 May 2001 01:24:45 -0400
> > > From: Fernando Nasser <fnasser@redhat.com>
> > >
> > > It is OK to only generate events for visible breakpoints, but the hook
> > > must run for all breakpoints.  Whatever is using the hook may need to
> > > know about the internal ones as well.
> >
> > I agree.  But I would go even farther: is it possible to explain why
> > is it a good idea to make this change?  What exactly is wrong with
> > generating breakpoint events as we do now?
> 
> Do the words "it's stupid" mean anything to you?

I looked them up ;-)

> From gdb-events.h:
> 
> /* User Interface Events.
>    Copyright 1999, 2000, 2001 Free Software Foundation, Inc.
> 
> "User Interface Events". User interfaces don't care about internal
> breakpoints -- only user-set breakpoints. IMO all of the breakpoints
> excluded from the event trigger are BACKEND breakpoints, and they should
> not ever be exposed to users (or non-backend code).
> 
> If someone writing ui code needs to know about internal breakpoints, he
> doing something wrong.

Sorry, I still don't get the relevance of this to what I asked.
Perhaps I'm stupid.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
  2001-05-11  8:04       ` Eli Zaretskii
@ 2001-05-11  8:14         ` Keith Seitz
  2001-05-11  9:57           ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Keith Seitz @ 2001-05-11  8:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: fnasser, gdb-patches

On Fri, 11 May 2001, Eli Zaretskii wrote:

> > Do the words "it's stupid" mean anything to you?
>
> I looked them up ;-)

Touche!

> > From gdb-events.h:
> >
> > /* User Interface Events.
> >    Copyright 1999, 2000, 2001 Free Software Foundation, Inc.
> >
> > "User Interface Events". User interfaces don't care about internal
> > breakpoints -- only user-set breakpoints. IMO all of the breakpoints
> > excluded from the event trigger are BACKEND breakpoints, and they should
> > not ever be exposed to users (or non-backend code).
> >
> > If someone writing ui code needs to know about internal breakpoints, he
> > doing something wrong.
>
> Sorry, I still don't get the relevance of this to what I asked.
> Perhaps I'm stupid.

No, I'm not explaining myself properly... Let me try again:

breakpoint_delete_event, breakpoint_create_event, and
breakpoint_modify_event are defined in gdb-events.[ch],sh. They are, as
the comments from the files explain, for user interface events.

Well, it is my contention that user interfaces need not (indeed, should
not) know anything about bp_none, bp_until, bp_finish, bp_longjmp,
bp_longjmp_resume, bp_step_resume, bp_through_sigtramp,
bp_watchpoint_scope, bp_call_dummy, bp_shlibevent, bp_thread_event,
bp_catch_unload, bp_catch_fork, bp_catch_vfork, bp_catch_exec,
bp_catch_catch, and bp_catch_throw breakpoint events (or some major subset
of these). What libgdb does for its own housekeeping is its business, and
its business alone. Breakpoints of these types are PRIVATE to gdb.

(I suspect this isn't helping too much, is it?)

I would even go further to argue with Fernando that we need to eliminate
modify_breakpoint_hook, delete_breakpoint_hook, and
create_breakpoint_hook, because these are not hooks: they are events. We
already have an event-reporting mechanism. Let people use it. :-)

I'm in the mood for making friends today! O:-)
Keith



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
  2001-05-11  7:05   ` Keith Seitz
@ 2001-05-11  8:54     ` Fernando Nasser
  0 siblings, 0 replies; 12+ messages in thread
From: Fernando Nasser @ 2001-05-11  8:54 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

Keith Seitz wrote:
> 
> On Fri, 11 May 2001, Fernando Nasser wrote:
> 
> > It is OK to only generate events for visible breakpoints, but the hook
> > must run for all breakpoints.  Whatever is using the hook may need to
> > know about the internal ones as well.
> 
> Despite the fact that NOTHING uses the hook, here is a revised patch:
> 

Keith, you have attached the same old patch!


I am not against removing the hook.  I would not be so certain that
NOTHING is using it as there are too many obscure academic projects
going on using Free Software.  That is how Free Software started (and
how I started hacking GDB) and I would not like to discard them.  But,
whoever is doing anything with GDB should be monitoring this list.  So,
if you post to gdb@sources saying that you will get rid of these hooks
and nobody answers in a reasonable time, "off with their heads"!



> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 breakpoint.c
> --- breakpoint.c        2001/05/06 22:22:02     1.35
> +++ breakpoint.c        2001/05/11 03:23:55
> @@ -318,6 +318,15 @@ int exception_support_initialized = 0;
>     error ("catch of library unloads not yet implemented on this platform")
>  #endif
> 
> +/* Is this breakpoint interesting to a user interface? */
> +#define REPORT_BREAKPOINT_EVENT(bp) \
> +((bp)->type == bp_breakpoint             \
> + || (bp)->type == bp_hardware_breakpoint \
> + || (bp)->type == bp_watchpoint          \
> + || (bp)->type == bp_hardware_watchpoint \
> + || (bp)->type == bp_read_watchpoint     \
> + || (bp)->type == bp_access_watchpoint)
> +
>  /* Set breakpoint count to NUM.  */
> 
>  void
> @@ -4373,9 +4382,12 @@ mention (struct breakpoint *b)
>       clean this up and at the same time replace the random calls to
>       breakpoint_changed with this hook, as has already been done for
>       delete_breakpoint_hook and so on.  */
> -  if (create_breakpoint_hook)
> -    create_breakpoint_hook (b);
> -  breakpoint_create_event (b->number);
> +  if (REPORT_BREAKPOINT_EVENT (b))
> +    {
> +      if (create_breakpoint_hook)
> +       create_breakpoint_hook (b);
> +      breakpoint_create_event (b->number);
> +    }
> 
>    switch (b->type)
>      {
> @@ -6733,9 +6745,12 @@ delete_breakpoint (struct breakpoint *bp
>    if (bpt->type == bp_none)
>      return;
> 
> -  if (delete_breakpoint_hook)
> -    delete_breakpoint_hook (bpt);
> -  breakpoint_delete_event (bpt->number);
> +  if (REPORT_BREAKPOINT_EVENT (bpt))
> +    {
> +      if (delete_breakpoint_hook)
> +       delete_breakpoint_hook (bpt);
> +      breakpoint_delete_event (bpt->number);
> +    }
> 
>    if (bpt->inserted)
>      remove_breakpoint (bpt, mark_uninserted);
> @@ -7302,9 +7317,12 @@ disable_breakpoint (struct breakpoint *b
> 
>    check_duplicates (bpt);
> 
> -  if (modify_breakpoint_hook)
> -    modify_breakpoint_hook (bpt);
> -  breakpoint_modify_event (bpt->number);
> +  if (REPORT_BREAKPOINT_EVENT (bpt))
> +    {
> +      if (modify_breakpoint_hook)
> +       modify_breakpoint_hook (bpt);
> +      breakpoint_modify_event (bpt->number);
> +    }
>  }
> 
>  /* ARGSUSED */
> @@ -7430,10 +7448,14 @@ have been allocated for other watchpoint
>        if (save_selected_frame_level >= 0)
>         select_frame (save_selected_frame, save_selected_frame_level);
>        value_free_to_mark (mark);
> +    }
> +
> +  if (REPORT_BREAKPOINT_EVENT (bpt))
> +    {
> +      if (modify_breakpoint_hook)
> +       modify_breakpoint_hook (bpt);
> +      breakpoint_modify_event (bpt->number);
>      }
> -  if (modify_breakpoint_hook)
> -    modify_breakpoint_hook (bpt);
> -  breakpoint_modify_event (bpt->number);
>  }
> 
>  void

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
  2001-05-11  8:14         ` Keith Seitz
@ 2001-05-11  9:57           ` Eli Zaretskii
  0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2001-05-11  9:57 UTC (permalink / raw)
  To: keiths; +Cc: fnasser, gdb-patches

> Date: Fri, 11 May 2001 08:13:49 -0700 (PDT)
> From: Keith Seitz <keiths@cygnus.com>
> 
> breakpoint_delete_event, breakpoint_create_event, and
> breakpoint_modify_event are defined in gdb-events.[ch],sh. They are, as
> the comments from the files explain, for user interface events.
> 
> Well, it is my contention that user interfaces need not (indeed, should
> not) know anything about bp_none, bp_until, bp_finish, bp_longjmp,
> bp_longjmp_resume, bp_step_resume, bp_through_sigtramp,
> bp_watchpoint_scope, bp_call_dummy, bp_shlibevent, bp_thread_event,
> bp_catch_unload, bp_catch_fork, bp_catch_vfork, bp_catch_exec,
> bp_catch_catch, and bp_catch_throw breakpoint events (or some major subset
> of these). What libgdb does for its own housekeeping is its business, and
> its business alone. Breakpoints of these types are PRIVATE to gdb.

So you are saying that the UI will never need to support the
maintenance commands which expose those internal events to the user?
I'm not experienced enough with GDB UIs, so I cannot judge whether
this is true, but won't someone want the maintenance commands?

> (I suspect this isn't helping too much, is it?)

Well, at least I understand what do you mean now ;-)


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
       [not found] <Pine.GSO.4.33.0105102023220.29679-100000@ryobi.cygnus.com>
  2001-05-10 22:26 ` [RFA] breakpoint.c: don't generate bp events for internal bps Fernando Nasser
@ 2001-05-11 18:56 ` Andrew Cagney
  1 sibling, 0 replies; 12+ messages in thread
From: Andrew Cagney @ 2001-05-11 18:56 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

Keith,

I think the patch is filtering things at the wrong level.  If Insight 
doesn't want to know about certain breakpoints then it should discard them.

You probably also want to check ui-out.h and defs.h:gdb_breakpoint_query().

	Andrew


> +/* Is this breakpoint interesting to a user interface? */
> +#define REPORT_BREAKPOINT_EVENT(bp) \
> +((bp)->type == bp_breakpoint             \
> + || (bp)->type == bp_hardware_breakpoint \
> + || (bp)->type == bp_watchpoint          \
> + || (bp)->type == bp_hardware_watchpoint \
> + || (bp)->type == bp_read_watchpoint     \
> + || (bp)->type == bp_access_watchpoint)
> +
>  /* Set breakpoint count to NUM.  */


BTW, macro's are bad, M'kay!

	Andrew



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
  2001-05-11 10:22   ` Michael Snyder
@ 2001-05-11 10:25     ` Fernando Nasser
  0 siblings, 0 replies; 12+ messages in thread
From: Fernando Nasser @ 2001-05-11 10:25 UTC (permalink / raw)
  To: Michael Snyder; +Cc: Keith Seitz, gdb-patches

Michael Snyder wrote:
> 
> Fernando Nasser wrote:
> >
> > Keith Seitz wrote:
> > >
> > > On Fri, 11 May 2001, Fernando Nasser wrote:
> > >
> > > > Keith, you have attached the same old patch!
> > >
> > > Maybe that was intentional! ([hypnotizing voice] You _will_ see things my
> > > way!)
> > >
> > > Here's the real revised patch. No really, it is.
> > >
> >
> > This looks good (and safe) to me.  But we need Michael's or Jim's
> > approval.
> 
> Since this is really about the user interface (not about breakpoints),
> I defer to Keith's and your judgement.
> 

Thank you Michael.


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
  2001-05-11  9:15 ` Fernando Nasser
@ 2001-05-11 10:22   ` Michael Snyder
  2001-05-11 10:25     ` Fernando Nasser
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Snyder @ 2001-05-11 10:22 UTC (permalink / raw)
  To: Fernando Nasser; +Cc: Keith Seitz, gdb-patches

Fernando Nasser wrote:
> 
> Keith Seitz wrote:
> >
> > On Fri, 11 May 2001, Fernando Nasser wrote:
> >
> > > Keith, you have attached the same old patch!
> >
> > Maybe that was intentional! ([hypnotizing voice] You _will_ see things my
> > way!)
> >
> > Here's the real revised patch. No really, it is.
> >
> 
> This looks good (and safe) to me.  But we need Michael's or Jim's
> approval.

Since this is really about the user interface (not about breakpoints),
I defer to Keith's and your judgement.

Michael


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFA] breakpoint.c: don't generate bp events for internal bps
       [not found] <Pine.GSO.4.33.0105110855100.9875-100000@ryobi.cygnus.com>
@ 2001-05-11  9:15 ` Fernando Nasser
  2001-05-11 10:22   ` Michael Snyder
  0 siblings, 1 reply; 12+ messages in thread
From: Fernando Nasser @ 2001-05-11  9:15 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches, Michael Snyder

Keith Seitz wrote:
> 
> On Fri, 11 May 2001, Fernando Nasser wrote:
> 
> > Keith, you have attached the same old patch!
> 
> Maybe that was intentional! ([hypnotizing voice] You _will_ see things my
> way!)
> 
> Here's the real revised patch. No really, it is.
> 

This looks good (and safe) to me.  But we need Michael's or Jim's
approval.

Or would this be an obvious fix?  You are just making the code do what
the documentation (comments) say it should do.  It is very safe as well,
it should not impact anything else (now that the hooks were left alone).

Fernando



> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 breakpoint.c
> --- breakpoint.c        2001/05/06 22:22:02     1.35
> +++ breakpoint.c        2001/05/11 16:01:17
> @@ -318,6 +318,15 @@ int exception_support_initialized = 0;
>     error ("catch of library unloads not yet implemented on this platform")
>  #endif
> 
> +/* Is this breakpoint interesting to a user interface? */
> +#define REPORT_BREAKPOINT_EVENT(bp) \
> +((bp)->type == bp_breakpoint             \
> + || (bp)->type == bp_hardware_breakpoint \
> + || (bp)->type == bp_watchpoint          \
> + || (bp)->type == bp_hardware_watchpoint \
> + || (bp)->type == bp_read_watchpoint     \
> + || (bp)->type == bp_access_watchpoint)
> +
>  /* Set breakpoint count to NUM.  */
> 
>  void
> @@ -4375,7 +4384,8 @@ mention (struct breakpoint *b)
>       delete_breakpoint_hook and so on.  */
>    if (create_breakpoint_hook)
>      create_breakpoint_hook (b);
> -  breakpoint_create_event (b->number);
> +  if (REPORT_BREAKPOINT_EVENT (b))
> +    breakpoint_create_event (b->number);
> 
>    switch (b->type)
>      {
> @@ -6735,7 +6745,8 @@ delete_breakpoint (struct breakpoint *bp
> 
>    if (delete_breakpoint_hook)
>      delete_breakpoint_hook (bpt);
> -  breakpoint_delete_event (bpt->number);
> +  if (REPORT_BREAKPOINT_EVENT (bpt))
> +    breakpoint_delete_event (bpt->number);
> 
>    if (bpt->inserted)
>      remove_breakpoint (bpt, mark_uninserted);
> @@ -7304,7 +7315,8 @@ disable_breakpoint (struct breakpoint *b
> 
>    if (modify_breakpoint_hook)
>      modify_breakpoint_hook (bpt);
> -  breakpoint_modify_event (bpt->number);
> +  if (REPORT_BREAKPOINT_EVENT (bpt))
> +    breakpoint_modify_event (bpt->number);
>  }
> 
>  /* ARGSUSED */
> @@ -7433,7 +7445,8 @@ have been allocated for other watchpoint
>      }
>    if (modify_breakpoint_hook)
>      modify_breakpoint_hook (bpt);
> -  breakpoint_modify_event (bpt->number);
> +  if (REPORT_BREAKPOINT_EVENT (bpt))
> +    breakpoint_modify_event (bpt->number);
>  }
> 
>  void
> 
> > I am not against removing the hook.  I would not be so certain that
> > NOTHING is using it as there are too many obscure academic projects
> > going on using Free Software.  That is how Free Software started (and
> > how I started hacking GDB) and I would not like to discard them.  But,
> > whoever is doing anything with GDB should be monitoring this list.  So,
> > if you post to gdb@sources saying that you will get rid of these hooks
> > and nobody answers in a reasonable time, "off with their heads"!
> 
> Ugh. I suddenly remembered why I didn't post a patch eliminating the
> hook.. annotate.c.
> 
> Maybe I should work on converting annotate.c, too? ;-)
> 
> Keith

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2001-05-11 18:56 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <Pine.GSO.4.33.0105102023220.29679-100000@ryobi.cygnus.com>
2001-05-10 22:26 ` [RFA] breakpoint.c: don't generate bp events for internal bps Fernando Nasser
2001-05-10 23:56   ` Eli Zaretskii
2001-05-11  7:09     ` Keith Seitz
2001-05-11  8:04       ` Eli Zaretskii
2001-05-11  8:14         ` Keith Seitz
2001-05-11  9:57           ` Eli Zaretskii
2001-05-11  7:05   ` Keith Seitz
2001-05-11  8:54     ` Fernando Nasser
2001-05-11 18:56 ` Andrew Cagney
     [not found] <Pine.GSO.4.33.0105110855100.9875-100000@ryobi.cygnus.com>
2001-05-11  9:15 ` Fernando Nasser
2001-05-11 10:22   ` Michael Snyder
2001-05-11 10:25     ` Fernando Nasser

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox