* Cleanup varobj children handling
@ 2006-12-08 20:00 Vladimir Prus
2006-12-08 20:22 ` Vladimir Prus
2006-12-21 15:25 ` Vladimir Prus
0 siblings, 2 replies; 11+ messages in thread
From: Vladimir Prus @ 2006-12-08 20:00 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 697 bytes --]
This patch changes varobj.c to use VEC for hodling children, thereby making three
functions unnecessary. No regressions in MI tests. 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.
(varobj_list_children): Adjust to work work vector.
(varobj_update): Likewise.
(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.
[-- Attachment #2: path_2_children__gdb_mainline.diff --]
[-- Type: text/x-diff, Size: 11557 bytes --]
--- gdb/testsuite/gdb.mi/mi2-var-child.exp (/patches/gdb/path_1/gdb_mainline) (revision 2561)
+++ gdb/testsuite/gdb.mi/mi2-var-child.exp (/patches/gdb/path_2_children/gdb_mainline) (revision 2561)
@@ -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 2561)
+++ gdb/testsuite/gdb.mi/mi-var-child.exp (/patches/gdb/path_2_children/gdb_mainline) (revision 2561)
@@ -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 2561)
+++ gdb/varobj.c (/patches/gdb/path_2_children/gdb_mainline) (revision 2561)
@@ -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,6 +134,7 @@ 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 */
@@ -178,14 +185,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);
@@ -714,7 +715,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 *));
@@ -724,13 +732,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;
+
+ 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) = child;
+ *((*childlist) + i) = *existing;
}
/* End of list is marked by a NULL pointer */
@@ -1080,11 +1093,9 @@ varobj_update (struct varobj **varp, str
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)
+ {
+ vpush (&stack, VEC_index (varobj_p, (*varp)->children, i));
}
/* Walk through the children, reconstructing them all. */
@@ -1092,11 +1103,9 @@ varobj_update (struct varobj **varp, str
while (v != NULL)
{
/* 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)
+ {
+ vpush (&stack, VEC_index (varobj_p, v->children, i));
}
/* Update this variable */
@@ -1186,16 +1195,17 @@ delete_variable_1 (struct cpstack **resu
{
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)
@@ -1217,7 +1227,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)
@@ -1346,22 +1356,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)
@@ -1382,9 +1376,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)
@@ -1402,46 +1393,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;
-
-}
\f
/*
--- gdb/Makefile.in (/patches/gdb/path_1/gdb_mainline) (revision 2561)
+++ gdb/Makefile.in (/patches/gdb/path_2_children/gdb_mainline) (revision 2561)
@@ -2844,7 +2844,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_expression/gdb_mainline:2560
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Cleanup varobj children handling
2006-12-08 20:00 Cleanup varobj children handling Vladimir Prus
@ 2006-12-08 20:22 ` Vladimir Prus
2006-12-18 7:28 ` Vladimir Prus
2006-12-21 15:25 ` Vladimir Prus
1 sibling, 1 reply; 11+ messages in thread
From: Vladimir Prus @ 2006-12-08 20:22 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 979 bytes --]
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?
I forgot to remove some more dead code -- struct varobj_child is no longer
needed. This patch removes it too.
- 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.
(varobj_list_children): Adjust to work work vector.
(varobj_update): Likewise.
(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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: path_2_children__gdb_mainline.diff --]
[-- Type: text/x-diff; name="path_2_children__gdb_mainline.diff", Size: 11882 bytes --]
--- gdb/testsuite/gdb.mi/mi2-var-child.exp (/patches/gdb/path_1/gdb_mainline) (revision 2563)
+++ gdb/testsuite/gdb.mi/mi2-var-child.exp (/patches/gdb/path_2_children/gdb_mainline) (revision 2563)
@@ -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 2563)
+++ gdb/testsuite/gdb.mi/mi-var-child.exp (/patches/gdb/path_2_children/gdb_mainline) (revision 2563)
@@ -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 2563)
+++ gdb/varobj.c (/patches/gdb/path_2_children/gdb_mainline) (revision 2563)
@@ -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,20 +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 */
@@ -178,14 +170,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);
@@ -714,7 +700,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 *));
@@ -724,13 +717,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;
+
+ 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) = child;
+ *((*childlist) + i) = *existing;
}
/* End of list is marked by a NULL pointer */
@@ -1080,11 +1078,9 @@ varobj_update (struct varobj **varp, str
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)
+ {
+ vpush (&stack, VEC_index (varobj_p, (*varp)->children, i));
}
/* Walk through the children, reconstructing them all. */
@@ -1092,11 +1088,9 @@ varobj_update (struct varobj **varp, str
while (v != NULL)
{
/* 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)
+ {
+ vpush (&stack, VEC_index (varobj_p, v->children, i));
}
/* Update this variable */
@@ -1184,18 +1178,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)
@@ -1217,7 +1210,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)
@@ -1346,22 +1339,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)
@@ -1382,9 +1359,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)
@@ -1402,46 +1376,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;
-
-}
\f
/*
--- gdb/Makefile.in (/patches/gdb/path_1/gdb_mainline) (revision 2563)
+++ gdb/Makefile.in (/patches/gdb/path_2_children/gdb_mainline) (revision 2563)
@@ -2844,7 +2844,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_expression/gdb_mainline:2562
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Cleanup varobj children handling
2006-12-08 20:22 ` Vladimir Prus
@ 2006-12-18 7:28 ` Vladimir Prus
0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Prus @ 2006-12-18 7:28 UTC (permalink / raw)
To: gdb-patches
Vladimir Prus wrote:
> 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?
>
> I forgot to remove some more dead code -- struct varobj_child is no longer
> needed. This patch removes it too.
>
> - 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.
> (varobj_list_children): Adjust to work work vector.
> (varobj_update): Likewise.
> (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.
PING? The above was posted 10 days ago.
- Volodya
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Cleanup varobj children handling
2006-12-08 20:00 Cleanup varobj children handling Vladimir Prus
2006-12-08 20:22 ` Vladimir Prus
@ 2006-12-21 15:25 ` Vladimir Prus
2006-12-21 22:42 ` Nick Roberts
2006-12-26 16:10 ` Daniel Jacobowitz
1 sibling, 2 replies; 11+ messages in thread
From: Vladimir Prus @ 2006-12-21 15:25 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: path_2_children__gdb_mainline.diff --]
[-- Type: text/x-diff; name="path_2_children__gdb_mainline.diff", Size: 15970 bytes --]
--- 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;
-
-}
\f
/*
@@ -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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Cleanup varobj children handling
2006-12-21 15:25 ` Vladimir Prus
@ 2006-12-21 22:42 ` Nick Roberts
2006-12-22 6:18 ` Vladimir Prus
2006-12-26 16:10 ` Daniel Jacobowitz
1 sibling, 1 reply; 11+ messages in thread
From: Nick Roberts @ 2006-12-21 22:42 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
> > 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?
I'm not familiar with vec.c yet - perhaps you and Daniel J discussed this issue
privately - all I can say is that varobj. c has the comment:
/* Every variable keeps a linked list of its children, described
by the following structure. */
/* FIXME: Deprecated. All should use vlist instead */
Buy perhaps this is not relevant.
--
Nick http://www.inet.net.nz/~nickrob
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Cleanup varobj children handling
2006-12-21 22:42 ` Nick Roberts
@ 2006-12-22 6:18 ` Vladimir Prus
2006-12-22 6:49 ` Nick Roberts
0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Prus @ 2006-12-22 6:18 UTC (permalink / raw)
To: Nick Roberts; +Cc: gdb-patches
On Friday 22 December 2006 01:37, Nick Roberts 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?
>
> I'm not familiar with vec.c yet - perhaps you and Daniel J discussed this issue
> privately - all I can say is that varobj. c has the comment:
>
> /* Every variable keeps a linked list of its children, described
> by the following structure. */
> /* FIXME: Deprecated. All should use vlist instead */
>
> Buy perhaps this is not relevant.
The comment was written before vec.c was available, so it naturally
does not suggest vec.c.
- Volodya
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Cleanup varobj children handling
2006-12-22 6:18 ` Vladimir Prus
@ 2006-12-22 6:49 ` Nick Roberts
2006-12-22 7:26 ` Vladimir Prus
0 siblings, 1 reply; 11+ messages in thread
From: Nick Roberts @ 2006-12-22 6:49 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
> > /* Every variable keeps a linked list of its children, described
> > by the following structure. */
> > /* FIXME: Deprecated. All should use vlist instead */
> >
> > Buy perhaps this is not relevant.
>
> The comment was written before vec.c was available, so it naturally
> does not suggest vec.c.
Before your change varobj.c had two (equivalent?) types of data structure:
vstack and vlist. After your change varobj.c has two (equivalent?) types of
data structure: VEC and vlist.
Should VEC not also be used for vlist?
--
Nick http://www.inet.net.nz/~nickrob
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Cleanup varobj children handling
2006-12-22 6:49 ` Nick Roberts
@ 2006-12-22 7:26 ` Vladimir Prus
0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Prus @ 2006-12-22 7:26 UTC (permalink / raw)
To: Nick Roberts; +Cc: gdb-patches
On Friday 22 December 2006 09:44, Nick Roberts wrote:
> > > /* Every variable keeps a linked list of its children, described
> > > by the following structure. */
> > > /* FIXME: Deprecated. All should use vlist instead */
> > >
> > > Buy perhaps this is not relevant.
> >
> > The comment was written before vec.c was available, so it naturally
> > does not suggest vec.c.
>
> Before your change varobj.c had two (equivalent?) types of data structure:
> vstack and vlist.
Actually, three: vstack, varobj_child and vlist.
> After your change varobj.c has two (equivalent?) types of
> data structure: VEC and vlist.
That's one data type off, plus vec.h is generic code, not private to varobj.c
> Should VEC not also be used for vlist?
Ultimately, yes. But I primarily wanted to cleanup the list of children, so open
path for other changes.
vlist does not get in my way, yet ;-)
- Volodya
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Cleanup varobj children handling
2006-12-21 15:25 ` Vladimir Prus
2006-12-21 22:42 ` Nick Roberts
@ 2006-12-26 16:10 ` Daniel Jacobowitz
2006-12-26 20:50 ` Vladimir Prus
1 sibling, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2006-12-26 16:10 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
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.
> - /* 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.
> if (var->num_children == -1)
> - var->num_children = number_of_children (var);
> + {
> + var->num_children = number_of_children (var);
> + }
Braces :-)
> + /* 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.
> @@ -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.
> /* 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);
> }
Braces again :-) A few other places too.
> + 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.
> - /* 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?
> @@ -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?
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Cleanup varobj children handling
2006-12-26 16:10 ` Daniel Jacobowitz
@ 2006-12-26 20:50 ` Vladimir Prus
2006-12-30 14:57 ` Daniel Jacobowitz
0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Prus @ 2006-12-26 20:50 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 5709 bytes --]
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
[-- Attachment #2: path_2_children__gdb_mainline.diff --]
[-- Type: text/x-diff, Size: 12938 bytes --]
--- 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;
-
-}
\f
/*
@@ -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
[-- Attachment #3: path_2_children_delta.diff --]
[-- Type: text/x-diff, Size: 6876 bytes --]
--- 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;
+ }
}
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Cleanup varobj children handling
2006-12-26 20:50 ` Vladimir Prus
@ 2006-12-30 14:57 ` Daniel Jacobowitz
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2006-12-30 14:57 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
On Tue, Dec 26, 2006 at 11:49:14PM +0300, Vladimir Prus wrote:
> 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?
No, this is OK to leave alone. If you think of the integer index as
opaque, it's really not that unlike a C++ iterator; since we can't make
*ix work, we use two variables.
> + /* If we're called when the list of children is not yet initialized,
> + allocated enough elements in it. */
Allocate, not allocated - this is what we're doing, not what we've
done.
> + /* 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". */
"so that the first child"
> + /* Child may be NULL is explicitly deleted by -var-delete. */
"if", not is.
> + /* Update this variable, unless it's root, which is already
> + updated. */
"unless it's a root"
Otherwise OK, with a changelog entry.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2006-12-30 14:57 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-12-08 20:00 Cleanup varobj children handling Vladimir Prus
2006-12-08 20:22 ` Vladimir Prus
2006-12-18 7:28 ` Vladimir Prus
2006-12-21 15:25 ` Vladimir Prus
2006-12-21 22:42 ` Nick Roberts
2006-12-22 6:18 ` Vladimir Prus
2006-12-22 6:49 ` Nick Roberts
2006-12-22 7:26 ` Vladimir Prus
2006-12-26 16:10 ` Daniel Jacobowitz
2006-12-26 20:50 ` Vladimir Prus
2006-12-30 14:57 ` Daniel Jacobowitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox