From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15146 invoked by alias); 10 Apr 2007 19:03:56 -0000 Received: (qmail 15135 invoked by uid 22791); 10 Apr 2007 19:03:55 -0000 X-Spam-Check-By: sourceware.org Received: from return.false.org (HELO return.false.org) (66.207.162.98) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 10 Apr 2007 20:03:44 +0100 Received: from return.false.org (localhost [127.0.0.1]) by return.false.org (Postfix) with ESMTP id 769824B26D; Tue, 10 Apr 2007 14:03:42 -0500 (CDT) Received: from caradoc.them.org (dsl093-172-095.pit1.dsl.speakeasy.net [66.93.172.95]) by return.false.org (Postfix) with ESMTP id 3D65D4B262; Tue, 10 Apr 2007 14:03:38 -0500 (CDT) Received: from drow by caradoc.them.org with local (Exim 4.63) (envelope-from ) id 1HbLcu-0006ZR-Ef; Tue, 10 Apr 2007 15:03:36 -0400 Date: Tue, 10 Apr 2007 19:03:00 -0000 From: Daniel Jacobowitz To: Vladimir Prus Cc: gdb-patches@sources.redhat.com Subject: Re: Ping: frozen variable objects Message-ID: <20070410190335.GA22313@caradoc.them.org> Mail-Followup-To: Vladimir Prus , gdb-patches@sources.redhat.com References: <200703251351.43195.vladimir@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200703251351.43195.vladimir@codesourcery.com> User-Agent: Mutt/1.5.15 (2007-04-09) 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-04/txt/msg00100.txt.bz2 On Sun, Mar 25, 2007 at 12:51:42PM +0300, Vladimir Prus wrote: > --- gdb/doc/gdb.texinfo (/mirrors/gdb_mainline) (revision 3540) > +++ gdb/doc/gdb.texinfo (/patches/gdb/frozen/gdb_mainline) (revision 3540) > @@ -19730,6 +19730,8 @@ access this functionality: > @tab set the value of this variable > @item @code{-var-update} > @tab update the variable and its children > +@item @code{-var-set-frozen} > +@tab set frozeness attribute > @end multitable > > In the next subsection we describe each operation in detail and suggest Hmm, that line at the bottom says there's detailed documentation below. But you didn't add any for -var-set-frozen. There's also this: > + if (varobj_get_frozen (var)) > + ui_out_field_int (uiout, "frozen", 1); Somewhere in the manual should mention how that can happen, and what it means. > + mi_check_varobj_value V1.nested.j 2 "check V1.nested.j: 2" > + mi_check_varobj_value V1.nested.k 3 "check V1.nested.j: 3" > + # Check that explicit update for elements of structures > + # works. > + # Update v1.j > + mi_varobj_update V1.nested.j {V1.nested.j} "update V1.nested.j" > + mi_check_varobj_value V1.i 1 "check V1.i: 1" > + mi_check_varobj_value V1.nested.j 8 "check V1.nested.j: 8" > + mi_check_varobj_value V1.nested.k 3 "check V1.nested.j: 3" > + # Update v1.nested, check that children is updated. > + mi_varobj_update V1.nested {V1.nested.k} "update V1.nested" > + mi_check_varobj_value V1.i 1 "check V1.i: 1" > + mi_check_varobj_value V1.nested.j 8 "check V1.nested.j: 8" > + mi_check_varobj_value V1.nested.k 9 "check V1.nested.j: 9" You've got a bunch of .j's in the test names that ought to be .k's. Happens some more below. > + if (argc != 2) > + error (_("mi_cmd_var_set_format: Usage: NAME FROZEN_FLAG.")); I think we decided this should be "-var-set-format:" instead of the function name. > + /* Is this variable frozen. Frozen variables are never implicitly > + updated by -var-update * or -var-update . > + */ Spaces between sentences, and */ on a line by itself - this is one of the awkward bits of our formatting standards when the */ doesn't quite fit on the previous line but I usually just wrap early. I think emacs's M-q will too. > @@ -943,10 +974,23 @@ install_new_value (struct varobj *var, s > /* The new value might be lazy. If the type is changeable, > that is we'll be comparing values of this type, fetch the > value now. Otherwise, on the next update the old value > - will be lazy, which means we've lost that old value. */ > + will be lazy, which means we've lost that old value. */ This added a bunch of trailing whitespace to the line for some reason. > + if (frozen && initial) > + { > + /* For variables that are frozen, or are children of frozen > + variables, we don't do fetch on initial assignment. > + For non-initial assignemnt we do the fetch, since it means we're > + explicitly asked to compare the new value with the old one. */ > + intentionally_not_fetched = 1; > + } Does this mean "-var-assign VAR value" is always going to read VAR? If it's frozen that seems undesirable; maybe we should just assume it's been updated. For hardware mapped registers, some of this may not be right: I noticed that we're saving the installed value, but the hardware could swizzle it as it is entered. The most common place this happens is with registers where a bit always reads as zero, no matter what you write to it. But that's a read-sensitivity problem, not a frozen varobjs problem. > - > - /* Quick comparison of NULL values. */ > - if (var->value == NULL && value == NULL) > - /* Equal. */ > - ; > - else if (var->value == NULL || value == NULL) > + if (var->not_fetched && value_lazy (var->value)) > { > - xfree (var->print_value); > - var->print_value = value_get_print_value (value, var->format); > + /* This is a frozen varobj and the value was never read. > + Presumably, UI shows some "never read" indicator. > + Now that we've fetched the real value, we need to report > + this varobj as changed so that UI can show the real > + value. */ > changed = 1; > } > - else > + else > { You don't actually need to change the nesting here. You can just add a leading if to the existing if / else if / else. That'd be easier to read. > if (cvalue && value) > { > - *cvalue = value_cast (TYPE_FIELD_TYPE (type, index), value); > + *cvalue = value_cast (TYPE_FIELD_TYPE (type, index), value); > } Random tab damage? -- Daniel Jacobowitz CodeSourcery