Re: [PATCH] Add bp_location to Python interface

[Tom Tromey <tromey@redhat.com>]

>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:

Kevin> ping

I'm sorry about the long delay on this.

Kevin>  free_bp_location (struct bp_location *loc)
[...]
Kevin> +  gdbpy_bplocation_free (loc);

I wonder whether tying these wrapper objects to the bp_location is
really best.

We do take this approach for many other objects exposed to Python, but
those objects tend to be rather long-lived, and also to some extent
visible across gdb -- as opposed to breakpoint locations, which are
entirely managed by the breakpoint module.

A different approach would be to instantiate and fill in the location
objects when the 'location' method is called, and give them "copy"
semantics instead.

I'm not totally convinced either way, and I would like to know what you
think about this.

Kevin> +@findex gdb.locations
Kevin> +@defun gdb.locations ()

The "gdb." here seems incorrect.
This is a method on Breakpoint.

Kevin> +Return a tuple containing a sequence of @code{gdb.BpLocation} objects 

"tuple containing a sequence" sounds weird.  I would say just "a
sequence" and not specify that it must be a tuple; it is commonplace in
Python to operate on sequences generically, and we might as well leave
ourselves whatever leeway we reasonably can.

Kevin> +@defvar BpLocation.inferior

Locations are really tied to program spaces, not inferiors.
Offhand it seems to me that we should respect this in the API.

Kevin> +bplocpy_breakpoint_deleted (struct breakpoint *b) {

Wrong brace placement.

Kevin> +  tuple = PyList_AsTuple (list);
Kevin> +  Py_DECREF (list);

In keeping with the doc change, you could just return the list here,
rather than convert to a tuple.

Kevin> +gdb_test "py print len(gdb.breakpoints())" "1" "ensure that threre is only one BP"

Typo, "threre".
Also the line should be split, here and elsewhere in the .exp.

Kevin> +# how to check address location ? != address(main)

It is tricky due to prologues, but you could check the output of "info
symbol" on the address and see that it is "main".

Tom