This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 01/11] Use 'struct varobj_item' to represent name and value pair


On 11/23/2013 06:09 PM, Yao Qi wrote:
name and value pair is widely used in varobj.c.  This patch is to add
a new struct varobj_item to represent them, so that the number of
function arguments can be reduced.  Finally, the iteration is done on
'struct varobj_item' instead of PyObject after this patch series.

I think this approach works. I agree: while we can use the existing dynamic varobj infrastructure internally, we may not want to actually expose that nomenclature to the user (in the documentation).

As I mentioned in a previous mail, if I could request one "big" change, it would be for the --available-only to be persistent (and alterable with a new command), e.g., -var-create ... --available-only. But I am not going to suggest that is required without the agreement of a maintainer.

I will proceed with review as the patches are written. You've done an excellent job of splitting these patches into manageable chunks. Thank you so much!

For this first patch (like most of the patches), I really only have minor nits.

Keith

2013-11-24  Yao Qi  <yao@codesourcery.com>

	* varobj.c (struct varobj_item): New structure.
	(create_child_with_value): Update declaration.
	(varobj_add_child): Replace arguments 'name' and 'value' with
	'item'.  Callers update.
                         ^^^^^^
Either "Callers updated." or "Update callers."

	(install_dynamic_child): Likewise.
	(update_dynamic_varobj_children): Likewise.
	(varobj_add_child): Likewise.
	(create_child_with_value): Likewise.
---
  gdb/varobj.c |   64 +++++++++++++++++++++++++++++++++++----------------------
  1 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/gdb/varobj.c b/gdb/varobj.c
index b78969a..4a10617 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -108,6 +108,16 @@ struct varobj_root
    struct varobj_root *next;
  };

+/* A node or item of varobj, composed by the name and the value.  */
                                ^^^^^^^^^^^
"composed of"

+
+struct varobj_item
+{
+  /* Name of this item.  */
+  char *name;

Add a newline here.

+  /* Value of this item.  */
+  struct value *value;
+};
+
  /* Dynamic part of varobj.  */

  struct varobj_dynamic
@@ -169,8 +179,8 @@ static void uninstall_variable (struct varobj *);
  static struct varobj *create_child (struct varobj *, int, char *);

  static struct varobj *
-create_child_with_value (struct varobj *parent, int index, char *name,
-			 struct value *value);
+create_child_with_value (struct varobj *parent, int index,
+			 struct varobj_item *item);

  /* Utility routines */

@@ -214,8 +224,7 @@ static int is_root_p (struct varobj *var);
  #if HAVE_PYTHON

  static struct varobj *varobj_add_child (struct varobj *var,
-					char *name,
-					struct value *value);
+					struct varobj_item *item);

  #endif /* HAVE_PYTHON */

@@ -714,13 +723,12 @@ install_dynamic_child (struct varobj *var,
  		       VEC (varobj_p) **unchanged,
  		       int *cchanged,
  		       int index,
-		       char *name,
-		       struct value *value)
+		       struct varobj_item *item)
  {
    if (VEC_length (varobj_p, var->children) < index + 1)
      {
        /* There's no child yet.  */
-      struct varobj *child = varobj_add_child (var, name, value);
+      struct varobj *child = varobj_add_child (var, item);

        if (new)
  	{
@@ -731,14 +739,14 @@ install_dynamic_child (struct varobj *var,
    else
      {
        varobj_p existing = VEC_index (varobj_p, var->children, index);
-      int type_updated = update_type_if_necessary (existing, value);
+      int type_updated = update_type_if_necessary (existing, item->value);

        if (type_updated)
  	{
  	  if (type_changed)
  	    VEC_safe_push (varobj_p, *type_changed, existing);
  	}
-      if (install_new_value (existing, value, 0))
+      if (install_new_value (existing, item->value, 0))
  	{
  	  if (!type_updated && changed)
  	    VEC_safe_push (varobj_p, *changed, existing);
@@ -889,7 +897,7 @@ update_dynamic_varobj_children (struct varobj *var,
  	{
  	  PyObject *py_v;
  	  const char *name;
-	  struct value *v;
+	  struct varobj_item varobj_item;
  	  struct cleanup *inner;
  	  int can_mention = from < 0 || i >= from;

@@ -901,15 +909,17 @@ update_dynamic_varobj_children (struct varobj *var,
  	      error (_("Invalid item from the child list"));
  	    }

-	  v = convert_value_from_python (py_v);
-	  if (v == NULL)
+	  varobj_item.value = convert_value_from_python (py_v);
+	  if (varobj_item.value == NULL)
  	    gdbpy_print_stack ();
+	  varobj_item.name = xstrdup (name);
+
  	  install_dynamic_child (var, can_mention ? changed : NULL,
  				 can_mention ? type_changed : NULL,
  				 can_mention ? new : NULL,
  				 can_mention ? unchanged : NULL,
  				 can_mention ? cchanged : NULL, i,
-				 xstrdup (name), v);
+				 &varobj_item);
  	  do_cleanups (inner);
  	}
        else
@@ -1028,11 +1038,11 @@ varobj_list_children (struct varobj *var, int *from, int *to)
  #if HAVE_PYTHON

  static struct varobj *
-varobj_add_child (struct varobj *var, char *name, struct value *value)
+varobj_add_child (struct varobj *var, struct varobj_item *item)
  {
-  varobj_p v = create_child_with_value (var,
+  varobj_p v = create_child_with_value (var,
  					VEC_length (varobj_p, var->children),
-					name, value);
+					item);

    VEC_safe_push (varobj_p, var->children, v);
    return v;
@@ -2098,13 +2108,17 @@ uninstall_variable (struct varobj *var)
  static struct varobj *
  create_child (struct varobj *parent, int index, char *name)
  {
-  return create_child_with_value (parent, index, name,
-				  value_of_child (parent, index));
+  struct varobj_item item;
+
+  item.name = name;
+  item.value = value_of_child (parent, index);
+
+  return create_child_with_value (parent, index, &item);
  }

  static struct varobj *
-create_child_with_value (struct varobj *parent, int index, char *name,
-			 struct value *value)
+create_child_with_value (struct varobj *parent, int index,
+			 struct varobj_item *item)
  {
    struct varobj *child;
    char *childs_name;
@@ -2112,7 +2126,7 @@ create_child_with_value (struct varobj *parent, int index, char *name,
    child = new_variable ();

    /* NAME is allocated by caller.  */
-  child->name = name;
+  child->name = item->name;
    child->index = index;
    child->parent = parent;
    child->root = parent->root;
@@ -2120,22 +2134,22 @@ create_child_with_value (struct varobj *parent, int index, char *name,
    if (varobj_is_anonymous_child (child))
      childs_name = xstrprintf ("%s.%d_anonymous", parent->obj_name, index);
    else
-    childs_name = xstrprintf ("%s.%s", parent->obj_name, name);
+    childs_name = xstrprintf ("%s.%s", parent->obj_name, item->name);
    child->obj_name = childs_name;

    install_variable (child);

    /* Compute the type of the child.  Must do this before
       calling install_new_value.  */
-  if (value != NULL)
+  if (item->value != NULL)
      /* If the child had no evaluation errors, var->value
         will be non-NULL and contain a valid type.  */
-    child->type = value_actual_type (value, 0, NULL);
+    child->type = value_actual_type (item->value, 0, NULL);
    else
      /* Otherwise, we must compute the type.  */
      child->type = (*child->root->lang_ops->type_of_child) (child->parent,
  							   child->index);
-  install_new_value (child, value, 1);
+  install_new_value (child, item->value, 1);

    return child;
  }



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]