From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14692 invoked by alias); 3 Nov 2007 23:14:14 -0000 Received: (qmail 14683 invoked by uid 22791); 3 Nov 2007 23:14:13 -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; Sat, 03 Nov 2007 23:14:08 +0000 Received: from kahikatea.snap.net.nz (81.60.255.123.dynamic.snap.net.nz [123.255.60.81]) by viper.snap.net.nz (Postfix) with ESMTP id 676AA3DA4D8; Sun, 4 Nov 2007 12:14:01 +1300 (NZDT) Received: by kahikatea.snap.net.nz (Postfix, from userid 1000) id 4DBA88FC6D; Sun, 4 Nov 2007 12:14:00 +1300 (NZDT) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Message-ID: <18221.183.198223.704715@kahikatea.snap.net.nz> Date: Sat, 03 Nov 2007 23:14:00 -0000 To: Vladimir Prus Cc: Daniel Jacobowitz , gdb-patches@sources.redhat.com Subject: Re: [PATCH] MI: lvalues and variable_editable In-Reply-To: <200711031247.54239.ghost@cs.msu.su> References: <18210.27153.559569.601092@kahikatea.snap.net.nz> <20071102113610.GA16883@caradoc.them.org> <18220.15856.752225.9086@kahikatea.snap.net.nz> <200711031247.54239.ghost@cs.msu.su> X-Mailer: VM 7.19 under Emacs 23.0.50.42 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-11/txt/msg00034.txt.bz2 > > =A0 /* Returns a malloc'ed list with all root variable objects */ > > --- 872,920 ---- > > =A0 =A0 struct value *value; > > =A0 =A0 int saved_input_radix =3D input_radix; >=20 > I suggest >=20 >=20 > gdb_assert (varobj_is_editable_p (var)); >=20 > here, so clearly document the assumptions the trailing > code makes. Sure. >... > > + =A0 if (CPLUS_FAKE_CHILD (var)) > > + =A0 =A0 return 0; > > +=20 > > + =A0 if (!(var->root->is_valid && var->value && VALUE_LVAL(var->value= ))) > > + =A0 =A0 return 0; >=20 > I believe this test captures CPLUS_FAKE_CHILD as well, so you don't > need to have separate test for that. I'll remove it. >... > > =A0 =A0 /* FIXME: define masks for attributes */ > > ! =A0 if (!varobj_editable_p (var)) > > =A0 =A0 =A0 error (_("mi_cmd_var_assign: Variable object is not editab= le")); >=20 > I presume the FIXME is now stale? I'll remove that too >... > Aside from the minor points above, I don't have any objections to this > patch. Dan, what would you say? >=20 > - Volodya --=20 Nick http://www.inet.net.nz/~nick= rob