From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23623 invoked by alias); 23 Jun 2011 15:57:14 -0000 Received: (qmail 23614 invoked by uid 22791); 23 Jun 2011 15:57:13 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BT,TW_DB,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; Thu, 23 Jun 2011 15:57:00 +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 p5NFuqit003098 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 23 Jun 2011 11:56:52 -0400 Received: from localhost.localdomain (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p5NFuodE005504; Thu, 23 Jun 2011 11:56:51 -0400 From: Phil Muldoon To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [patch] [python] Expose some breakpoint operations to Python References: <201106231507.14817.pedro@codesourcery.com> <201106231629.39370.pedro@codesourcery.com> Reply-to: pmuldoon@redhat.com X-URL: http://www.redhat.com Date: Thu, 23 Jun 2011 15:57:00 -0000 In-Reply-To: <201106231629.39370.pedro@codesourcery.com> (Pedro Alves's message of "Thu, 23 Jun 2011 16:29:39 +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: 2011-06/txt/msg00359.txt.bz2 Pedro Alves writes: > On Thursday 23 June 2011 15:46:14, Phil Muldoon wrote: > No plan yet. I just started out by adding a single breakpoint_ops instance > that handles all breakpoint types that currently aren't using breakpoint_ops, > and moving all default actions to the corresponding callback. This would > be straightforward if the API is a good fit. But take a look at > the fallback code in print_one_breakpoint_location, and print_one_breakpoint. > Breakpoints with multiple locations are handled a bit differently than > just a fallbach, with multiple calls to print_one_breakpoint. Maybe > we could move all that !print_one code inside the new print_one > callback? Yes the multiple locations thing is a bit weird. But from (my Python) point of view, that is an internal abstraction that the Python glue code will absorb. I don't think this will affect Python API users too much. > Not sure. Also, maybe we should split printing the "what" from > the "address"? This may be an external API change. But we can always abstract that from the user by using keywords in the Python side instead of actual positional arguments. > Not sure either, but it looks bizarre to me that > current print_one implementations need to know to call annotate_field. > Then there are implementations that print free form text > surrounding those values, e.g., see print_one_catch_vfork. These > issues certainly inpact the python api. The annotate_field thing was somewhat interesting. I take care of this for the Python user though so I am not too worried about that. Where the field annotation happens is not a big deal for "me", we can move it from the py-breakpoint code back to some new refactored breakpoint.c code. The user does not need to know about this. > (You'll also note that the ->insert_location and > ->remove_location call sites reveal that something isn't fully > abstracted, since failing to insert different kinds of breakpoints > leads to different reactions in the code). > > Note, the doesn't even compile. I'm not sure about this. The Python breakpoints are just regular breakpoints - we don't even support catchpoints at this point, they are just too flaky to wrap right now. In fact breakpoints created in Python - even the internal ones - are just regular old breakpoints which we masquerade to the user. > Something else that's obviously missing with exposing breakpoint_ops > to python as is (yes I know you haven't exported all of it), is that most > certainly users will want to be able to call the "super" method. That is, > say, e.g, override, the ->check_status method, and still call the default > method, whatever it was. I actually have no (solid) plans to expose anymore of the breakpoint operations than are already exposed in this patch. The other operations require substantial knowledge of GDB's inner-workings and access to non-exported data-structures to be feasible at this youngling Python stage. It is not something that won't ever happen, but it won't happen for awhile. So in this context, refactorings should not affect the Python code. > Well, what does grep tell us? > > breakpoint.c: PRINT_SRC_ONLY: Means we printed something, and we do *not* desire > breakpoint.c: PRINT_SRC_ONLY: Means we printed something, but there is no need > breakpoint.c: if (val == PRINT_SRC_ONLY > breakpoint.h: PRINT_SRC_ONLY, > infrun.c: case PRINT_SRC_ONLY: > > Looks like it's never used? Maybe gdbtk uses it, haven't checked. > Certainly a good example of that cleaning up the house a little before > exposing the API is a good idea. Yeah I did the grep thing too. I ended up exporting it because it was there. We can just remove it if it is too flaky. While we have a strict philosophy on avoiding API changes to the Python API, we can always add content later. But I take your point. I'm trying to position this patch so that users can access a limited subset of printing breakpoint operations. The time-line is important here too. If this refactoring internally is just going to take a few weeks, hey, no big deal, I can just wait and adjust. But if it is a long term thing, I think we could expose the limited functionality we expose now. Cheers Phil -- Registered in England and Wales under Company Registration No. 03798903 Directors: Michael Cunningham (USA), Brendan Lane (Ireland), Matt Parson (USA), Charlie Peters (USA)