This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Events when inferior is modified


>>>>> "Nick" == Nick Bull <nicholaspbull@gmail.com> writes:

Nick> This patch adds new observers, and corresponding Python events, for
Nick> various actions on an inferior: calling a function (by hand),
Nick> modifying registers or modifying memory.

Thanks.

Nick> This is my first patch submission, so (a) apologies for things I've
Nick> got wrong and (b) I believe I may need to do some paperwork for
Nick> copyright assignment.

I don't remember if I got you started on the paperwork or not.
I will send you email off-list.

Don't worry about (a).  This review is mostly little nits.

Nick>  +@deftypefun void inferior_call_pre (void)
Nick> +An inferior function is about to be called.
Nick> +@end deftypefun
Nick> +
Nick> +@deftypefun void inferior_call_post (void)
Nick> +An inferior function has just been called.
Nick> +@end deftypefun

I'm curious to know why you wanted pre- and post-observers for inferior
calls, but not anything else.

Also, the patch got a little mangled (see the space before the "+" on
the first quoted line here).  This happened in a few spots.

Nick>  extern int emit_exited_event (const LONGEST *exit_code, struct inferior *inf);
Nick>  +typedef enum {

Blank line between the previous declaration and this one.
The brace should go on its own line.

Nick> +
Nick> +static PyObject *
Nick> +create_inferior_call_event_object (int flag)
Nick> +{

Needs an intro comment.  It can be short.

I think it would be better if the "int" were instead
"inferior_altered_kind".

Nick> +  PyObject * event;

No space after "*".

Nick> +    default:
Nick> +      return NULL;

I think gdb_assert_not_reached is more correct here.
However if you want to return NULL then you have to set the Python
exception.

Nick> +int
Nick> +emit_inferior_altered_event (inferior_altered_kind kind)
Nick> +{

I think it would be fine to make this static and put all the observers
into this file.

Nick> diff --git a/gdb/target.c b/gdb/target.c
Nick> index 920f916..1e85bdd 100644
Nick> --- a/gdb/target.c
Nick> +++ b/gdb/target.c
Nick> @@ -43,6 +43,7 @@
Nick>  #include "tracepoint.h"
Nick>  #include "gdb/fileio.h"
Nick>  #include "agent.h"
Nick> +#include "observer.h"
Nick>  static void target_info (char *, int);
Nick>  @@ -1611,6 +1612,8 @@ memory_xfer_partial_1 (struct target_ops *ops,
Nick> enum target_object object,
Nick>   do
Nick>      {
Nick> +      if (writebuf)
Nick> +	observer_notify_memory_change ();

I think "if (writebuf != NULL)", here and elsewhere.


It seem to me that this may just be an approximation of "dirty" in the
remote case.  E.g., gdbserver may write a breakpoint and otherwise
modify things without notifying gdb.

On the other hand, maybe you want to rule out breakpoints from being
considered "dirty".  But then the change has to be more complicated in
another way.

Tom


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]