From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23250 invoked by alias); 9 Jan 2007 21:22:08 -0000 Received: (qmail 23236 invoked by uid 22791); 9 Jan 2007 21:22:06 -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; Tue, 09 Jan 2007 21:21:58 +0000 Received: from Debian-exim by zigzag.lvk.cs.msu.su with spam-scanned (Exim 4.50) id 1H4OPm-0008N1-7J for gdb-patches@sources.redhat.com; Wed, 10 Jan 2007 00:21:55 +0300 Received: from localhost ([127.0.0.1] helo=ip6-localhost) by zigzag.lvk.cs.msu.su with esmtp (Exim 4.50) id 1H4OPc-0008Mo-NF; Wed, 10 Jan 2007 00:21:41 +0300 From: Vladimir Prus Subject: Re: Fix 'selected frame' varobjs To: Daniel Jacobowitz , gdb-patches@sources.redhat.com Date: Tue, 09 Jan 2007 21:22:00 -0000 References: <200612310301.42649.ghost@cs.msu.su> <20061231214221.GB26604@nevyn.them.org> User-Agent: KNode/0.10.2 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart1755815.VjR8qlL5CH" Content-Transfer-Encoding: 7Bit Message-Id: 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/msg00266.txt.bz2 --nextPart1755815.VjR8qlL5CH Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8Bit Content-length: 1638 Daniel Jacobowitz wrote: > On Sun, Dec 31, 2006 at 03:01:42AM +0300, Vladimir Prus wrote: >> @@ -1061,6 +1061,14 @@ varobj_update (struct varobj **varp, str >> if (new == NULL) >> { >> (*varp)->error = 1; >> + /* Also set the value to NULL, so that >> + when the value becomes valid in future, >> + -var-update notice the change. */ >> + if ((*varp)->value != NULL) >> + { >> + value_free ((*varp)->value); >> + (*varp)->value = NULL; >> + } >> return -1; >> } >> > > "in the future", "notices the change". Should we be doing this > everywhere that we set var->error (probably add a new helper to do > that)? There's about a half-dozen places. I believe this was the only place where we don't set value to NULL on errors. This reminded me that I wanted to remove var->error for quite some time -- it's used in a single place, and using 'value == NULL' to indicate error is quite as reasonable. What about the attached? - Volodya 2006-12-31 Daniel Jacobowitz Vladimir Prus Fix 'selected frame' varobjs. * varobj.c (struct varobj): Remove the error field. (varobj_set_value): Don't check var->error. (install_new_value): Don't set var->error. (varobj_update): Always pass the new value of the root via install_new_value. (create_child): Don't set error field. (new_variable): Likewise. (c_value_of_root): Always reevaluate the value of selected frame varobjs in the selected frame. Don't call reinit_frame_cache. --nextPart1755815.VjR8qlL5CH Content-Type: text/x-diff; name="at_varobjs__gdb_mainline.diff" Content-Transfer-Encoding: 8Bit Content-Disposition: attachment; filename="at_varobjs__gdb_mainline.diff" Content-length: 4011 --- gdb/varobj.c (/patches/gdb/varobj_update_memleak/gdb_mainline) (revision 3137) +++ gdb/varobj.c (/patches/gdb/at_varobjs/gdb_mainline) (revision 3137) @@ -107,14 +107,12 @@ struct varobj /* The type of this variable. This may NEVER be NULL. */ struct type *type; - /* The value of this expression or subexpression. This may be NULL. + /* The value of this expression or subexpression. A NULL value + indicates there was an error getting this value. Invariant: if varobj_value_is_changeable_p (this) is non-zero, the value is either NULL, or not lazy. */ struct value *value; - /* Did an error occur evaluating the expression or getting its value? */ - int error; - /* The number of (immediate) children this variable has */ int num_children; @@ -811,7 +809,7 @@ varobj_set_value (struct varobj *var, ch struct value *value; int saved_input_radix = input_radix; - if (var->value != NULL && variable_editable (var) && !var->error) + if (var->value != NULL && variable_editable (var)) { char *s = expression; int i; @@ -909,7 +907,6 @@ install_new_value (struct varobj *var, s int need_to_fetch; int changed = 0; - var->error = 0; /* We need to know the varobj's type to decide if the value should be fetched or not. C++ fake children (public/protected/private) don't have a type. */ @@ -946,14 +943,11 @@ install_new_value (struct varobj *var, s { if (!gdb_value_fetch_lazy (value)) { - var->error = 1; /* Set the value to NULL, so that for the next -var-update, we don't try to compare the new value with this value, that we couldn't even read. */ value = NULL; } - else - var->error = 0; } /* If the type is changeable, compare the old and the new values. @@ -1078,12 +1072,6 @@ varobj_update (struct varobj **varp, str if (fi) select_frame (fi); - if (new == NULL) - { - (*varp)->error = 1; - return -1; - } - /* If this is a "use_selected_frame" varobj, and its type has changed, them note that it's changed. */ if (type_changed) @@ -1097,6 +1085,14 @@ varobj_update (struct varobj **varp, str VEC_safe_push (varobj_p, result, *varp); } + if (new == NULL) + { + /* This means the varobj itself is out of scope. + Report it. */ + VEC_free (varobj_p, result); + return -1; + } + VEC_safe_push (varobj_p, stack, *varp); /* Walk through the children, reconstructing them all. */ @@ -1373,9 +1369,6 @@ create_child (struct varobj *parent, int child->index); install_new_value (child, value, 1); - if ((!CPLUS_FAKE_CHILD (child) && child->value == NULL) || parent->error) - child->error = 1; - return child; } @@ -1396,7 +1389,6 @@ new_variable (void) var->index = -1; var->type = NULL; var->value = NULL; - var->error = 0; var->num_children = -1; var->parent = NULL; var->children = NULL; @@ -1975,11 +1967,10 @@ c_value_of_root (struct varobj **var_han /* Determine whether the variable is still around. */ - if (var->root->valid_block == NULL) + if (var->root->valid_block == NULL || var->root->use_selected_frame) within_scope = 1; else { - reinit_frame_cache (); fi = frame_find_by_id (var->root->frame); within_scope = fi != NULL; /* FIXME: select_frame could fail */ @@ -2001,11 +1992,8 @@ c_value_of_root (struct varobj **var_han go on */ if (gdb_evaluate_expression (var->root->exp, &new_val)) { - var->error = 0; release_value (new_val); } - else - var->error = 1; return new_val; } Property changes on: ___________________________________________________________________ Name: csl:base +/all/patches/gdb/varobj_update_memleak/gdb_mainline Name: svk:merge +d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/varobj_update_memleak/gdb_mainline:3134 +e7755896-6108-0410-9592-8049d3e74e28:/mirrors/gdb/trunk:159718 --nextPart1755815.VjR8qlL5CH--