From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10791 invoked by alias); 4 Jan 2007 04:20:54 -0000 Received: (qmail 10776 invoked by uid 22791); 4 Jan 2007 04:20:52 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Thu, 04 Jan 2007 04:20:45 +0000 Received: from drow by nevyn.them.org with local (Exim 4.63) (envelope-from ) id 1H2K5m-00012h-Rv; Wed, 03 Jan 2007 23:20:38 -0500 Date: Thu, 04 Jan 2007 04:20:00 -0000 From: Daniel Jacobowitz To: Nick Roberts Cc: Vladimir Prus , gdb-patches@sources.redhat.com Subject: Re: RFC: MI - Detecting change of string contents with variable objects Message-ID: <20070104042038.GA3918@nevyn.them.org> Mail-Followup-To: Nick Roberts , Vladimir Prus , gdb-patches@sources.redhat.com References: <17797.65268.689590.797944@kahikatea.snap.net.nz> <17798.19683.251190.740216@kahikatea.snap.net.nz> <200612181136.02429.ghost@cs.msu.su> <20061218133827.GA24800@nevyn.them.org> <17799.3497.476593.138858@kahikatea.snap.net.nz> <20070103224605.GO17935@nevyn.them.org> <17820.32496.851404.587153@kahikatea.snap.net.nz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <17820.32496.851404.587153@kahikatea.snap.net.nz> User-Agent: Mutt/1.5.13 (2006-08-11) 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: 2007-01/txt/msg00101.txt.bz2 On Thu, Jan 04, 2007 at 05:13:36PM +1300, Nick Roberts wrote: > I've probably done it the other way round to how you're asking (used > value_get_print_value in c_value_of_variable) but, likewise, it involves > re-use. Works out the same. It looks reasonable to me. > struct type *type; > > + > /* The value of this expression or subexpression. This may be NULL. Please try not to gratuitously move whitespace around (you did this in a bunch of places). I really appreciate that when reviewing. > ! if (var->print_value) > ! { > ! if (strcmp (var->print_value, > ! value_get_print_value (value, var->format))) > ! { > ! xfree (var->print_value); > ! var->print_value = > ! xstrdup (value_get_print_value (value, var->format)); > ! changed = 1; > ! } > ! } > ! else > ! var->print_value = > ! xstrdup (value_get_print_value (value, var->format)); value_get_print_value does a bunch of work, and returns a malloced string. So you should avoid calling it twice, and avoid leaking the return value. -- Daniel Jacobowitz CodeSourcery