From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25952 invoked by alias); 14 Dec 2010 17:28:55 -0000 Received: (qmail 25943 invoked by uid 22791); 14 Dec 2010 17:28:54 -0000 X-SWARE-Spam-Status: No, hits=-6.9 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; Tue, 14 Dec 2010 17:28:44 +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 oBEHSeWd006806 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 14 Dec 2010 12:28:40 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id oBEHSeH1028895; Tue, 14 Dec 2010 12:28:40 -0500 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 oBEHSdUa031868; Tue, 14 Dec 2010 12:28:39 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id 148F437848F; Tue, 14 Dec 2010 10:28:38 -0700 (MST) From: Tom Tromey To: Doug Evans Cc: pmuldoon@redhat.com, gdb-patches@sourceware.org Subject: Re: [patch] Add an evaluation function hook to Python breakpoints. References: Date: Tue, 14 Dec 2010 17:28:00 -0000 In-Reply-To: (Doug Evans's message of "Mon, 13 Dec 2010 12:45:38 -0800") 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: 2010-12/txt/msg00246.txt.bz2 >>>>> "Doug" == Doug Evans writes: Doug> Collecting data in the "evaluate" function feels too hacky for Doug> something we explicitly tell users is the published way to do this Doug> kind of thing. Can you explain what is hacky about it? I'm not trying to be difficult, I really do not understand. I will explain why I think it is ok. This patch addresses two bits of functionality we have needed in Python. First, we need a clean way to run some code at a given breakpoint location -- but with the caveats that the code always be run, regardless of other breakpoints, and that the code not interfere with stepping. By "clean" I just mean that we want it not to have user-visible effects other than the effects we intend. Yes, we can do it using a python convenience function -- but the convenience function's name is visible and in a flat namespace. The use case for this is something like gdb-heap, where we want to install a breakpoint that collects some information but doesn't interfere with ordinary debugging. Second, we want a way to stop the inferior when such the data collection step decides. Our use case here is a (to-be-written) mutex-tracking extension, that collects information about pthread locking and unlocking, and stops when a deadlock is detected. I think Phil's patch accomplishes all of these goals. Doug> And the name "evaluate" doesn't feel right either. I agree. Doug> I realize the _p convention mightn't be sufficiently common to use in Doug> the Python API, but for example a better name might be stop_p. Doug> And then one would have another method (I don't have a good name for Doug> it ATM) to use to collect data. Doug> Setting aside name choices, I like that API better. Having two methods seems worse to me. It is more complicated without any added benefits. First, both such methods will be called in the same place in gdb. This is necessary to implement the needed features. But then .. why call two methods when you can just call one? Second, consider the mutex-watching use case. To implement this in the two-method case, you must have the data collector set a flag, which is then returned by the condition method. Or ... just do all the work in the condition method -- which is where we are now. Doug> OTOH, it seems like Python-based breakpoints have two conditions Doug> now (or at least two kinds of conditions one has to think about). Doug> One set with the "condition" command and one from the "evaluate" Doug> API function. IWBN to have only one, at least conceptually. Doug> Maybe you could rename "evaluate" to "condition" (or some such, I Doug> realize there's already a "condition"), and have the default be to Doug> use the CLI condition. This would be fine with me as long as it meets all the goals above. Tom