Re: Breakpoint MI notifications

[Vladimir Prus <vladimir at codesourcery dot com>]

On Tuesday, April 26, 2011 20:03:43 Tom Tromey wrote:
> >>>>> "Volodya" == Vladimir Prus <vladimir@codesourcery.com> writes:
> Volodya> The enclosed patch implements MI notifications about breakpoint
> Volodya> changes.  As other MI notifications, those are emitted only
> Volodya> when breakpoint changes by something that is not a MI command
> Volodya> to change a breakpoint. For example, -break-condition will not
> Volodya> cause a breakpoint-changed notification to be emitted.
> 
> Volodya> This functionality was independently written by Tom and myself,
> Volodya> and this patch merges the patches together. As a side effect,
> Volodya> existing breakpoint observers were modified to take a pointer
> Volodya> to breakpoint rather than it, which eliminates quite some
> Volodya> busywork.
> 
> Thanks for doing this.
> 
> A few nits on the code part, nothing serious though.

Thanks for the review. Is this version better?

- Volodya

-- 
Vladimir Prus
Mentor Graphics
+7 (812) 677-68-40

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index bc08d7b..36b9056 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5083,7 +5083,6 @@ print_one_breakpoint (struct breakpoint *b,
 	  && !is_hardware_watchpoint (b)
 	  && (b->loc->next || !b->loc->enabled))
 	{
-	  struct cleanup *inner = make_cleanup (null_cleanup, 0);
 	  struct bp_location *loc;
 	  int n = 1;
 
@@ -5094,8 +5093,6 @@ print_one_breakpoint (struct breakpoint *b,
 	      print_one_breakpoint_location (b, loc, n, last_loc, allflag);
 	      do_cleanups (inner2);
 	    }
-
-	  do_cleanups (inner);
 	}
     }
 }
@@ -10820,6 +10817,8 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
   return sal;
 }
 
+/* Returns 1 iff locations A and B are sufficiently same that
+   we don't need to report breakpoint as changed.  */
 static int
 locations_are_equal (struct bp_location *a, struct bp_location *b)
 {
@@ -11471,7 +11470,7 @@ do_map_disable_breakpoint (struct breakpoint *b, void *ignore)
   disable_breakpoint (b);
 }
 
-void
+static void
 disable_command (char *args, int from_tty)
 {
   struct breakpoint *bpt;
@@ -11573,7 +11572,7 @@ do_map_enable_breakpoint (struct breakpoint *b, void *ignore)
    breakpoints) so they once again become (or continue to be) effective
    in stopping the inferior.  */
 
-void
+static void
 enable_command (char *args, int from_tty)
 {
   struct breakpoint *bpt;
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index f103f3a..d8c3924 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -164,13 +164,13 @@ A new breakpoint @var{b} has been created.
 @end deftypefun
 
 @deftypefun void breakpoint_deleted (struct breakpoint *@var{b})
-A breakpoint has been destroyed.  The argument @var{bpnum} is the
-number of the newly-destroyed breakpoint.
+A breakpoint has been destroyed.  The argument @var{b} is the
+pointer to the destroyed breakpoint.
 @end deftypefun
 
 @deftypefun void breakpoint_modified (struct breakpoint *@var{b})
-A breakpoint has been modified in some way.  The argument @var{bpnum}
-is the number of the modified breakpoint.
+A breakpoint has been modified in some way.  The argument @var{b}
+is the modified breakpoint.
 @end deftypefun
 
 @deftypefun void tracepoint_created (int @var{tpnum})
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index e9f5eca..c075b0c 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -452,8 +452,11 @@ mi_about_to_proceed (void)
   mi_proceeded = 1;
 }
 
+/* When non-zero, no MI notifications will be emitted in
+   response to breakpoint change observers.  */
 int mi_suppress_breakpoint_notifications = 0;
 
+/* Emit notification about a created breakpoint.  */
 static void
 mi_breakpoint_created (struct breakpoint *b)
 {
@@ -485,6 +488,7 @@ mi_breakpoint_created (struct breakpoint *b)
   gdb_flush (mi->event_channel);
 }
 
+/* Emit notification about deleted breakpoint.  */
 static void
 mi_breakpoint_deleted (struct breakpoint *b)
 {
@@ -504,6 +508,7 @@ mi_breakpoint_deleted (struct breakpoint *b)
   gdb_flush (mi->event_channel);
 }
 
+/* Emit notification about modified breakpoint.  */
 static void
 mi_breakpoint_modified (struct breakpoint *b)
 {
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index a78b6fd..8c82f41 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2088,7 +2088,7 @@ mi_cmd_execute (struct mi_parse *parse)
 
   current_context = parse;
 
-  if (strstr (parse->command, "break-") == parse->command)
+  if (strncmp (parse->command, "break-", sizeof ("break-")) == parse->command)
     {
       make_cleanup_restore_integer (&mi_suppress_breakpoint_notifications);
       mi_suppress_breakpoint_notifications = 1;
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index ec43eb8..39578f1 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -787,6 +787,9 @@ gdbpy_breakpoint_created (struct breakpoint *bp)
   breakpoint_object *newbp;
   PyGILState_STATE state;
 
+  if (num < 0 && bppy_pending_object == NULL)
+    return;
+
   if (bp->type != bp_breakpoint 
       && bp->type != bp_watchpoint
       && bp->type != bp_hardware_watchpoint