From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21922 invoked by alias); 26 Apr 2011 16:04:06 -0000 Received: (qmail 21899 invoked by uid 22791); 26 Apr 2011 16:04:05 -0000 X-SWARE-Spam-Status: No, hits=-6.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_FILL_THIS_FORM_SHORT,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 26 Apr 2011 16:03:45 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p3QG3iE7025314 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 26 Apr 2011 12:03:45 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p3QG3iVi013877; Tue, 26 Apr 2011 12:03:44 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p3QG3hu0010900; Tue, 26 Apr 2011 12:03:44 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 626F13781C7; Tue, 26 Apr 2011 10:03:43 -0600 (MDT) From: Tom Tromey To: Vladimir Prus Cc: gdb-patches@sources.redhat.com Subject: Re: Breakpoint MI notifications References: <201104261508.20380.vladimir@codesourcery.com> Date: Tue, 26 Apr 2011 16:04:00 -0000 In-Reply-To: <201104261508.20380.vladimir@codesourcery.com> (Vladimir Prus's message of "Tue, 26 Apr 2011 15:08:20 +0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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 X-SW-Source: 2011-04/txt/msg00479.txt.bz2 >>>>> "Volodya" == Vladimir Prus writes: Volodya> The enclosed patch implements MI notifications about breakpoint Volodya> changes. As other MI notifications, those are emitted only Volodya> when breakpoint changes by something that is not a MI command Volodya> to change a breakpoint. For example, -break-condition will not Volodya> cause a breakpoint-changed notification to be emitted. Volodya> This functionality was independently written by Tom and myself, Volodya> and this patch merges the patches together. As a side effect, Volodya> existing breakpoint observers were modified to take a pointer Volodya> to breakpoint rather than it, which eliminates quite some Volodya> busywork. Thanks for doing this. A few nits on the code part, nothing serious though. Volodya> @@ -5061,7 +5060,12 @@ print_one_breakpoint (struct breakpoint *b, Volodya> struct bp_location **last_loc, Volodya> int allflag) Volodya> { [...] Volodya> + struct cleanup *inner = make_cleanup (null_cleanup, 0); I don't think you need this cleanup. Volodya> +static int Volodya> +locations_are_equal (struct bp_location *a, struct bp_location *b) A new function should have an introductory comment. Volodya> -static void Volodya> +void Volodya> disable_command (char *args, int from_tty) I didn't see any use of this function elsewhere, so I think the 'static' can stay. Volodya> -static void Volodya> +void Volodya> enable_command (char *args, int from_tty) Likewise. Volodya> +@deftypefun void breakpoint_deleted (struct breakpoint *@var{b}) Volodya> A breakpoint has been destroyed. The argument @var{bpnum} is the Volodya> number of the newly-destroyed breakpoint. Volodya> @end deftypefun The body of the documentation should be updated too. Volodya> -@deftypefun void breakpoint_modified (int @var{bpnum}) Volodya> +@deftypefun void breakpoint_modified (struct breakpoint *@var{b}) Volodya> A breakpoint has been modified in some way. The argument @var{bpnum} Volodya> is the number of the modified breakpoint. Likewise. Volodya> +int mi_suppress_breakpoint_notifications = 0; Volodya> + Volodya> +static void Volodya> +mi_breakpoint_created (struct breakpoint *b) Volodya> +{ Comments on new functions and globals. Volodya> + if (strstr (parse->command, "break-") == parse->command) I think strncmp would be more idiomatic here. This implementation seems fragile, but that is your call. Volodya> /* Callback that is used when a breakpoint is created. This function Volodya> will create a new Python breakpoint object. */ Volodya> static void Volodya> -gdbpy_breakpoint_created (int num) Volodya> +gdbpy_breakpoint_created (struct breakpoint *bp) Volodya> { Volodya> breakpoint_object *newbp; Volodya> - struct breakpoint *bp = NULL; Volodya> PyGILState_STATE state; Volodya> - bp = get_breakpoint (num); Volodya> - if (! bp) Volodya> - return; Volodya> - Volodya> - if (num < 0 && bppy_pending_object == NULL) Volodya> - return; I am not sure that removing this test is ok. Tom