Re: [RFC - Python] New ObjFile event

[Kevin Pouget <kevin dot pouget at gmail dot com>]

Hello,


> Kevin> + ? ? ? ?py-value.o \
> Kevin> + ? ? ? ?py-newobjfileevent.o
>
> Alphabetical, please. ?Applies elsewhere in the Makefile patch too.

fixed, sorry about that

> Kevin> ?# found by configure; if GNU Make is not found, we fall back to a
> Kevin> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> Kevin> index 08d6c8c..3ea5068 100644
> Kevin> --- a/gdb/doc/ChangeLog
> Kevin> +++ b/gdb/doc/ChangeLog
> Kevin> @@ -1,3 +1,9 @@
> Kevin> +2011-03-29 ?Kevin Pouget <kevin.pouget@st.com>
> Kevin> + ? ? ? ?* gdb.textinfo (Events In Python): Document `gdb.NewObjFileEvent'
>
> Missing blank line after the date line, and typo: "texinfo", not
> "textinfo".

I removed the Changelog entries from the patch as suggested by Pedro
in another thread,
typo fixed at the bottom of the mail

> Kevin> +@item events.newobjfile
> Kevin> +Emits @code{gdb.NewObjFileEvent} which indicates that a new object-file has
> Kevin> +been loaded in the inferior.
>
> This is not an accurate description -- it may or may not be related to a
> change in the inferior.
>
> E.g., if the user did "set auto-solib-add off", then no objfile will be
> made when a shared library is loaded.
>
> Or, if the user does "add-symbol-file", then a new objfile will be made
> even though nothing has changed in the inferior.
>
> Maybe this isn't the event you actually wanted? ?I mean -- it is fine to
> have this, and I'd like it if you followed through :-), but it may not
> do what you are expecting.

you're right, I changed it to:
 +been loaded by @value{GDBN}.

About the behaviour, I actually wanted to mimic the way GDB decides to
enable [linux/thread-db] thread-debugging, ie checking the name of the
shared library loaded by GDB. As you mentioned that's not perfect,
"set auto-solib-add off" or "add-symbol-file" might change the
behaviour, but I guess it's easy enough to cope with it

> Kevin> + ?/* Will be NULL when clearing the symtab. */
> Kevin> + ?if (objfile)
> Kevin> + ? ?cleanup = ensure_python_env (get_objfile_arch (objfile), current_language);
> Kevin> + ?else
> Kevin> + ? ?cleanup = ensure_python_env (get_current_arch (), current_language);
>
> I think it is better to just return early if objfile==NULL.
> Or, emit a different event in this case.

I found some more documentation about objfile == NULL in the Internals:
> Called with objfile equal to NULL to indicate previously loaded symbol table data has now been invalidated.

so as you suggested, I just return immediately and leave symbol table
invalidation for another patch

> Kevin> +2011-03-29 ?Kevin Pouget <kevin.pouget@st.com>
> Kevin> + ? ? ? ?* testsuite/gdb.python/py-events-shlib.c: New file.
>
> Missing newline.

cf above, ChangeLog removed from the diff patch and fixed at the bottom

> Kevin> +# Start with a fresh gdb.
> Kevin> +
> Kevin> +gdb_exit
> Kevin> +gdb_start
>
> I think clean_restart is preferred here.
> But maybe skip_python_tests has to be inserted in the middle of the
> sequence?

fixed

> Kevin> +gdb_test "run" ".*event type: new_objfile.*
> Kevin> +.*new objfile name.*"
>
> This style of test is hard to read. ?I'd prefer a \n in there.

Fixed (I didn't want to test the '\n' specifically but just make it
more readable:).
However, JFYI the test file already contains such formatting:

# Test continue event and breakpoint stop event
gdb_test "continue" ".*event type: continue.*
.*event type: stop.*
.*stop reason: breakpoint.*
.*breakpoint number: 2.*
all threads stopped"

#test exited event.
gdb_test "continue" ".*event type: continue.*
.*event type: exit.*
.*exit code: 12.*"



thanks for your review,

Kevin

--

2011-03-29  Kevin Pouget  <kevin.pouget@st.com>

	Allow Python notification of new object-file loadings.
	* Makefile.in (SUBDIR_PYTHON_SRCS): Add py-newobjfilevent.c.
	(SUBDIR_PYTHON_OBS): Add py-newobjfileevent.o.
	Add build rule for this file.
	* python/py-event.h (emit_new_objfile_event): New prototype.
	(newobjfile): New Python event emitter.
	* python/py-evts.c (gdbpy_initialize_py_events): Add newobjfile to
	Python event registry.
	* python/py-inferior.c: Include objfiles.h
	(python_new_objfile): New function.
	(gdbpy_initialize_inferior): Add python_new_objfile to the new objfile
	observers.
	* python/py-newobjfileevent.c: New file.
	* python-internal.h (gdbpy_initialize_new_objfile_event): New
	prototype.
	(gdbpy_current_objfile): New global variable.
	* python/python.c (gdbpy_current_objfile): Make global.
	(_initialize_python): Add gdbpy_initialize_new_objfile_event call.

2011-03-29  Kevin Pouget  <kevin.pouget@st.com>

	Allow Python notification of new object-file loadings.
	* gdb.texinfo (Events In Python): Document `gdb.NewObjFileEvent'
	events emitter.
	(Objfiles In Python): Indicate that `gdb.current_objfile' is also set
	during new object-file callbacks.

Attachment: new-obj-file.txt
Description: Text document