From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21089 invoked by alias); 8 Oct 2010 14:04:48 -0000 Received: (qmail 21078 invoked by uid 22791); 8 Oct 2010 14:04:47 -0000 X-SWARE-Spam-Status: No, hits=-5.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,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; Fri, 08 Oct 2010 14:04:42 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o98E4YR0004705 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 8 Oct 2010 10:04:34 -0400 Received: from localhost.localdomain.redhat.com (ovpn-113-95.phx2.redhat.com [10.3.113.95]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o98E4V5p009075; Fri, 8 Oct 2010 10:04:32 -0400 From: Phil Muldoon To: Pedro Alves Cc: gdb-patches@sourceware.org, dan@codesourcery.com Subject: Re: [patch] Add visible flag to breakpoints. References: <201009301741.32379.pedro@codesourcery.com> <201010081435.15174.pedro@codesourcery.com> Reply-to: pmuldoon@redhat.com X-URL: http://www.redhat.com Date: Fri, 08 Oct 2010 14:04:00 -0000 In-Reply-To: <201010081435.15174.pedro@codesourcery.com> (Pedro Alves's message of "Fri, 8 Oct 2010 14:35:14 +0100") 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 X-IsSubscribed: yes 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: 2010-10/txt/msg00140.txt.bz2 Pedro Alves writes: > On Friday 08 October 2010 13:50:34, Phil Muldoon wrote: >> The @var{internal} argument has no effect with watchpoints. > > Should it be an error instead (or made to work)? I can make it an error. I decided not to do watchpoints. The consequences of setting hidden watchpoints from a script that could turn out to be software watchpoints seemed a little troubling. > The non-python bits looked fine. I'd prefer Tom or someone > else to look and approve those. I did notice: > >> #define BPPY_REQUIRE_VALID(Breakpoint) \ >> do { \ >> - if (! bpnum_is_valid ((Breakpoint)->number)) \ >> + if (Breakpoint == NULL) \ >> return PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \ >> (Breakpoint)->number); \ > '(Breakpoint)->number' when Breakpoint is NULL is not going to work. Oops, thanks for catching that. (Breakpoint)->number is left over from the old macro. Will fix. > Also, missing ()s: > > if ((Breakpoint) == NULL) Thanks. > I would also suggest using a VEC for the breakpoints instead > of a linked list. > > (Also, the old code appeared to have been designed for O(1) access to > the python breakpoint objects given a breakpoing number: I have no > clue it if that matters but you could preserve that easily with two > VECs or bppy_breakpoints arrays.) Yeah the old code used vectors, and I converted it to a linked list to cope with negative breakpoint numbers. I did consider keeping two vectors. But I wasn't sure if the 0(1) design choice was conscious for performance or whether it was just done that way. FWIW we only track user watchpoints and breakpoints (and now the special breakpoint we have introduced). Even in a potentially heavy situation I would expect the number of user breakpoints to be < 1000? Is it really necessary? It's a trade-off of for keeping two reallocated vectors over the lifetime to maintaining a linked list. I'm ambivalent to how we decide to do it. I just wanted to explain my rationale. Cheers, Phil