From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3118 invoked by alias); 30 Jul 2013 21:04:37 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 3108 invoked by uid 89); 30 Jul 2013 21:04:36 -0000 X-Spam-SWARE-Status: No, score=-4.6 required=5.0 tests=AWL,BAYES_05,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RDNS_NONE,SPF_HELO_PASS,SPF_PASS autolearn=no version=3.3.1 Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 30 Jul 2013 21:04:36 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r6UL4SSK014181 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 30 Jul 2013 17:04:28 -0400 Received: from barimba (ovpn-113-128.phx2.redhat.com [10.3.113.128]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r6UL4Q0F021442 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 30 Jul 2013 17:04:27 -0400 From: Tom Tromey To: Nick Bull Cc: gdb-patches@sourceware.org Subject: Re: Events when inferior is modified References: <51CDBD48.6010903@gmail.com> Date: Tue, 30 Jul 2013 21:04:00 -0000 In-Reply-To: <51CDBD48.6010903@gmail.com> (Nick Bull's message of "Fri, 28 Jun 2013 17:43:52 +0100") Message-ID: <87k3k7kbdy.fsf@fleche.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-07/txt/msg00801.txt.bz2 >>>>> "Nick" == Nick Bull 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