From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24165 invoked by alias); 21 Dec 2006 15:25:35 -0000 Received: (qmail 24047 invoked by uid 22791); 21 Dec 2006 15:25:33 -0000 X-Spam-Check-By: sourceware.org Received: from main.gmane.org (HELO ciao.gmane.org) (80.91.229.2) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 21 Dec 2006 15:25:20 +0000 Received: from list by ciao.gmane.org with local (Exim 4.43) id 1GxPnH-0006pi-1G for gdb-patches@sources.redhat.com; Thu, 21 Dec 2006 16:25:15 +0100 Received: from 73-198.umostel.ru ([82.179.73.198]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 21 Dec 2006 16:25:15 +0100 Received: from ghost by 73-198.umostel.ru with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 21 Dec 2006 16:25:15 +0100 To: gdb-patches@sources.redhat.com From: Vladimir Prus Subject: Re: Cleanup varobj children handling Date: Thu, 21 Dec 2006 15:25:00 -0000 Message-ID: References: <200612082300.06688.ghost@cs.msu.su> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart1438989.gjTufMedSp" Content-Transfer-Encoding: 7Bit User-Agent: KNode/0.10.2 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: 2006-12/txt/msg00290.txt.bz2 --nextPart1438989.gjTufMedSp Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8Bit Content-length: 1127 Vladimir Prus wrote: > > This patch changes varobj.c to use VEC for hodling children, thereby > making three functions unnecessary. No regressions in MI tests. OK? Here's a revised patch, that additionally replaces 'vstack' code in varobj.c with VEC(). All in all, some 130 lines of code gets wiped now. OK? - Volodya gdb/ * varobj.c: Include "vec.h". (varobj_p): New typedef, declare vector of those. (struct varobj): Use vector for the 'children' member. (child_exists): Remove. (save_child_in_parent): Remove. (remove_child_from_parent): Remove. (struct varobj_child): Remove. (struct vstack): Remove. (vpush, vpop): Remove. (varobj_list_children): Adjust to work work vector. (varobj_update): Likewise. Use vectors for working stack and result. (delete_variable_1): Likewise. * Makefile.in (varobj.o): Update dependencies. testsuite/ * gdb.mi/mi-var-child.exp: Adjust to the change in order of varobjs reported by -var-update. * gdb.mi/mi2-var-child.exp: Likewise. --nextPart1438989.gjTufMedSp Content-Type: text/x-diff; name="path_2_children__gdb_mainline.diff" Content-Transfer-Encoding: 8Bit Content-Disposition: attachment; filename="path_2_children__gdb_mainline.diff" Content-length: 15970 --- gdb/testsuite/gdb.mi/mi2-var-child.exp (/patches/gdb/path_1/gdb_mainline) (revision 2879) +++ gdb/testsuite/gdb.mi/mi2-var-child.exp (/patches/gdb/path_2_children/gdb_mainline) (revision 2879) @@ -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.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\"\}\\\]" \ + "\\^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\"\}\\\]" \ "update all vars struct_declarations.long_array.3-9 changed" --- gdb/testsuite/gdb.mi/mi-var-child.exp (/patches/gdb/path_1/gdb_mainline) (revision 2879) +++ gdb/testsuite/gdb.mi/mi-var-child.exp (/patches/gdb/path_2_children/gdb_mainline) (revision 2879) @@ -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.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\"\}\\\]" \ + "\\^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\"\}\\\]" \ "update all vars struct_declarations.long_array.3-9 changed" --- gdb/varobj.c (/patches/gdb/path_1/gdb_mainline) (revision 2879) +++ gdb/varobj.c (/patches/gdb/path_2_children/gdb_mainline) (revision 2879) @@ -31,6 +31,8 @@ #include "gdb_string.h" #include "varobj.h" +#include "vec.h" + /* Non-zero if we want to see trace of varobj level stuff. */ @@ -79,6 +81,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 +121,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 +134,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 +161,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 +181,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); @@ -711,7 +684,14 @@ 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. */ + 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 *)); @@ -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; - *((*childlist) + i) = child; + 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); + } + + *((*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,15 +1056,11 @@ 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)) @@ -1087,31 +1068,26 @@ varobj_update (struct varobj **varp, str /* 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); + for (i = 0; i < VEC_length (varobj_p, (*varp)->children); ++i) + { + VEC_safe_push (varobj_p, stack, + VEC_index (varobj_p, (*varp)->children, i)); } /* Walk through the children, reconstructing them all. */ - v = vpop (&stack); - while (v != NULL) + while (!VEC_empty (varobj_p, stack)) { + v = VEC_pop (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); + 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 */ @@ -1119,46 +1095,23 @@ varobj_update (struct varobj **varp, str if (install_new_value (v, new, 0 /* type not changed */)) { /* Note that it's changed */ - vpush (&result, v); + VEC_safe_push (varobj_p, result, v); v->updated = 0; - changed++; } - - /* 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 +1147,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 +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) @@ -1356,22 +1308,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 +1328,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 +1345,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 +1478,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 2879) +++ gdb/Makefile.in (/patches/gdb/path_2_children/gdb_mainline) (revision 2879) @@ -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 --nextPart1438989.gjTufMedSp--