From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27366 invoked by alias); 4 Jan 2007 20:57:57 -0000 Received: (qmail 27358 invoked by uid 22791); 4 Jan 2007 20:57:56 -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; Thu, 04 Jan 2007 20:57:52 +0000 Received: from Debian-exim by zigzag.lvk.cs.msu.su with spam-scanned (Exim 4.50) id 1H2Zei-0005gw-E6 for gdb-patches@sources.redhat.com; Thu, 04 Jan 2007 23:57:49 +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 1H2ZeR-0005fU-B4; Thu, 04 Jan 2007 23:57:27 +0300 From: Vladimir Prus To: Nick Roberts Subject: Re: RFC: MI - Detecting change of string contents with variable objects Date: Thu, 04 Jan 2007 20:57:00 -0000 User-Agent: KMail/1.9.1 Cc: Daniel Jacobowitz , gdb-patches@sources.redhat.com References: <17797.65268.689590.797944@kahikatea.snap.net.nz> <20070104194033.GB24634@nevyn.them.org> <17821.25837.573239.858406@kahikatea.snap.net.nz> In-Reply-To: <17821.25837.573239.858406@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: <200701042356.46995.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/msg00142.txt.bz2 On Thursday 04 January 2007 23:34, Nick Roberts wrote: > > > + 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. How so? For -var-create the 'initial' parameter to install_new_value should be 1, so we have: if (!initial && changeable) { ................ 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; } It seems to be this code is *never* reached during call from -var-create. In fact, I don't seem to understand how var->print_value will be initialized by -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. The code in question is executed only for changeable values, and so will never be executed for structures. - Volodya