From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21002 invoked by alias); 4 Feb 2009 20:16:03 -0000 Received: (qmail 20994 invoked by uid 22791); 4 Feb 2009 20:16:03 -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; Wed, 04 Feb 2009 20:15:56 +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 n14KFqRA006164; Wed, 4 Feb 2009 15:15:52 -0500 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 n14KFpci025417; Wed, 4 Feb 2009 15:15:52 -0500 Received: from opsy.redhat.com (vpn-14-42.rdu.redhat.com [10.11.14.42]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n14KFpFI001640; Wed, 4 Feb 2009 15:15:51 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id 60B4C5080FB; Wed, 4 Feb 2009 13:15:49 -0700 (MST) To: Thiago Jung Bauermann Cc: gdb-patches ml Subject: Re: [RFC][python] Add support for convenience functions implemented in Python References: <1233580604.7000.18.camel@localhost.localdomain> <1233709654.14735.34.camel@localhost.localdomain> From: Tom Tromey Reply-To: tromey@redhat.com Date: Wed, 04 Feb 2009 20:16:00 -0000 In-Reply-To: <1233709654.14735.34.camel@localhost.localdomain> (Thiago Jung Bauermann's message of "Tue\, 03 Feb 2009 23\:07\:34 -0200") 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-02/txt/msg00106.txt.bz2 >>>>> "Thiago" == Thiago Jung Bauermann writes: Thiago> This is a new version of the patch [...] I have a couple little nits. Also I want to point out the memory allocation oddities in this patch ... I am not super concerned by them but I would be interested in other opinions, if there are any. Thiago> diff --git a/gdb/parse.c b/gdb/parse.c Thiago> index eee1f8e..dc2ccb1 100644 [...] Thiago> - this is strictly for the convenience of debugging gdb itself. Gdb Thiago> + this is strictly for the convenience of debugging gdb itself. This is fine but it seems unrelated to this patch. Thiago> + error(_("Error while executing Python code.")); Missing a space before the first "(". Thiago> + error(_("Error while executing Python code.")); Likewise. Thiago> +void Thiago> +gdbpy_initialize_functions (void) Thiago> +{ Thiago> + fnpy_object_type.tp_new = PyType_GenericNew; Thiago> + fnpy_object_type.tp_init = fnpy_init; Lately I am preferring to just put these directly into the global's definition. Thiago> +value_create_internal_function (const char *name, [...] Thiago> + /* The internal_function object is leaked here -- to make it truly Thiago> + deletable, we would have to reference count it and add special Thiago> + code to value_free and value_copy. The setup here is a bit odd Thiago> + in general. It would be better to have a special case in Thiago> + help_command. */ So, this is the primary memory leak. You can see it in action by defining a convenience function, then assigning something else to the variable. That is, if your function is $foo, invoking "set $foo = 0" will leak the function definition. I could implement the reference counting idea if this seems important to anybody. I tend to doubt this will happen much -- it isn't common to define a lot of convenience functions and it is even less common to delete them. Another idea would be to make them un-assignable. Tom