From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18762 invoked by alias); 9 Apr 2009 00:52:41 -0000 Received: (qmail 18754 invoked by uid 22791); 9 Apr 2009 00:52:40 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 09 Apr 2009 00:52:32 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n390qRop017941; Wed, 8 Apr 2009 20:52:27 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n390qSYV001211; Wed, 8 Apr 2009 20:52:28 -0400 Received: from opsy.redhat.com (vpn-13-1.rdu.redhat.com [10.11.13.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n390qQa8025336; Wed, 8 Apr 2009 20:52:27 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id AFCFE3781B3; Wed, 8 Apr 2009 18:52:24 -0600 (MDT) To: Thiago Jung Bauermann Cc: gdb-patches ml Subject: Re: Python pretty-printing [5/6] References: <1238863179.3236.134.camel@localhost.localdomain> From: Tom Tromey Reply-To: Tom Tromey Date: Thu, 09 Apr 2009 00:52:00 -0000 In-Reply-To: <1238863179.3236.134.camel@localhost.localdomain> (Thiago Jung Bauermann's message of "Sat\, 04 Apr 2009 13\:39\:38 -0300") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.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: 2009-04/txt/msg00167.txt.bz2 >>>>> "Thiago" == Thiago Jung Bauermann writes: Thiago> The pretty-printing specific code in python.c is enough to Thiago> warrant it to go in its own .c file. WDYT of moving it to, Thiago> say, python-prettyprint.c? Ok, I'll do that. Thiago> Perhaps we should explain which Python objects can be Thiago> convertible to a GDB value? Thanks, I did this too. Tom> +We recommend that you put your core pretty-printers into a versioned Tom> +python package, and then restrict your auto-loaded code to idempotent Thiago> Sorry if this is silly, but: at least to me, it's not immediately clear Thiago> what "versioned python package" means. The first thing I think of is a Thiago> python package checked into a VCS but since this cannot be what the text Thiago> is talking about, my brain raises a parser error. Perhaps rewording this Thiago> to "... into a python package whose name includes the library version" Thiago> or something like that? Did it. Tom> +find_pretty_printer (PyObject *value) Thiago> Also, if this function returns NULL, sometimes a Python exception will Thiago> be set, sometimes it won't. Is this intended? If so, this should be Thiago> noted. I looked again and I don't see how it can return NULL without setting the exception. Can you point it out to me? Thiago> The comment about convert_value_from_python above is outdated. Jim Thiago> Blandy fixed it to always return a caller-owned result. Thanks, I fixed this. Thiago> Because of this, pretty_print_one_value can now probably just call Thiago> convert_value_from_python and return a struct value in all cases and be Thiago> done with it. Either this function should be changed to work that way, Thiago> or the comment above removed. I made the minimal change here. I don't want to refactor this right now; Phil is in the middle of doing that for the embedded \0 problem. We can apply his fix separately; the current code is not ideal, due to this problem, but it is still usable for a variety of printers. [...] Thiago> But only because varobj.c uses and expects Thiago> \0-terminated strings, so IMHO it's better to make the change, and Thiago> either fix or accomodate varobj.c. WDYT? I'll make sure Phil looks at the varobj part of this problem as well. It is less clear to me how MI ought to work with embedded \0 in a string, so if an MI expert wants to speak up... Tom> +gdbpy_instantiate_printer (PyObject *cons, PyObject *value) Thiago> Is it worth having this function, which is in fact just a call to Thiago> PyObject_CallFunctionObjArgs? Nope, it was a leftover. I nuked it. Tom> +char * Tom> +gdbpy_get_display_hint (PyObject *printer) Thiago> I use the *py_ prefix for functions that can be directly called from Thiago> Python. I think it's a useful hint. I don't think I ever mentioned this Thiago> practice though. Thiago> If you agree it's useful, this method should be renamed to not use the Thiago> gdbpy_ prefix. I am leaving this for now. We use the gdbpy_ prefix inconsistently, even in CVS gdb, and I would prefer to see a single cleanup if we are going to adopt a solid naming convention. I will send an updated patch shortly. Tom