From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19828 invoked by alias); 9 Apr 2009 15:00:05 -0000 Received: (qmail 19690 invoked by uid 22791); 9 Apr 2009 14:59:59 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from e24smtp05.br.ibm.com (HELO e24smtp05.br.ibm.com) (32.104.18.26) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 09 Apr 2009 14:59:53 +0000 Received: from d24relay01.br.ibm.com (d24relay01.br.ibm.com [9.8.31.16]) by e24smtp05.br.ibm.com (8.13.1/8.13.1) with ESMTP id n39EthGd031440 for ; Thu, 9 Apr 2009 11:55:43 -0300 Received: from d24av02.br.ibm.com (d24av02.br.ibm.com [9.18.232.47]) by d24relay01.br.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n39Fx72J3817554 for ; Thu, 9 Apr 2009 12:59:07 -0300 Received: from d24av02.br.ibm.com (loopback [127.0.0.1]) by d24av02.br.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n39ExmbN012814 for ; Thu, 9 Apr 2009 11:59:48 -0300 Received: from [9.8.10.215] ([9.8.10.215]) by d24av02.br.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id n39ExlNA012806; Thu, 9 Apr 2009 11:59:47 -0300 Subject: Re: Python pretty-printing [5/6] From: Thiago Jung Bauermann To: Tom Tromey Cc: gdb-patches ml In-Reply-To: References: <1238863179.3236.134.camel@localhost.localdomain> Content-Type: text/plain; charset="UTF-8" Date: Thu, 09 Apr 2009 15:00:00 -0000 Message-Id: <1239289186.30578.18.camel@localhost.localdomain> Mime-Version: 1.0 Content-Transfer-Encoding: 8bit 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: 2009-04/txt/msg00173.txt.bz2 El mié, 08-04-2009 a las 18:52 -0600, Tom Tromey escribió: > >>>>> "Thiago" == Thiago Jung Bauermann writes: > 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? I can't find it either, now. Should have pointed out the first time around. :-( I probably thought search_pp_list could return NULL without setting a Python exception, and because of this find_pretty_printer would blindly return NULL too. But I can't find that case anymore. I probably made a mistake. I found one potential problem, which could cause the function to return NULL without an exception being set (it's not the case I thought of before, I think): suppose there's no objfile Python object when this function is called, to the ALL_OBJFILES loop will skip all objs, then the gdb module has no pretty_printers attribute, or the pretty_printers value is not a list object. In that case, the function will return NULL without a Python exception being set. Can it happen? Also, I noticed that the function may return Py_None if no pretty-printer is found, and the callers even expect that. The function comment needs to be fixed then: /* Find the pretty-printing constructor function for TYPE. If no pretty-printer exists, return NULL. If one exists, return a new reference. */ > 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. Fine with me. > 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. Alright. -- []'s Thiago Jung Bauermann IBM Linux Technology Center