Re: [python][patch] Add temporary breakpoint features to Python breakpoints.

[Phil Muldoon <pmuldoon at redhat dot com>]

On 09/09/13 19:47, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> 
> Phil> This patch allows Python breakpoints to be made temporary on
> Phil> creation.  It adds an optional argument, and keyword: "temporary".
> 
> Phil> OK?
> 
> Thanks, Phil.
> 
> Phil> 2013-09-09  Phil Muldoon  <pmuldoon@redhat.com>
> 
> Phil> 	* python/py-breakpoint.c (bppy_get_temporary): New function.
> Phil> 	(bppy_init): New keyword: temporary. Parse it and set breakpoint
> Phil> 	to temporary if True.
> 
> Is there a PR associated with this?
> I thought there was, but I didn't check.

I will double-check.

 
> Phil> +@defvar Breakpoint.temporary
> Phil> +This attribute indicates whether the breakpoint was created as a
> Phil> +temporary breakpoint.  Temporary breakpoints are automatically deleted
> Phil> +after that breakpoint has been hit.  Access to this attribute, and all
> Phil> +other attributes and functions other than the @code{is_valid}
> Phil> +function, will result in an error after the breakpoint has been hit
> Phil> +(as it has been automatically deleted).  This attribute is not
> Phil> +writable.

I ended up rewriting this paragraph several times to try to explain
hit ;) In the end I deleted as we already use "hit" as a verb in the
API (and in the documentation).  (IE, hit_count).  I could not come up
with a good turn of phrase without turning the paragraph of the
constructor for breakpoints into a mess of technical discussion with
how GDB handles stop events for breakpoints.  What do you think we
should use here?

Breakpoints with a disposition of del, or del_at_next_stop are deleted
when the when the inferior stops.  But good point, there could
possibly be a case where the "stop" and disposition may conflict. (When
the stop callbacks are executed, the inferior is stopped by the
kernel, but the state has not yet changed in the GDB record keeping).
I will double check.

 
> I think it is worth spelling out what "hit" means.
> In particular I was wondering how it interacts with the "stop" method.
> 
> Phil> +  if (self_bp->bp->disposition == disp_del ||
> Phil> +      self_bp->bp->disposition == disp_del_at_next_stop)
> 
> "||" at the wrong spot.
> 
> Phil> +  static char *keywords[] = { "spec", "type", "wp_class", "internal", "temporary", NULL };
> 
> This line is too long.
> 
> Phil> +  if (temporary)
> 
> temporary != NULL

OK thanks for catching these.

Cheers,

Phil