From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15777 invoked by alias); 26 Dec 2006 20:50:21 -0000 Received: (qmail 15743 invoked by uid 22791); 26 Dec 2006 20:50:16 -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, 26 Dec 2006 20:50:12 +0000 Received: from Debian-exim by zigzag.lvk.cs.msu.su with spam-scanned (Exim 4.50) id 1GzJFL-0002qH-6D for gdb-patches@sources.redhat.com; Tue, 26 Dec 2006 23:50:09 +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 1GzJF5-0002pu-Sb; Tue, 26 Dec 2006 23:49:49 +0300 From: Vladimir Prus To: Daniel Jacobowitz Subject: Re: Cleanup varobj children handling Date: Tue, 26 Dec 2006 20:50:00 -0000 User-Agent: KMail/1.9.1 Cc: gdb-patches@sources.redhat.com References: <200612082300.06688.ghost@cs.msu.su> <20061226161041.GD16188@nevyn.them.org> In-Reply-To: <20061226161041.GD16188@nevyn.them.org> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_KrYkFzAgeqaWago" Message-Id: <200612262349.14988.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: 2006-12/txt/msg00330.txt.bz2 --Boundary-00=_KrYkFzAgeqaWago Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 5709 On Tuesday 26 December 2006 19:10, Daniel Jacobowitz wrote: > Thanks for doing this, I think it's a good cleanup. To answer Nick's > questions: Vlad and I hadn't talked about this change, but I agree with > him. The FIXME about should use vlists goes along with this one that > Vlad deleted: > > /* FIXME: This should be a generic add to list */ > > vec.h is basically the generic list structure the code was asking for. > > On Thu, Dec 21, 2006 at 06:25:04PM +0300, Vladimir Prus wrote: > > #include "varobj.h" > > +#include "vec.h" > > + > > Extra blank line. Fixed. > > - /* A list of this object's children */ > > - struct varobj_child *children; > > + /* Children of this object. */ > > + VEC (varobj_p) *children; > > I've been omitting the space before the parenthesis for VEC, since it's > a macro creating a type and syntactically very different from a > function call. I can't remember where I got that habit though! Either > way is fine. Unfortunately, my emacs immediately shows VEC (varobj_p) in red, because I have a some code to catch the (numerous) cases when I try to use C++ coding style. I guess it will be a bit hard to to suppress this check for VEC. > > + /* If we're called when the list of children is not yet initialized, > > + allocated enough elements in it. */ > > + while (VEC_length (varobj_p, var->children) < var->num_children) > > + VEC_safe_push (varobj_p, var->children, NULL); > > You need the NULLs initialized here, right? Otherwise you could use > grow instead, but that doesn't initialize. Yes, I need new elements to be NULL. > > @@ -721,13 +701,18 @@ varobj_list_children (struct varobj *var > > /* Mark as the end in case we bail out */ > > *((*childlist) + i) = NULL; > > > > - /* check if child exists, if not create */ > > - name = name_of_child (var, i); > > - child = child_exists (var, name); > > - if (child == NULL) > > - child = create_child (var, i, name); > > + varobj_p *existing = VEC_address (varobj_p, var->children) + i; > > Please declare existing at the top of the block, instead of in the > middle; we only require C90 compatibility. Do you really need to use > VEC_address here? You could use VEC_index to read it and VEC_replace > to modify it; I find that easier to read than a pointer into the vec's > storage. I've changed to use VEC_replace. I don't see much improvement, though. > > + for (i = 0; i < VEC_length (varobj_p, (*varp)->children); ++i) > > + { > > + VEC_safe_push (varobj_p, stack, > > + VEC_index (varobj_p, (*varp)->children, i)); > > } > > Here and elsewhere, VEC_iterate avoids having to call VEC_index - > that's what it's for. I personally find VEC_iterate to be less clear -- because it does not correspond to any iteration pattern in any language I know. Do you insist on using it? > > - /* Copy from result stack to list */ > > - vleft = changed; > > - *cv = vpop (&result); > > - while ((*cv != NULL) && (vleft > 0)) > > - { > > - vleft--; > > - cv++; > > - *cv = vpop (&result); > > - } > > - if (vleft) > > - warning (_("varobj_update: assertion failed - vleft <> 0")); > > + cv = *changelist; > > > > - if (changed > 1) > > + for (i = 0; i < changed; ++i) > > { > > - /* Now we revert the order. */ > > - for (i = 0; i < changed; i++) > > - *(*changelist + i) = *(templist + changed - 1 - i); > > - *(*changelist + changed) = NULL; > > + *cv = VEC_index (varobj_p, result, i); > > + gdb_assert (*cv != NULL); > > + ++cv; > > } > > + *cv = 0; > > > > if (type_changed) > > return -2; > > It looks to me as if the old code was somewhat careful about the > ordering, and the old ordering was a bit nicer. But it also looks to > me as if you're preserving the same ordering - vpop takes what would be > the "last" item in the vstack. So why does this patch reverse the order > for the testsuite? Here's what happens. The old code used vpush to construct the result, and then a loop ("revert the order" comment above) that vpops elements and places them to the right position. The new code uses VEC_safe_push. We don't need to reverse the order -- because with VEC we have random access anyway. So, no order change here. However, the order of children is different. Old code used a hand-made list, with each new constructed child added the the front of the list. New code uses VEC, with each new child added at the end of the list. So, for children we iterate them in natural order and push them to working stack. We pop the last child, see that it changed, and push it to the result vector. At this point, the order of children of any given varobj is reversed. I've just worked this around by pushing the children in reverse order. > > @@ -1227,7 +1179,7 @@ delete_variable_1 (struct cpstack **resu > > discarding the list afterwards */ > > if ((remove_from_parent_p) && (var->parent != NULL)) > > { > > - remove_child_from_parent (var->parent, var); > > + VEC_replace (varobj_p, var->parent->children, var->index, NULL); > > } > > > > if (var->obj_name != NULL) > > I think that this will cause crashes in -var-update, since that walks > the list of children without checking for NULL. If I'm right, you > probably want to add NULL checks rather than use VEC_ordered_remove, > so that the indexes still tell you which child it is? Ick! You're right, this will segfault. Not a common case though -- I don't think anybody's going to delete select children of a varobj. Fixed. Please find the revised patch attached, as well as delta relative to the previous version. - Volodya --Boundary-00=_KrYkFzAgeqaWago Content-Type: text/x-diff; charset="iso-8859-1"; name="path_2_children__gdb_mainline.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="path_2_children__gdb_mainline.diff" Content-length: 12938 --- gdb/varobj.c (/patches/gdb/path_1/gdb_mainline) (revision 2957) +++ gdb/varobj.c (/patches/gdb/path_2_children/gdb_mainline) (revision 2957) @@ -31,6 +31,7 @@ #include "gdb_string.h" #include "varobj.h" +#include "vec.h" /* Non-zero if we want to see trace of varobj level stuff. */ @@ -79,6 +80,10 @@ struct varobj_root struct varobj_root *next; }; +typedef struct varobj *varobj_p; + +DEF_VEC_P (varobj_p); + /* Every variable in the system has a structure of this type defined for it. This structure holds all information necessary to manipulate a particular object variable. Members which must be freed are noted. */ @@ -115,8 +120,8 @@ struct varobj /* If this object is a child, this points to its immediate parent. */ struct varobj *parent; - /* A list of this object's children */ - struct varobj_child *children; + /* Children of this object. */ + VEC (varobj_p) *children; /* Description of the root variable. Points to root variable for children. */ struct varobj_root *root; @@ -128,29 +133,6 @@ struct varobj int updated; }; -/* Every variable keeps a linked list of its children, described - by the following structure. */ -/* FIXME: Deprecated. All should use vlist instead */ - -struct varobj_child -{ - - /* Pointer to the child's data */ - struct varobj *child; - - /* Pointer to the next child */ - struct varobj_child *next; -}; - -/* A stack of varobjs */ -/* FIXME: Deprecated. All should use vlist instead */ - -struct vstack -{ - struct varobj *var; - struct vstack *next; -}; - struct cpstack { char *name; @@ -178,14 +160,8 @@ static int install_variable (struct varo static void uninstall_variable (struct varobj *); -static struct varobj *child_exists (struct varobj *, char *); - static struct varobj *create_child (struct varobj *, int, char *); -static void save_child_in_parent (struct varobj *, struct varobj *); - -static void remove_child_from_parent (struct varobj *, struct varobj *); - /* Utility routines */ static struct varobj *new_variable (void); @@ -204,10 +180,6 @@ static struct type *get_target_type (str static enum varobj_display_formats variable_default_display (struct varobj *); -static void vpush (struct vstack **pstack, struct varobj *var); - -static struct varobj *vpop (struct vstack **pstack); - static void cppush (struct cpstack **pstack, char *name); static char *cppop (struct cpstack **pstack); @@ -713,21 +685,34 @@ varobj_list_children (struct varobj *var if (var->num_children == -1) var->num_children = number_of_children (var); + /* If we're called when the list of children is not yet initialized, + allocated enough elements in it. */ + while (VEC_length (varobj_p, var->children) < var->num_children) + VEC_safe_push (varobj_p, var->children, NULL); + /* List of children */ *childlist = xmalloc ((var->num_children + 1) * sizeof (struct varobj *)); for (i = 0; i < var->num_children; i++) { + varobj_p existing; + /* Mark as the end in case we bail out */ *((*childlist) + i) = NULL; - /* check if child exists, if not create */ - name = name_of_child (var, i); - child = child_exists (var, name); - if (child == NULL) - child = create_child (var, i, name); + existing = VEC_index (varobj_p, var->children, i); + + if (existing == NULL) + { + /* Either it's the first call to varobj_list_children for + this variable object, and the child was never created, + or it was explicitly deleted by the client. */ + name = name_of_child (var, i); + existing = create_child (var, i, name); + VEC_replace (varobj_p, var->children, i, existing); + } - *((*childlist) + i) = child; + *((*childlist) + i) = existing; } /* End of list is marked by a NULL pointer */ @@ -1034,8 +1019,8 @@ varobj_update (struct varobj **varp, str struct varobj **cv; struct varobj **templist = NULL; struct value *new; - struct vstack *stack = NULL; - struct vstack *result = NULL; + VEC (varobj_p) *stack = NULL; + VEC (varobj_p) *result = NULL; struct frame_id old_fid; struct frame_info *fi; @@ -1071,94 +1056,64 @@ varobj_update (struct varobj **varp, str return -1; } - /* Initialize a stack for temporary results */ - vpush (&result, NULL); - /* If this is a "use_selected_frame" varobj, and its type has changed, them note that it's changed. */ if (type_changed) - { - vpush (&result, *varp); - changed++; - } + VEC_safe_push (varobj_p, result, *varp); if (install_new_value ((*varp), new, type_changed)) { /* If type_changed is 1, install_new_value will never return non-zero, so we'll never report the same variable twice. */ gdb_assert (!type_changed); - vpush (&result, (*varp)); - changed++; + VEC_safe_push (varobj_p, result, *varp); } - /* Initialize a stack */ - vpush (&stack, NULL); - - /* Push the root's children */ - if ((*varp)->children != NULL) - { - struct varobj_child *c; - for (c = (*varp)->children; c != NULL; c = c->next) - vpush (&stack, c->child); - } + VEC_safe_push (varobj_p, stack, *varp); /* Walk through the children, reconstructing them all. */ - v = vpop (&stack); - while (v != NULL) + while (!VEC_empty (varobj_p, stack)) { - /* Push any children */ - if (v->children != NULL) - { - struct varobj_child *c; - for (c = v->children; c != NULL; c = c->next) - vpush (&stack, c->child); - } + v = VEC_pop (varobj_p, stack); - /* Update this variable */ - new = value_of_child (v->parent, v->index); - if (install_new_value (v, new, 0 /* type not changed */)) - { - /* Note that it's changed */ - vpush (&result, v); - v->updated = 0; - changed++; + /* Push any children. Use reverse order so that first + child is popped from the work stack first, and so + will be added to result first. This does not + affect correctness, just "nicer". */ + for (i = VEC_length (varobj_p, v->children)-1; i >= 0; --i) + { + varobj_p c = VEC_index (varobj_p, v->children, i); + /* Child may be NULL is explicitly deleted by -var-delete. */ + if (c != NULL) + VEC_safe_push (varobj_p, stack, c); + } + + /* Update this variable, unless it's root, which is already + updated. */ + if (v != *varp) + { + new = value_of_child (v->parent, v->index); + if (install_new_value (v, new, 0 /* type not changed */)) + { + /* Note that it's changed */ + VEC_safe_push (varobj_p, result, v); + v->updated = 0; + } } - - /* Get next child */ - v = vpop (&stack); } /* Alloc (changed + 1) list entries */ - /* FIXME: add a cleanup for the allocated list(s) - because one day the select_frame called below can longjump */ + changed = VEC_length (varobj_p, result); *changelist = xmalloc ((changed + 1) * sizeof (struct varobj *)); - if (changed > 1) - { - templist = xmalloc ((changed + 1) * sizeof (struct varobj *)); - cv = templist; - } - else - cv = *changelist; - - /* Copy from result stack to list */ - vleft = changed; - *cv = vpop (&result); - while ((*cv != NULL) && (vleft > 0)) - { - vleft--; - cv++; - *cv = vpop (&result); - } - if (vleft) - warning (_("varobj_update: assertion failed - vleft <> 0")); + cv = *changelist; - if (changed > 1) + for (i = 0; i < changed; ++i) { - /* Now we revert the order. */ - for (i = 0; i < changed; i++) - *(*changelist + i) = *(templist + changed - 1 - i); - *(*changelist + changed) = NULL; + *cv = VEC_index (varobj_p, result, i); + gdb_assert (*cv != NULL); + ++cv; } + *cv = 0; if (type_changed) return -2; @@ -1194,18 +1149,17 @@ delete_variable_1 (struct cpstack **resu struct varobj *var, int only_children_p, int remove_from_parent_p) { - struct varobj_child *vc; - struct varobj_child *next; + int i; /* Delete any children of this variable, too. */ - for (vc = var->children; vc != NULL; vc = next) - { + for (i = 0; i < VEC_length (varobj_p, var->children); ++i) + { + varobj_p child = VEC_index (varobj_p, var->children, i); if (!remove_from_parent_p) - vc->child->parent = NULL; - delete_variable_1 (resultp, delcountp, vc->child, 0, only_children_p); - next = vc->next; - xfree (vc); + child->parent = NULL; + delete_variable_1 (resultp, delcountp, child, 0, only_children_p); } + VEC_free (varobj_p, var->children); /* if we were called to delete only the children we are done here */ if (only_children_p) @@ -1227,7 +1181,7 @@ delete_variable_1 (struct cpstack **resu discarding the list afterwards */ if ((remove_from_parent_p) && (var->parent != NULL)) { - remove_child_from_parent (var->parent, var); + VEC_replace (varobj_p, var->parent->children, var->index, NULL); } if (var->obj_name != NULL) @@ -1356,22 +1310,6 @@ uninstall_variable (struct varobj *var) } -/* Does a child with the name NAME exist in VAR? If so, return its data. - If not, return NULL. */ -static struct varobj * -child_exists (struct varobj *var, char *name) -{ - struct varobj_child *vc; - - for (vc = var->children; vc != NULL; vc = vc->next) - { - if (strcmp (vc->child->name, name) == 0) - return vc->child; - } - - return NULL; -} - /* Create and install a child of the parent of the given name */ static struct varobj * create_child (struct varobj *parent, int index, char *name) @@ -1392,9 +1330,6 @@ create_child (struct varobj *parent, int child->obj_name = childs_name; install_variable (child); - /* Save a pointer to this child in the parent */ - save_child_in_parent (parent, child); - /* Compute the type of the child. Must do this before calling install_new_value. */ if (value != NULL) @@ -1412,46 +1347,6 @@ create_child (struct varobj *parent, int return child; } - -/* FIXME: This should be a generic add to list */ -/* Save CHILD in the PARENT's data. */ -static void -save_child_in_parent (struct varobj *parent, struct varobj *child) -{ - struct varobj_child *vc; - - /* Insert the child at the top */ - vc = parent->children; - parent->children = - (struct varobj_child *) xmalloc (sizeof (struct varobj_child)); - - parent->children->next = vc; - parent->children->child = child; -} - -/* FIXME: This should be a generic remove from list */ -/* Remove the CHILD from the PARENT's list of children. */ -static void -remove_child_from_parent (struct varobj *parent, struct varobj *child) -{ - struct varobj_child *vc, *prev; - - /* Find the child in the parent's list */ - prev = NULL; - for (vc = parent->children; vc != NULL;) - { - if (vc->child == child) - break; - prev = vc; - vc = vc->next; - } - - if (prev == NULL) - parent->children = vc->next; - else - prev->next = vc->next; - -} /* @@ -1585,36 +1480,6 @@ variable_default_display (struct varobj /* FIXME: The following should be generic for any pointer */ static void -vpush (struct vstack **pstack, struct varobj *var) -{ - struct vstack *s; - - s = (struct vstack *) xmalloc (sizeof (struct vstack)); - s->var = var; - s->next = *pstack; - *pstack = s; -} - -/* FIXME: The following should be generic for any pointer */ -static struct varobj * -vpop (struct vstack **pstack) -{ - struct vstack *s; - struct varobj *v; - - if ((*pstack)->var == NULL && (*pstack)->next == NULL) - return NULL; - - s = *pstack; - v = s->var; - *pstack = (*pstack)->next; - xfree (s); - - return v; -} - -/* FIXME: The following should be generic for any pointer */ -static void cppush (struct cpstack **pstack, char *name) { struct cpstack *s; --- gdb/Makefile.in (/patches/gdb/path_1/gdb_mainline) (revision 2957) +++ gdb/Makefile.in (/patches/gdb/path_2_children/gdb_mainline) (revision 2957) @@ -2842,7 +2842,7 @@ value.o: value.c $(defs_h) $(gdb_string_ $(gdb_assert_h) $(regcache_h) $(block_h) varobj.o: varobj.c $(defs_h) $(exceptions_h) $(value_h) $(expression_h) \ $(frame_h) $(language_h) $(wrapper_h) $(gdbcmd_h) $(gdb_assert_h) \ - $(gdb_string_h) $(varobj_h) + $(gdb_string_h) $(varobj_h) $(vec_h) vaxbsd-nat.o: vaxbsd-nat.c $(defs_h) $(inferior_h) $(regcache_h) $(target_h) \ $(vax_tdep_h) $(inf_ptrace_h) $(bsd_kvm_h) vax-nat.o: vax-nat.c $(defs_h) $(inferior_h) $(gdb_assert_h) $(vax_tdep_h) \ Property changes on: ___________________________________________________________________ Name: csl:base +/all/patches/gdb/path_1/gdb_mainline Name: svk:merge +d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_1/gdb_mainline:2869 +d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_expression/gdb_mainline:2562 e7755896-6108-0410-9592-8049d3e74e28:/mirrors/gdb/trunk:157978 --Boundary-00=_KrYkFzAgeqaWago Content-Type: text/x-diff; charset="iso-8859-1"; name="path_2_children_delta.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="path_2_children_delta.diff" Content-length: 6876 --- gdb/testsuite/gdb.mi/mi-var-child.exp (revision 2956) +++ gdb/testsuite/gdb.mi/mi-var-child.exp (local) @@ -835,7 +835,7 @@ mi_execute_to "exec-step 7" "end-steppin # Test: c_variable-5.8 # Desc: check that long_array[3-9] changed mi_gdb_test "-var-update *" \ - "\\^done,changelist=\\\[\{name=\"struct_declarations.long_array.9\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.8\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.7\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.6\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.5\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.4\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.3\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \ + "\\^done,changelist=\\\[\{name=\"struct_declarations.long_array.3\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.4\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.5\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.6\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.7\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.8\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.9\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \ "update all vars struct_declarations.long_array.3-9 changed" --- gdb/testsuite/gdb.mi/mi2-var-child.exp (revision 2956) +++ gdb/testsuite/gdb.mi/mi2-var-child.exp (local) @@ -831,7 +831,7 @@ mi_execute_to "exec-step 7" "end-steppin # Test: c_variable-5.8 # Desc: check that long_array[3-9] changed mi_gdb_test "-var-update *" \ - "\\^done,changelist=\\\[\{name=\"struct_declarations.long_array.9\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.8\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.7\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.6\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.5\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.4\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.3\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \ + "\\^done,changelist=\\\[\{name=\"struct_declarations.long_array.3\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.4\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.5\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.6\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.7\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.8\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"struct_declarations.long_array.9\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \ "update all vars struct_declarations.long_array.3-9 changed" --- gdb/varobj.c (revision 2956) +++ gdb/varobj.c (local) @@ -33,7 +33,6 @@ #include "varobj.h" #include "vec.h" - /* Non-zero if we want to see trace of varobj level stuff. */ int varobjdebug = 0; @@ -684,9 +683,7 @@ varobj_list_children (struct varobj *var *childlist = NULL; if (var->num_children == -1) - { - var->num_children = number_of_children (var); - } + var->num_children = number_of_children (var); /* If we're called when the list of children is not yet initialized, allocated enough elements in it. */ @@ -698,21 +695,24 @@ varobj_list_children (struct varobj *var for (i = 0; i < var->num_children; i++) { + varobj_p existing; + /* Mark as the end in case we bail out */ *((*childlist) + i) = NULL; - varobj_p *existing = VEC_address (varobj_p, var->children) + i; + existing = VEC_index (varobj_p, var->children, i); - if (*existing == NULL) + if (existing == NULL) { /* Either it's the first call to varobj_list_children for this variable object, and the child was never created, or it was explicitly deleted by the client. */ name = name_of_child (var, i); - *existing = create_child (var, i, name); + existing = create_child (var, i, name); + VEC_replace (varobj_p, var->children, i, existing); } - *((*childlist) + i) = *existing; + *((*childlist) + i) = existing; } /* End of list is marked by a NULL pointer */ @@ -1059,9 +1059,7 @@ varobj_update (struct varobj **varp, str /* If this is a "use_selected_frame" varobj, and its type has changed, them note that it's changed. */ if (type_changed) - { - VEC_safe_push (varobj_p, result, *varp); - } + VEC_safe_push (varobj_p, result, *varp); if (install_new_value ((*varp), new, type_changed)) { @@ -1071,32 +1069,36 @@ varobj_update (struct varobj **varp, str VEC_safe_push (varobj_p, result, *varp); } - /* Push the root's children */ - for (i = 0; i < VEC_length (varobj_p, (*varp)->children); ++i) - { - VEC_safe_push (varobj_p, stack, - VEC_index (varobj_p, (*varp)->children, i)); - } + VEC_safe_push (varobj_p, stack, *varp); /* Walk through the children, reconstructing them all. */ while (!VEC_empty (varobj_p, stack)) { v = VEC_pop (varobj_p, stack); - /* Push any children */ - for (i = 0; i < VEC_length (varobj_p, v->children); ++i) - { - VEC_safe_push (varobj_p, stack, - VEC_index (varobj_p, v->children, i)); - } - - /* Update this variable */ - new = value_of_child (v->parent, v->index); - if (install_new_value (v, new, 0 /* type not changed */)) - { - /* Note that it's changed */ - VEC_safe_push (varobj_p, result, v); - v->updated = 0; + /* Push any children. Use reverse order so that first + child is popped from the work stack first, and so + will be added to result first. This does not + affect correctness, just "nicer". */ + for (i = VEC_length (varobj_p, v->children)-1; i >= 0; --i) + { + varobj_p c = VEC_index (varobj_p, v->children, i); + /* Child may be NULL is explicitly deleted by -var-delete. */ + if (c != NULL) + VEC_safe_push (varobj_p, stack, c); + } + + /* Update this variable, unless it's root, which is already + updated. */ + if (v != *varp) + { + new = value_of_child (v->parent, v->index); + if (install_new_value (v, new, 0 /* type not changed */)) + { + /* Note that it's changed */ + VEC_safe_push (varobj_p, result, v); + v->updated = 0; + } } } --Boundary-00=_KrYkFzAgeqaWago--