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 03/12] Iterate over 'struct varobj_item' instead of PyObject


On 05/22/2014 02:01 AM, Tom Tromey wrote:
> Yao> -  if (!gdb_python_initialized)
> Yao> -    return 0;
> 
> Where is this check done now?  I couldn't find the location.
> 

Good catch.

> IIRC this check was added in response to some bug report; I think it
> only arises if Python can't be properly initialized somehow.
> 
> If it was dropped I think it may need to be reinstated somewhere.

It should be moved to 'next' method of python varobj iterator,
because update_dynamic_varobj_children is independent of python,
and all python related code should be moved to python varobj
iterator.

At the beginning of update_dynamic_varobj_children,

  if (update_children || var->dynamic->child_iter == NULL)
    {
      varobj_iter_delete (var->dynamic->child_iter);
      var->dynamic->child_iter = varobj_get_iterator (var,
						      var->root->lang_ops);

      varobj_clear_saved_item (var->dynamic);

      i = 0;

      if (var->dynamic->child_iter == NULL)
	return 0;
    }
  else
    i = VEC_length (varobj_p, var->children);

if gdb_python_initialized is false, varobj_get_iterator returns
NULL and update_dynamic_varobj_children returns zero too.  The
behaviour is unchanged.

-- 
Yao (éå)

In previous patch, "saved_item" is still a PyOjbect and iteration is
still performed over PyObject.  This patch continues to decouple
iteration from python code, so it changes its type to "struct
varobj_item *", so that the iterator itself is independent of python.

 V2:
 - Call varobj_delete_iter in free_variable.
 - Fix changelog entries.
 - Use XNEW.

 V3:
 - Return NULL early in py_varobj_iter_next if gdb_python_initialized
   is false.

gdb:

2014-05-22  Pedro Alves  <pedro@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* python/py-varobj.c (py_varobj_iter_next): Return NULL if
	gdb_python_initialized is false.  Move some code from varobj.c.
	* varobj-iter.h (struct varobj_item): Moved from varobj.c.
	* varobj.c: Move "varobj-iter.h" inclusion earlier.
	(struct varobj_item): Moved to varobj-iter.h".
	(varobj_clear_saved_item): New function.
	(update_dynamic_varobj_children): Move python-related code to
	py-varobj.c.
	(free_variable): Call varobj_clear_saved_item and
	varobj_iter_delete.

---
 gdb/python/py-varobj.c | 20 ++++++++++++-
 gdb/varobj-iter.h      | 13 +++++++--
 gdb/varobj.c           | 76 +++++++++++++++++---------------------------------
 3 files changed, 56 insertions(+), 53 deletions(-)

diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index 50bea40..40b28f4 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -53,6 +53,12 @@ py_varobj_iter_next (struct varobj_iter *self)
   struct py_varobj_iter *t = (struct py_varobj_iter *) self;
   struct cleanup *back_to;
   PyObject *item;
+  PyObject *py_v;
+  varobj_item *vitem;
+  const char *name = NULL;
+
+  if (!gdb_python_initialized)
+    return NULL;
 
   back_to = varobj_ensure_python_env (self->var);
 
@@ -100,9 +106,21 @@ py_varobj_iter_next (struct varobj_iter *self)
 	}
     }
 
+  if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
+    {
+      gdbpy_print_stack ();
+      error (_("Invalid item from the child list"));
+    }
+
+  vitem = xmalloc (sizeof *vitem);
+  vitem->value = convert_value_from_python (py_v);
+  if (vitem->value == NULL)
+    gdbpy_print_stack ();
+  vitem->name = xstrdup (name);
+
   self->next_raw_index++;
   do_cleanups (back_to);
-  return item;
+  return vitem;
 }
 
 /* The 'vtable' of pretty-printed python varobj iterators.  */
diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
index 3a530bc..9eb672d 100644
--- a/gdb/varobj-iter.h
+++ b/gdb/varobj-iter.h
@@ -14,9 +14,18 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-struct varobj_iter_ops;
+/* A node or item of varobj, composed of the name and the value.  */
+
+typedef struct varobj_item
+{
+  /* Name of this item.  */
+  char *name;
 
-typedef PyObject varobj_item;
+  /* Value of this item.  */
+  struct value *value;
+} varobj_item;
+
+struct varobj_iter_ops;
 
 /* A dynamic varobj iterator "class".  */
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 02cce5a..7e8b364 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -33,6 +33,7 @@
 #include "vec.h"
 #include "gdbthread.h"
 #include "inferior.h"
+#include "varobj-iter.h"
 
 #if HAVE_PYTHON
 #include "python/python.h"
@@ -41,8 +42,6 @@
 typedef int PyObject;
 #endif
 
-#include "varobj-iter.h"
-
 /* Non-zero if we want to see trace of varobj level stuff.  */
 
 unsigned int varobjdebug = 0;
@@ -110,17 +109,6 @@ struct varobj_root
   struct varobj_root *next;
 };
 
-/* A node or item of varobj, composed of the name and the value.  */
-
-struct varobj_item
-{
-  /* Name of this item.  */
-  char *name;
-
-  /* Value of this item.  */
-  struct value *value;
-};
-
 /* Dynamic part of varobj.  */
 
 struct varobj_dynamic
@@ -788,6 +776,18 @@ varobj_get_iterator (struct varobj *var)
 requested an iterator from a non-dynamic varobj"));
 }
 
+/* Release and clear VAR's saved item, if any.  */
+
+static void
+varobj_clear_saved_item (struct varobj_dynamic *var)
+{
+  if (var->saved_item != NULL)
+    {
+      value_free (var->saved_item->value);
+      xfree (var->saved_item);
+      var->saved_item = NULL;
+    }
+}
 #endif
 
 static int
@@ -802,14 +802,8 @@ update_dynamic_varobj_children (struct varobj *var,
 				int to)
 {
 #if HAVE_PYTHON
-  struct cleanup *back_to;
   int i;
 
-  if (!gdb_python_initialized)
-    return 0;
-
-  back_to = varobj_ensure_python_env (var);
-
   *cchanged = 0;
 
   if (update_children || var->dynamic->child_iter == NULL)
@@ -817,16 +811,12 @@ update_dynamic_varobj_children (struct varobj *var,
       varobj_iter_delete (var->dynamic->child_iter);
       var->dynamic->child_iter = varobj_get_iterator (var);
 
-      Py_XDECREF (var->dynamic->saved_item);
-      var->dynamic->saved_item = NULL;
+      varobj_clear_saved_item (var->dynamic);
 
       i = 0;
 
       if (var->dynamic->child_iter == NULL)
-	{
-	  do_cleanups (back_to);
-	  return 0;
-	}
+	return 0;
     }
   else
     i = VEC_length (varobj_p, var->children);
@@ -835,10 +825,10 @@ update_dynamic_varobj_children (struct varobj *var,
      are more children.  */
   for (; to < 0 || i < to + 1; ++i)
     {
-      PyObject *item;
+      varobj_item *item;
 
       /* See if there was a leftover from last time.  */
-      if (var->dynamic->saved_item)
+      if (var->dynamic->saved_item != NULL)
 	{
 	  item = var->dynamic->saved_item;
 	  var->dynamic->saved_item = NULL;
@@ -846,6 +836,10 @@ update_dynamic_varobj_children (struct varobj *var,
       else
 	{
 	  item = varobj_iter_next (var->dynamic->child_iter);
+	  /* Release vitem->value so its lifetime is not bound to the
+	     execution of a command.  */
+	  if (item != NULL && item->value != NULL)
+	    release_value_or_incref (item->value);
 	}
 
       if (item == NULL)
@@ -858,36 +852,19 @@ update_dynamic_varobj_children (struct varobj *var,
       /* We don't want to push the extra child on any report list.  */
       if (to < 0 || i < to)
 	{
-	  PyObject *py_v;
-	  const char *name;
-	  struct varobj_item varobj_item;
-	  struct cleanup *inner;
 	  int can_mention = from < 0 || i >= from;
 
-	  inner = make_cleanup_py_decref (item);
-
-	  if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
-	    {
-	      gdbpy_print_stack ();
-	      error (_("Invalid item from the child list"));
-	    }
-
-	  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,
-				 &varobj_item);
-	  do_cleanups (inner);
+				 item);
+
+	  xfree (item);
 	}
       else
 	{
-	  Py_XDECREF (var->dynamic->saved_item);
 	  var->dynamic->saved_item = item;
 
 	  /* We want to truncate the child list just before this
@@ -913,7 +890,6 @@ update_dynamic_varobj_children (struct varobj *var,
 
   var->num_children = VEC_length (varobj_p, var->children);
 
-  do_cleanups (back_to);
   return 1;
 #else
   gdb_assert_not_reached ("should never be called if Python is not enabled");
@@ -2191,12 +2167,12 @@ free_variable (struct varobj *var)
 
       Py_XDECREF (var->dynamic->constructor);
       Py_XDECREF (var->dynamic->pretty_printer);
-      Py_XDECREF (var->dynamic->child_iter);
-      Py_XDECREF (var->dynamic->saved_item);
       do_cleanups (cleanup);
     }
 #endif
 
+  varobj_iter_delete (var->dynamic->child_iter);
+  varobj_clear_saved_item (var->dynamic);
   value_free (var->value);
 
   /* Free the expression if this is a root variable.  */
-- 
1.9.0


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