From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13054 invoked by alias); 4 Jan 2007 20:35:10 -0000 Received: (qmail 13043 invoked by uid 22791); 4 Jan 2007 20:35:10 -0000 X-Spam-Check-By: sourceware.org Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.8) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 04 Jan 2007 20:35:04 +0000 Received: from kahikatea.snap.net.nz (p202-124-120-54.snap.net.nz [202.124.120.54]) by viper.snap.net.nz (Postfix) with ESMTP id 7D6DC3D838C; Fri, 5 Jan 2007 09:34:56 +1300 (NZDT) Received: by kahikatea.snap.net.nz (Postfix, from userid 500) id 95C394F6CA; Fri, 5 Jan 2007 09:34:55 +1300 (NZDT) From: Nick Roberts Message-ID: <17821.25837.573239.858406@kahikatea.snap.net.nz> Date: Thu, 04 Jan 2007 20:35:00 -0000 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit To: Daniel Jacobowitz Cc: Vladimir Prus , gdb-patches@sources.redhat.com Subject: Re: RFC: MI - Detecting change of string contents with variable objects In-Reply-To: <20070104194033.GB24634@nevyn.them.org> 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> <20070104042038.GA3918@nevyn.them.org> <17820.39505.966623.305338@kahikatea.snap.net.nz> <20070104194033.GB24634@nevyn.them.org> X-Mailer: VM 7.19 under Emacs 22.0.92.5 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/msg00135.txt.bz2 > > + char* print_value = value_get_print_value (value, var->format); > > Use "char *print_value", please. OK. > > gdb_assert (!value_lazy (var->value)); > > > > ! if (var->print_value) > > ! { > > ! if (strcmp (var->print_value, print_value)) > > ! { > > ! xfree (var->print_value); > > ! var->print_value = print_value; > > ! changed = 1; > > ! } > > ! } > > ! else > > ! var->print_value = print_value; > > Should we set changed = 1 in the "else"? No. This is only reached in the initial call to install_new_value i.e -var-create. > Otherwise the patch seems fine, if it tests OK, but I'm still a little > nervous about it. For example, you'll call val_print on a struct > or array to see if it's changed. Depending on things like "set print > elements", that might not print out the whole string. This is > probably a behavior change. Is it a harmless one? If it is, then > should we be sharing the code with c_value_of_variable that avoids > printing structs, unions, and arrays, and never mark them as changed > unless their types change? The function val_print is already used for -var-evaluate-expression. AFAICS "set print elements" has no effect on variable objects, perhaps because val_print is only called on the leaves but I can see that using it might expose MI to the vagaries of CLI. Currently, however, string changes don't get reported at all, without the user changing configuration values. -- Nick http://www.inet.net.nz/~nickrob