From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10341 invoked by alias); 23 Jan 2007 21:35:22 -0000 Received: (qmail 10332 invoked by uid 22791); 23 Jan 2007 21:35:21 -0000 X-Spam-Check-By: sourceware.org Received: from zigzag.lvk.cs.msu.su (HELO zigzag.lvk.cs.msu.su) (158.250.17.23) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 23 Jan 2007 21:35:17 +0000 Received: from Debian-exim by zigzag.lvk.cs.msu.su with spam-scanned (Exim 4.50) id 1H9TIK-0005PK-W5 for gdb-patches@sources.redhat.com; Wed, 24 Jan 2007 00:35:14 +0300 Received: from localhost ([127.0.0.1] helo=ip6-localhost) by zigzag.lvk.cs.msu.su with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA:32) (Exim 4.50) id 1H9TI6-0005Ou-Pq; Wed, 24 Jan 2007 00:34:54 +0300 From: Vladimir Prus To: Nick Roberts Subject: Re: [PATCH] MI: Free values when updating Date: Tue, 23 Jan 2007 21:35:00 -0000 User-Agent: KMail/1.9.1 Cc: Daniel Jacobowitz , gdb-patches@sources.redhat.com References: <17845.48393.877158.536969@kahikatea.snap.net.nz> <20070123121147.GA32010@nevyn.them.org> <17846.31726.382269.143176@kahikatea.snap.net.nz> In-Reply-To: <17846.31726.382269.143176@kahikatea.snap.net.nz> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200701240034.52325.ghost@cs.msu.su> 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: 2007-01/txt/msg00484.txt.bz2 On Wednesday 24 January 2007 00:19, Nick Roberts wrote: > > > I think if you also remove the (3) calls to release_value in > > > c_value_of_child and cplus_value_of_child this is equivalent to my change > > > (and more tidy). > > > > No, those are different. They come from things like the call to > > gdb_value_ind in c_describe_child. That creates a new value, which is > > returned to the caller (the MI front end, to be printed and later > > released). It's the ones in c_value_of_root which matter, because we > > save them in the varobj. > > In varobj_update: > > new = value_of_child (v->parent, v->index); > if (install_new_value (v, new, 0 /* type not changed */)) > > In create_child: > > value = value_of_child (parent, index); > ... > install_new_value (child, value, 1); > > Now install_new_value calls release_value on new, it's not neeeded in > *_value_of_child. I think that's right. All calls to release_value except in install_new_value can be removed I believe, and I'll post a patch to that effect tomorrow. > I've tried this change on the MI testsuite and see no fails > and it has about the same time improvement that my patch had. If this is not > the right patch then the testsuite is lacking the appropriate test. Can you > create a test where this patch fails? You cannot. For non-reference types, release_value gets called twice, which is benign now. I'll try adding assert in release_value, though. For reference and array types, extra release_value gives you a memleak, and there's nothing that will cause a test to fail due to that. - Volodya