Re: Breakpoint MI notifications

[Tom Tromey <tromey at redhat dot com>]

>>>>> "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.

Volodya> @@ -5061,7 +5060,12 @@ print_one_breakpoint (struct breakpoint *b,
Volodya>  		      struct bp_location **last_loc, 
Volodya>  		      int allflag)
Volodya>  {
[...]
Volodya> +	  struct cleanup *inner = make_cleanup (null_cleanup, 0);

I don't think you need this cleanup.

Volodya> +static int
Volodya> +locations_are_equal (struct bp_location *a, struct bp_location *b)

A new function should have an introductory comment.

Volodya> -static void
Volodya> +void
Volodya>  disable_command (char *args, int from_tty)

I didn't see any use of this function elsewhere, so I think the 'static'
can stay.

Volodya> -static void
Volodya> +void
Volodya>  enable_command (char *args, int from_tty)

Likewise.

Volodya> +@deftypefun void breakpoint_deleted (struct breakpoint *@var{b})
Volodya>  A breakpoint has been destroyed.  The argument @var{bpnum} is the
Volodya>  number of the newly-destroyed breakpoint.
Volodya>  @end deftypefun

The body of the documentation should be updated too.

Volodya> -@deftypefun void breakpoint_modified (int @var{bpnum})
Volodya> +@deftypefun void breakpoint_modified (struct breakpoint *@var{b})
Volodya>  A breakpoint has been modified in some way.  The argument @var{bpnum}
Volodya>  is the number of the modified breakpoint.

Likewise.

Volodya> +int mi_suppress_breakpoint_notifications = 0;
Volodya> +
Volodya> +static void
Volodya> +mi_breakpoint_created (struct breakpoint *b)
Volodya> +{

Comments on new functions and globals.

Volodya> +  if (strstr (parse->command, "break-") == parse->command)

I think strncmp would be more idiomatic here.

This implementation seems fragile, but that is your call.

Volodya>  /* Callback that is used when a breakpoint is created.  This function
Volodya>     will create a new Python breakpoint object.  */
Volodya>  static void
Volodya> -gdbpy_breakpoint_created (int num)
Volodya> +gdbpy_breakpoint_created (struct breakpoint *bp)
Volodya>  {
Volodya>    breakpoint_object *newbp;
Volodya> -  struct breakpoint *bp = NULL;
Volodya>    PyGILState_STATE state;
 
Volodya> -  bp = get_breakpoint (num);
Volodya> -  if (! bp)
Volodya> -    return;
Volodya> -
Volodya> -  if (num < 0 && bppy_pending_object == NULL)
Volodya> -    return;

I am not sure that removing this test is ok.

Tom