From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31987 invoked by alias); 4 Jan 2007 21:05:35 -0000 Received: (qmail 31977 invoked by uid 22791); 4 Jan 2007 21:05:34 -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 21:05:30 +0000 Received: from Debian-exim by zigzag.lvk.cs.msu.su with spam-scanned (Exim 4.50) id 1H2Zm5-0006cO-TS for gdb-patches@sources.redhat.com; Fri, 05 Jan 2007 00:05:27 +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 1H2Zlu-0006c0-Hh; Fri, 05 Jan 2007 00:05:10 +0300 From: Vladimir Prus To: Nick Roberts Subject: Re: RFC: MI - Detecting change of string contents with variable objects Date: Thu, 04 Jan 2007 21:05: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> <20070104042038.GA3918@nevyn.them.org> <17820.39505.966623.305338@kahikatea.snap.net.nz> In-Reply-To: <17820.39505.966623.305338@kahikatea.snap.net.nz> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Message-Id: <200701050004.30631.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/msg00144.txt.bz2 On Thursday 04 January 2007 09:10, Nick Roberts wrote: > *** varobj.c=A0=A0=A0=A004 Jan 2007 16:22:25 +1300=A0=A0=A0=A0=A0=A01.68 > --- varobj.c=A0=A0=A0=A004 Jan 2007 19:04:25 +1300=A0=A0=A0=A0=A0=A0 > *************** struct varobj > *** 127,132 **** > --- 127,135 ---- > =A0=20 > =A0 =A0 /* Was this variable updated via a varobj_set_value operation */ > =A0 =A0 int updated; > +=20 > + =A0 /* Last print value */ Dot and two spaces ;-) > + =A0 char *print_value; > =A0 }; > =A0=20 > =A0 /* Every variable keeps a linked list of its children, described > *************** static int variable_editable (struct var > *** 234,239 **** > --- 237,245 ---- > =A0=20 > =A0 static char *my_value_of_variable (struct varobj *var); > =A0=20 > + static char *value_get_print_value (struct value *value, > + =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 =A0 =A0enum varobj_display_formats format); > +=20 I don't see a comment to this function either here or at the definition poi= nt. > =A0 static int type_changeable (struct varobj *var); > =A0=20 > =A0 /* C implementation */ > *************** install_new_value (struct varobj *var, s > *** 978,1003 **** > =A0 =A0=A0=A0=A0=A0=A0 =A0 =A0changed =3D 1; > =A0 =A0=A0=A0=A0=A0=A0 =A0else > =A0 =A0=A0=A0=A0=A0=A0 =A0 =A0{ > =A0 =A0=A0=A0=A0=A0=A0 =A0 =A0 =A0gdb_assert (!value_lazy (var->value)); > - =A0=A0=A0=A0=A0=A0 =A0 =A0 =A0gdb_assert (!value_lazy (value)); Did you remove this because it failed? If so, I'd like to understand why. I think this assert is needed -- if the value is lazy, then even if printing code will fetch the value, you'll be comparing current value with current value. That's a definite bug, so must be asserted. > =A0 =A0=A0=A0=A0=A0=A0 =A0 =A0 =A0 > ! =A0=A0=A0=A0=A0=A0 =A0 =A0 =A0if (!value_contents_equal (var->value, va= lue)) > ! =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0changed =3D 1; > =A0 =A0=A0=A0=A0=A0=A0 =A0 =A0} > =A0 =A0=A0=A0=A0=A0=A0} > =A0 =A0 =A0 } > ! =A0 =A0=20 > =A0 =A0 /* We must always keep the new value, since children depend on it= . =A0*/ > =A0 =A0 if (var->value !=3D NULL) > =A0 =A0 =A0 value_free (var->value); > =A0 =A0 var->value =3D value; > =A0 =A0 var->updated =3D 0; > ! =A0=20 > =A0 =A0 gdb_assert (!var->value || value_type (var->value)); Is that a formatting change above? > =A0 /* Update the values for a variable and its children. =A0This is a > =A0 =A0 =A0two-pronged attack. =A0First, re-parse the value for the root's > --- 984,1017 ---- > =A0 =A0=A0=A0=A0=A0=A0 =A0 =A0changed =3D 1; > =A0 =A0=A0=A0=A0=A0=A0 =A0else > =A0 =A0=A0=A0=A0=A0=A0 =A0 =A0{ > + =A0=A0=A0=A0=A0=A0 =A0 =A0 =A0char* print_value =3D value_get_print_val= ue (value, var->format); > =A0 =A0=A0=A0=A0=A0=A0 =A0 =A0 =A0gdb_assert (!value_lazy (var->value)); > =A0 =A0=A0=A0=A0=A0=A0 =A0 =A0 =A0 > ! =A0=A0=A0=A0=A0=A0 =A0 =A0 =A0if (var->print_value) > ! =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0{ > ! =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0if (strcmp (var->print_va= lue, print_value)) Can you use=20 strcmp (var->print_value, print_value) !=3D 0 for legibility? > ! =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 =A0{ > ! =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 =A0 =A0xfree (var->print= _value); > ! =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 =A0 =A0var->print_value = =3D print_value; > ! =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 =A0 =A0changed =3D 1; > ! =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 =A0} So, if values differ you xfree the old one and assign the new one. If the values are the same -- where is 'print_value' freed? - Volodya