Re: [PATCH:MI] Return a subset of a variable object's children

Nick Roberts
2008-05-10T02:54:43Z

> > > Clearly I'm not going to do either but we could simply go back to using
> > > the linked list structures that were already in varobj.c. It's a
> > > question of whether the convenience outweighs the handicap of having to
> > > work with vectors all the time or not. IMHO it doesn't.
> >
> > I disagree, and I haven't yet seen a practical "handicap". We're not going
> > back to hand-written data structures for MI implementation.
>
> Well it's a shame that convenience should dictate the type of structures that
> are used because I can get this patch to work with an old Gdb using linked
> lists.

As you've made your decision, the patch below isn't a RFA or RFC but a record
for the archives to ensure it's not lost like other patches that I've left
lying around. I feel sure that one day the issue will arise again, probably
when Eclipse finds a requirement for it.

It's against current Gdb and uses the old linked lists. This means that the
size of the data structure is only proportional to the number of children
created and not the total number of possible children. Unlike the previous
patch, children are listed in sequence for both creation and update. Also
children can be deleted. Searching for a location to insert a child in the
list is linear (as before) but I anticipate that a front end would only ever
create a small number at any one time and delete children (that are not
visible) if the number became too great. In any case the search could be made
binary (O(log N)), if necessary.

Oh, yes, and it's a unified diff!

Here's a sample output. Note that var1.7 is deleted and therefore not
listed in the output of -var-update.

-var-create - * m
^done,name="var1",numchild="10",value="[10]",type="int [10]",thread-id="1"
(gdb)
-var-list-children -f 7 -n 2 var1
^done,numchild="2",children=[child={name="var1.7",exp="7",numchild="0",value="-1209567320",type="int",thread-id="1"},child={name="var1.8",exp="8",numchild="0",value="134513116",type="int",thread-id="1"}]
(gdb)
-var-list-children -f 4 -n 3 -s 2 var1
^done,numchild="4",children=[child={name="var1.4",exp="4",numchild="0",value="-1208142416",type="int",thread-id="1"},child={name="var1.6",exp="6",numchild="0",value="134513375",type="int",thread-id="1"},child={name="var1.7",exp="7",numchild="0",value="-1209567320",type="int",thread-id="1"},child={name="var1.8",exp="8",numchild="0",value="134513116",type="int",thread-id="1"}]
(gdb)
-var-delete var1.7
^done,ndeleted="1"
(gdb)
n
&"n\n"
~"151\t int i, n, m[10] = {0, 1, 4, 9, 16, 25, 36, 49, 64, 81};\n"
^done
(gdb)
n
&"n\n"
~"152\t int m1[2][3] = {{0, 1, 2}, {10, 11, 12}};\n"
^done
(gdb)
-var-update --all-values *
^done,changelist=[{name="var1.4",value="16",in_scope="true",type_changed="false"},{name="var1.6",value="36",in_scope="true",type_changed="false"},{name="var1.8",value="64",in_scope="true",type_changed="false"}]
(gdb)

--
Nick http://www.inet.net.nz/~nickrob

2008-05-10 Nick Roberts <nickrob@snap.net.nz>

* mi/mi-cmd-var.c (mi_cmd_var_list_children): Add options to select
a subset of children.

* varobj.h (varobj_list_children): Declare new parameters.

* varobj.c (struct varobj): New member current_children.
(varobj_list_children): Create any new varobjs for children
specified by mi_cmd_var_list_children.
(create_child): Add parameter real_index. Use it.
+ Go back to using the linked list structures.

--- mi-cmd-var.c 20 Apr 2008 10:20:39 +1200 1.50
+++ mi-cmd-var.c 06 May 2008 21:59:54 +1200
@@ -367,47 +367,114 @@ mi_print_value_p (struct type *type, enu
enum mi_cmd_result
mi_cmd_var_list_children (char *command, char **argv, int argc)
{
- struct varobj *var;
- VEC(varobj_p) *children;
- struct varobj *child;
+ struct varobj *var;
+ struct varobj **childlist;
+ struct varobj **cc;
struct cleanup *cleanup_children;
int numchild;
enum print_values print_values;
- int ix;
+ int start, number, stride = 1;

- if (argc != 1 && argc != 2)
- error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
+ enum opt
+ {
+ NO_VALUES, SIMPLE_VALUES, ALL_VALUES, FROM_OPT, NUMBER_OPT, STRIDE_OPT
+ };

- /* Get varobj handle, if a valid var obj name was specified */
- if (argc == 1)
- var = varobj_get_handle (argv[0]);
- else
- var = varobj_get_handle (argv[1]);
- if (var == NULL)
- error (_("Variable object not found"));
+ static struct mi_opt opts[] =
+ {
+ { "-no-values", NO_VALUES, 0 },
+ { "-simple-values", SIMPLE_VALUES, 0 },
+ { "-all-values", ALL_VALUES, 0 },
+ { "f", FROM_OPT, 1 },
+ { "n", NUMBER_OPT, 1 },
+ { "s", STRIDE_OPT, 1 },
+ { 0, 0, 0 }
+ };

- children = varobj_list_children (var);
- ui_out_field_int (uiout, "numchild", VEC_length (varobj_p, children));
- if (argc == 2)
- print_values = mi_parse_values_option (argv[0]);
+ if (argc == 1 || argc == 2)
+ {
+ /* Get varobj handle, if a valid var obj name was specified */
+ if (argc == 1)
+ var = varobj_get_handle (argv[0]);
+ else
+ var = varobj_get_handle (argv[1]);
+ if (var == NULL)
+ error (_("Variable object not found"));
+
+ if (argc == 2)
+ print_values = mi_parse_values_option (argv[0]);
+ else
+ print_values = PRINT_NO_VALUES;
+
+ start = 0;
+ number = varobj_get_num_children (var);
+ }
else
- print_values = PRINT_NO_VALUES;
+ {
+ int optind = 0;
+ char *optarg;
+ while (1)
+ {
+ int opt = mi_getopt ("mi_cmd_var_list_children",
+ argc, argv, opts, &optind, &optarg);
+ if (opt < 0)
+ break;
+ switch ((enum opt) opt)
+ {
+ case NO_VALUES:
+ print_values = PRINT_NO_VALUES;
+ break;
+ case SIMPLE_VALUES:
+ print_values = PRINT_SIMPLE_VALUES;
+ break;
+ case ALL_VALUES:
+ print_values = PRINT_ALL_VALUES;
+ break;
+ case FROM_OPT:
+ start = atol (optarg);
+ if (start < 0)
+ start = 0;
+ break;
+ case NUMBER_OPT:
+ number = atol (optarg);
+ break;
+ case STRIDE_OPT:
+ stride = atol (optarg);
+ if (stride < 1)
+ error (_("Positive stride values only"));
+ break;
+ }
+ }
+ if (optind >= argc)
+ error (_("Missing VARNAME"));
+
+ if (optind < argc - 1)
+ error (_("Garbage at end of command"));
+
+ var = varobj_get_handle (argv[optind]);
+ }
+
+ numchild = varobj_list_children (var, start, number, stride, &childlist);
+ ui_out_field_int (uiout, "numchild", numchild);

- if (VEC_length (varobj_p, children) == 0)
+ if (numchild <= 0)
return MI_CMD_DONE;

if (mi_version (uiout) == 1)
cleanup_children = make_cleanup_ui_out_tuple_begin_end (uiout, "children");
else
cleanup_children = make_cleanup_ui_out_list_begin_end (uiout, "children");
- for (ix = 0; VEC_iterate (varobj_p, children, ix, child); ++ix)
+ cc = childlist;
+ while (*cc != NULL)
{
struct cleanup *cleanup_child;
cleanup_child = make_cleanup_ui_out_tuple_begin_end (uiout, "child");
- print_varobj (child, print_values, 1 /* print expression */);
+ print_varobj (*cc, print_values, 1 /* print expression */);
+ cc++;
do_cleanups (cleanup_child);
}
do_cleanups (cleanup_children);
+ xfree (childlist);
return MI_CMD_DONE;
}

--- varobj.h 10 Apr 2008 08:41:40 +1200 1.18
+++ varobj.h 06 May 2008 22:00:27 +1200
@@ -20,7 +20,6 @@

#include "symtab.h"
#include "gdbtypes.h"
-#include "vec.h"

/* Enumeration for the format types */
enum varobj_display_formats
@@ -62,9 +61,6 @@ extern char *varobj_language_string[];
/* Struct thar describes a variable object instance */
struct varobj;

-typedef struct varobj *varobj_p;
-DEF_VEC_P (varobj_p);
-
/* API functions */

extern struct varobj *varobj_create (char *objname,
@@ -89,17 +85,16 @@ extern enum varobj_display_formats varob
extern enum varobj_display_formats varobj_get_display_format (
struct varobj *var);

+extern int varobj_get_num_children (struct varobj *var);
+
+extern int varobj_list_children (struct varobj *var, int start, int number, int stride,
+ struct varobj ***childlist);
extern int varobj_get_thread_id (struct varobj *var);

extern void varobj_set_frozen (struct varobj *var, int frozen);

extern int varobj_get_frozen (struct varobj *var);

-extern int varobj_get_num_children (struct varobj *var);
-
-/* Return the list of children of VAR. The returned vector
- should not be modified in any way. */
-extern VEC (varobj_p)* varobj_list_children (struct varobj *var);

extern char *varobj_get_type (struct varobj *var);

--- varobj.c 10 May 2008 14:38:50 +1200 1.112
+++ varobj.c 10 May 2008 14:39:47 +1200
@@ -30,7 +30,6 @@
#include "gdb_string.h"

#include "varobj.h"
-#include "vec.h"
#include "gdbthread.h"
#include "inferior.h"

@@ -131,11 +130,15 @@ struct varobj
/* The number of (immediate) children this variable has */
int num_children;

+ /* The number of (immediate) children this variable that exist as varobjs.
+ current_children < num_children */
+ int current_children;
+
/* If this object is a child, this points to its immediate parent. */
struct varobj *parent;

- /* Children of this object. */
- VEC (varobj_p) *children;
+ /* A list of this object's children */
+ struct varobj_child *children;

/* Description of the root variable. Points to root variable for children. */
struct varobj_root *root;
@@ -160,6 +163,29 @@ struct varobj
int not_fetched;
};

+/* 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;
@@ -187,8 +213,14 @@ 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);
@@ -207,6 +239,10 @@ 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);
@@ -751,41 +787,58 @@ varobj_get_num_children (struct varobj *
/* Creates a list of the immediate children of a variable object;
the return code is the number of such children or -1 on error */

-VEC (varobj_p)*
-varobj_list_children (struct varobj *var)
+int
+varobj_list_children (struct varobj *var, int start, int number, int stride,
+ struct varobj ***childlist)
{
struct varobj *child;
+ struct varobj_child *vc;
char *name;
- int i;
+ int i, new_children = 0;

- if (var->num_children == -1)
- var->num_children = number_of_children (var);
+ /* sanity check: have we been passed a pointer? */
+ if (childlist == NULL)
+ return -1;
+
+ *childlist = NULL;

- /* If that failed, give up. */
if (var->num_children == -1)
- return var->children;
+ var->num_children = number_of_children (var);

- /* If we're called when the list of children is not yet initialized,
- allocate enough elements in it. */
- while (VEC_length (varobj_p, var->children) < var->num_children)
- VEC_safe_push (varobj_p, var->children, NULL);
+ /* Allocate maximum required memory */
+ *childlist = xmalloc ((var->current_children + number + 1)
+ * sizeof (struct varobj *));

- for (i = 0; i < var->num_children; i++)
+ for (i = 0; i < number; i++)
{
- varobj_p existing = VEC_index (varobj_p, var->children, i);
+ int real_index = start + i * stride;
+ if (real_index >= var->num_children)
+ break;
+
+ /* Mark as the end in case we bail out */
+ *((*childlist) + i) = NULL;
+
+ /* check if child exists, if not create */
+ name = name_of_child (var, real_index);
+ child = child_exists (var, name);

- if (existing == NULL)
+ if (child == 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);
+ child = create_child (var, real_index, name);
+ new_children++;
}
}

- return var->children;
+ var->current_children += new_children;
+
+ /* Revert the order. */
+ for (vc = var->children, i = 0; vc != NULL; vc = vc->next, i++)
+ *((*childlist) + var->current_children - 1 - i) = vc->child;
+
+ /* End of list is marked by a NULL pointer */
+ *((*childlist) + i) = NULL;
+
+ return var->current_children;
}

/* Obtain the type of an object Variable as a string similar to the one gdb
@@ -1145,8 +1198,8 @@ varobj_update (struct varobj **varp, str
struct varobj **cv;
struct varobj **templist = NULL;
struct value *new;
- VEC (varobj_p) *stack = NULL;
- VEC (varobj_p) *result = NULL;
+ struct vstack *stack = NULL;
+ struct vstack *result = NULL;
struct frame_info *fi;

/* sanity check: have we been passed a pointer? */
@@ -1162,6 +1215,9 @@ varobj_update (struct varobj **varp, str

if (!(*varp)->root->is_valid)
return INVALID;
+
+ /* Initialize a stack for temporary results */
+ vpush (&result, NULL);

if ((*varp)->root->rootvar == *varp)
{
@@ -1173,47 +1229,47 @@ varobj_update (struct varobj **varp, str
new = value_of_root (varp, &type_changed);

/* If this is a floating varobj, and its type has changed,
- them note that it's changed. */
+ then note that it's changed. */
if (type_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);
- VEC_safe_push (varobj_p, result, *varp);
- }
+ {
+ vpush (&result, *varp);
+ changed++;
+ }
+
+ 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++;
+ }

if (new == NULL)
{
/* This means the varobj itself is out of scope.
Report it. */
- VEC_free (varobj_p, result);
+ vpop (&result);
return NOT_IN_SCOPE;
}
}
+
+ /* Initialize a stack */
+ vpush (&stack, NULL);

- VEC_safe_push (varobj_p, stack, *varp);
-
- /* Walk through the children, reconstructing them all. */
- while (!VEC_empty (varobj_p, stack))
+ /* Walk through the children, reconstructing them all. */
+ v = *varp;
+ while (v != NULL)
{
- v = VEC_pop (varobj_p, stack);
-
- /* Push any children. Use reverse order so that the 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)
+ /* Push any unfrozen children */
+ if (v->children != NULL)
{
- varobj_p c = VEC_index (varobj_p, v->children, i);
- /* Child may be NULL if explicitly deleted by -var-delete. */
- if (c != NULL && !c->frozen)
- VEC_safe_push (varobj_p, stack, c);
+ struct varobj_child *c;
+ for (c = v->children; c != NULL; c = c->next)
+ if (!c->child->frozen) vpush (&stack, c->child);
}

- /* Update this variable, unless it's a root, which is already
+ /* Update this variable unless it's a root, which is already
updated. */
if (v->root->rootvar != v)
{
@@ -1221,27 +1277,48 @@ varobj_update (struct varobj **varp, str
if (install_new_value (v, new, 0 /* type not changed */))
{
/* Note that it's changed */
- VEC_safe_push (varobj_p, result, v);
+ vpush (&result, v);
v->updated = 0;
+ changed++;
}
}
+
+ /* Get next child */
+ v = vpop (&stack);
}

- /* Alloc (changed + 1) list entries. */
- changed = VEC_length (varobj_p, result);
+ /* Alloc (changed + 1) list entries */
+ /* FIXME: add a cleanup for the allocated list(s)
+ because one day the select_frame called below can longjump */
*changelist = xmalloc ((changed + 1) * sizeof (struct varobj *));
- cv = *changelist;
+ if (changed > 1)
+ {
+ templist = xmalloc ((changed + 1) * sizeof (struct varobj *));
+ cv = templist;
+ }
+ else
+ cv = *changelist;

- for (i = 0; i < changed; ++i)
+ /* Copy from result stack to list */
+ vleft = changed;
+ *cv = vpop (&result);
+ while ((*cv != NULL) && (vleft > 0))
{
- *cv = VEC_index (varobj_p, result, i);
- gdb_assert (*cv != NULL);
- ++cv;
+ vleft--;
+ cv++;
+ *cv = vpop (&result);
}
- *cv = 0;
+ if (vleft)
+ warning (_("varobj_update: assertion failed - vleft <> 0"));

- VEC_free (varobj_p, stack);
- VEC_free (varobj_p, result);
+ if (changed > 1)
+ {
+ /* Now we revert the order. */
+ for (i = 0; i < changed; i++)
+ // *(*changelist + i) = *(templist + i);
+ *(*changelist + i) = *(templist + changed - 1 - i);
+ *(*changelist + changed) = NULL;
+ }

if (type_changed)
return TYPE_CHANGED;
@@ -1277,19 +1354,18 @@ delete_variable_1 (struct cpstack **resu
struct varobj *var, int only_children_p,
int remove_from_parent_p)
{
- int i;
+ struct varobj_child *vc;
+ struct varobj_child *next;

/* Delete any children of this variable, too. */
- for (i = 0; i < VEC_length (varobj_p, var->children); ++i)
- {
- varobj_p child = VEC_index (varobj_p, var->children, i);
- if (!child)
- continue;
+ for (vc = var->children; vc != NULL; vc = next)
+ {
if (!remove_from_parent_p)
- child->parent = NULL;
- delete_variable_1 (resultp, delcountp, child, 0, only_children_p);
+ vc->child->parent = NULL;
+ delete_variable_1 (resultp, delcountp, vc->child, 0, only_children_p);
+ next = vc->next;
+ xfree (vc);
}
- VEC_free (varobj_p, var->children);

/* if we were called to delete only the children we are done here */
if (only_children_p)
@@ -1311,7 +1387,7 @@ delete_variable_1 (struct cpstack **resu
discarding the list afterwards */
if ((remove_from_parent_p) && (var->parent != NULL))
{
- VEC_replace (varobj_p, var->parent->children, var->index, NULL);
+ remove_child_from_parent (var->parent, var);
}

if (var->obj_name != NULL)
@@ -1440,6 +1516,22 @@ 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)
@@ -1460,6 +1552,9 @@ 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)
@@ -1474,6 +1569,57 @@ 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, *prev = NULL, *next;
+
+ /* Insert the child according to index. */
+ vc = parent->children;
+ while (vc != NULL && vc->child->index > child->index)
+ {
+ prev = vc;
+ vc = vc->next;
+ }
+
+ next = vc;
+ vc = (struct varobj_child *) xmalloc (sizeof (struct varobj_child));
+
+ vc->next = next;
+ vc->child = child;
+
+ if (prev == NULL)
+ parent->children = vc;
+ else
+ prev->next = vc;
+}
+
+/* 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

/*
@@ -1494,6 +1640,7 @@ new_variable (void)
var->type = NULL;
var->value = NULL;
var->num_children = -1;
+ var->current_children = 0;
var->parent = NULL;
var->children = NULL;
var->format = 0;
@@ -1629,6 +1776,36 @@ 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;