[PATCH] Add bp_location to Python interface

[Kevin Pouget]

On Fri, Mar 30, 2012 at 9:51 PM, Tom Tromey <tromey@redhat.com> wrote:
>
> >>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:
>
> Kevin> ping
>
> I'm sorry about the long delay on this.

thanks for the follow-up, I've been busy as well and had no time to ping it.

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

I think it's a good idea, it avoids the confusion of "short life"
objects which can't be saved, etc.
Now, I state in the documentation that

> Breakpoint location
> objects represent the state of the breakpoint location at the time the method
> @code{Breakpoint.locations} is called, and will not be further updated.

which sounds better than the previous version

> Kevin> +@findex gdb.locations
> Kevin> +@defun gdb.locations ()
>
> The "gdb." here seems incorrect.
> This is a method on Breakpoint.

changed

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

changed, the original sentence doesn't really make sense to me, "a
tuple containing" was just redundant

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

I'm not convinced here; actually, I'm not sure to understand the point
of the Progspaces class, in its current state at least; and there is
no (as far as I've seen) equivalent of
"find_inferior_for_program_space" in Py, so one of the key interest of
BpLocation would be unavailable ...

> Kevin> +bplocpy_breakpoint_deleted (struct breakpoint *b) {
>
> Wrong brace placement.

fixed

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

the idea here was that a tuple is not mutable, whereas a list is -- ()
vs. []. I've removed the tuple, although although I'm not sure when
I'm supposed to use the List or the 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.

fixed

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

I've added a test going this way, but I'm not very happy with the implementation

> gdb_test "py gdb.execute('info symbol %s' % loc1.address)"  "main .* in section \.text.*" "verify location address"
because the exact result is "main + 15" ... but I don't know if there
is any better way to do it.



there's no regression against the current git tree on x86_64/F16

2012-03-04  Kevin Pouget  <kevin.pouget@st.com>

	Add bp_location to Python interface
	* Makefile.in (SUBDIR_PYTHON_OBS): Add py-bploc.o
	(SUBDIR_PYTHON_SRCS): Add python/py-bploc.c
	Add build rule for this file.
	* breakpoint.h (struct bploc_object): Forward declaration.
	(struct bp_location): Add py_bploc_obj.
	* python/py-bploc.c: New file.
	* python/py-breakpoint.c (bppy_locations): New function.
	(breakpoint_object_methods): New method binding: locations().
	* python/python-internal.h (bploc_object): New typedef.
	(bplocation_to_bplocation_object): New prototype.
	(gdbpy_initialize_bplocation): Likewise.
	
doc/
	Add bp_location to Python interface
	* gdb.texinfo (Breakpoints In Python): Document
	gdb.Breakpoint.locations and gdb.BpLocation.


testsuite/
	Add bp_location to Python interface
	* gdb.python/py-breakpoint.exp: Test gdb.BpLocation.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-gdb.BpLocation-to-Python-interface.patch
Type: application/octet-stream
Size: 18784 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20120403/9a3d0333/attachment.obj>