[PATCH 02/11] Generalize varobj iterator
Keith Seitz
keiths@redhat.com
Tue Jan 21 20:44:00 GMT 2014
On 11/23/2013 06:09 PM, Yao Qi wrote:
> This patch generalizes varobj iterator, in a python-independent way.
> Note varobj_item is still a typedef of PyObject, we can only focus on
> API changes, and leave the data type changes to the next patch. As a
> result, we include "varobj-iter.h" after the typedef of PyObject in
> varobj.c, but it is an intermediate state. Finally, varobj-iter.h is
> independent of PyObject.
This looks good to me with the trivial problems mentioned corrected.
> gdb:
>
> 2013-11-24 Pedro Alves <pedro@codesourcery.com>
> Yao Qi <yao@codesourcery.com>
>
> * Makefile.in (SUBDIR_PYTHON_OBS): Add "py-varobj.o".
> (SUBDIR_PYTHON_SRCS): Add "python/py-varobj.c".
> (HFILES_NO_SRCDIR): Add "varobj-iter.h".
> (py-varobj.o): New rule.
> * python/py-varobj.c: New file.
> * python/python-internal.h (py_varobj_get_iterator): Declare.
> * varobj-iter.h: New file.
> * varobj.c: Include "varobj-iter.h"
^
Missing '.'
> (struct varobj) <child_iter>: Change its type from "PyObject *"
> to "struct varobj_iter *".
> <saved_item>: Likewise.
> [HAVE_PYTHON] (varobj_ensure_python_env): Make it extern.
> [HAVE_PYTHON] (varobj_get_iterator): New function.
> (update_dynamic_varobj_children) [HAVE_PYTHON]: Move
> python-specific code to python/py-varobj.c.
> (install_visualizer): Call varobj_iter_delete instead of
> Py_XDECREF.
> * varobj.h (varobj_ensure_python_env): Declare.
> ---
> gdb/Makefile.in | 13 +++-
> gdb/python/py-varobj.c | 183 ++++++++++++++++++++++++++++++++++++++++++
> gdb/python/python-internal.h | 4 +
> gdb/varobj-iter.h | 62 ++++++++++++++
> gdb/varobj.c | 111 ++++++++------------------
> gdb/varobj.h | 2 +
> 6 files changed, 295 insertions(+), 80 deletions(-)
> create mode 100644 gdb/python/py-varobj.c
> create mode 100644 gdb/varobj-iter.h
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 0591279..7d8a365 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -312,7 +312,8 @@ SUBDIR_PYTHON_OBS = \
> py-threadevent.o \
> py-type.o \
> py-utils.o \
> - py-value.o
> + py-value.o \
> + py-varobj.o
>
> SUBDIR_PYTHON_SRCS = \
> python/python.c \
> @@ -348,7 +349,8 @@ SUBDIR_PYTHON_SRCS = \
> python/py-threadevent.c \
> python/py-type.c \
> python/py-utils.c \
> - python/py-value.c
> + python/py-value.c \
> + python/py-varobj.c
> SUBDIR_PYTHON_DEPS =
> SUBDIR_PYTHON_LDFLAGS=
> SUBDIR_PYTHON_CFLAGS=
> @@ -797,7 +799,8 @@ proc-utils.h aarch64-tdep.h arm-tdep.h ax-gdb.h ppcfbsd-tdep.h \
> ppcnbsd-tdep.h cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
> exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h amd64bsd-nat.h \
> i386bsd-nat.h xml-support.h xml-tdesc.h alphabsd-tdep.h gdb_obstack.h \
> -ia64-tdep.h ada-lang.h varobj.h frv-tdep.h nto-tdep.h serial.h \
> +ia64-tdep.h ada-lang.h ada-varobj.h varobj.h varobj-iter.h frv-tdep.h \
> +nto-tdep.h serial.h \
> c-lang.h d-lang.h go-lang.h frame.h event-loop.h block.h cli/cli-setshow.h \
> cli/cli-decode.h cli/cli-cmds.h cli/cli-utils.h \
> cli/cli-script.h macrotab.h symtab.h common/version.h \
> @@ -2285,6 +2288,10 @@ py-value.o: $(srcdir)/python/py-value.c
> $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-value.c
> $(POSTCOMPILE)
>
> +py-varobj.o: $(srcdir)/python/py-varobj.c
> + $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-varobj.c
> + $(POSTCOMPILE)
> +
> #
> # Dependency tracking. Most of this is conditional on GNU Make being
> # found by configure; if GNU Make is not found, we fall back to a
> diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
> new file mode 100644
> index 0000000..6588fc2
> --- /dev/null
> +++ b/gdb/python/py-varobj.c
> @@ -0,0 +1,183 @@
> +/* Copyright (C) 2013 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include "defs.h"
> +#include "python-internal.h"
> +#include "varobj.h"
> +#include "varobj-iter.h"
> +
> +/* A dynamic varobj iterator "class" for python pretty-printed
> + varobjs. This inherits struct varobj_iter. */
> +
> +struct py_varobj_iter
> +{
> + /* The 'base class'. */
> + struct varobj_iter base;
> +
> + /* The python iterator returned by the printer's 'children' method,
> + or NULL if not available. */
> + PyObject *iter;
> +};
> +
> +/* Implementation of the 'dtor' method of pretty-printed varobj
> + iterators. */
> +
> +static void
> +py_varobj_iter_dtor (struct varobj_iter *self)
> +{
> + struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
> +
> + Py_XDECREF (dis->iter);
> +}
> +
> +/* Implementation of the 'next' method of pretty-pretty varobj
^^^^^^^^^^^^^
"pretty-printed" :-)
> + iterators. */
> +
> +static varobj_item *
> +py_varobj_iter_next (struct varobj_iter *self)
> +{
> + struct py_varobj_iter *t = (struct py_varobj_iter *) self;
> + struct cleanup *back_to;
> + PyObject *item;
> +
> + back_to = varobj_ensure_python_env (self->var);
> +
> + item = PyIter_Next (t->iter);
> +
> + if (item == NULL)
> + {
> + /* Normal end of iteration. */
> + if (!PyErr_Occurred ())
> + return NULL;
> +
> + /* If we got a memory error, just use the text as the item. */
> + if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
> + {
> + PyObject *type, *value, *trace;
> + char *name_str, *value_str;
> +
> + PyErr_Fetch (&type, &value, &trace);
> + value_str = gdbpy_exception_to_string (type, value);
> + Py_XDECREF (type);
> + Py_XDECREF (value);
> + Py_XDECREF (trace);
> + if (value_str == NULL)
> + {
> + gdbpy_print_stack ();
> + return NULL;
> + }
> +
> + name_str = xstrprintf ("<error at %d>",
> + self->next_raw_index++);
> + item = Py_BuildValue ("(ss)", name_str, value_str);
> + xfree (name_str);
> + xfree (value_str);
> + if (item == NULL)
> + {
> + gdbpy_print_stack ();
> + return NULL;
> + }
> + }
> + else
> + {
> + /* Any other kind of error. */
> + gdbpy_print_stack ();
> + return NULL;
> + }
> + }
> +
> + self->next_raw_index++;
> + do_cleanups (back_to);
> + return item;
> +}
> +
> +/* The 'vtable' of pretty-printed python varobj iterators. */
> +
> +static const struct varobj_iter_ops py_varobj_iter_ops =
> +{
> + py_varobj_iter_dtor,
> + py_varobj_iter_next
> +};
> +
> +/* Constructor of pretty-printed varobj iterators. VAR is the varobj
> + whose children the iterator will be iterating over. PYITER is the
> + python iterator actually responsible for the iteration. */
> +
> +static void
> +py_varobj_iter_ctor (struct py_varobj_iter *self,
> + struct varobj *var, PyObject *pyiter)
> +{
> + self->base.var = var;
> + self->base.ops = &py_varobj_iter_ops;
> + self->base.next_raw_index = 0;
> + self->iter = pyiter;
> +}
> +
> +/* Allocate and construct a pretty-printed varobj iterator. VAR is
> + the varobj whose children the iterator will be iterating over.
> + PYITER is the python iterator actually responsible for the
> + iteration. */
> +
> +static struct py_varobj_iter *
> +py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
> +{
> + struct py_varobj_iter *self;
> +
> + self = xmalloc (sizeof (struct py_varobj_iter));
You can use "XNEW (struct py_varobj_iter)" as a shorthand for this.
> + py_varobj_iter_ctor (self, var, pyiter);
> + return self;
> +}
> +
> +/* Return a new pretty-printed varobj iterator suitable to iterate
> + over VAR's children. */
> +
> +struct varobj_iter *
> +py_varobj_get_iterator (struct varobj *var, PyObject *printer)
> +{
> + PyObject *children;
> + int i;
> + PyObject *iter;
> + struct py_varobj_iter *py_iter;
> + struct cleanup *back_to = varobj_ensure_python_env (var);
> +
> + if (!PyObject_HasAttr (printer, gdbpy_children_cst))
> + {
> + do_cleanups (back_to);
> + return NULL;
> + }
> +
> + children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
> + NULL);
> + if (children == NULL)
> + {
> + gdbpy_print_stack ();
> + error (_("Null value returned for children"));
> + }
> +
> + make_cleanup_py_decref (children);
> +
> + iter = PyObject_GetIter (children);
> + if (iter == NULL)
> + {
> + gdbpy_print_stack ();
> + error (_("Could not get children iterator"));
> + }
> +
> + py_iter = py_varobj_iter_new (var, iter);
> +
> + do_cleanups (back_to);
> +
> + return &py_iter->base;
> +}
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index 7d8c4ad..93f9c0a 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -494,4 +494,8 @@ int gdb_pymodule_addobject (PyObject *module, const char *name,
> PyObject *object)
> CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
>
> +struct varobj_iter;
> +struct varobj;
> +struct varobj_iter *py_varobj_get_iterator (struct varobj *var,
> + PyObject *printer);
> #endif /* GDB_PYTHON_INTERNAL_H */
> diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
> new file mode 100644
> index 0000000..59be278
> --- /dev/null
> +++ b/gdb/varobj-iter.h
> @@ -0,0 +1,62 @@
> +/* Iterator of varobj.
> + Copyright (C) 2013 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + 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;
> +
> +typedef PyObject varobj_item;
> +
> +/* A dynamic varobj iterator "class". */
> +
> +struct varobj_iter
> +{
> + /* The 'vtable'. */
> + const struct varobj_iter_ops *ops;
> +
> + /* The varobj this iterator is listing children for. */
> + struct varobj *var;
> +
> + /* The next raw index we will try to check is available. If is
"If it is" or "If this is"
> + equal to number_of_children, then we've already iterated the
> + whole set. */
> + int next_raw_index;
> +};
> +
> +/* The vtable of the varobj iterator class. */
Missing newline
> +struct varobj_iter_ops
> +{
> + /* Destructor. Releases everything from SELF (but not SELF
> + itself). */
> + void (*dtor) (struct varobj_iter *self);
> +
> + /* Returns the next object or NULL if it has reached the end. */
> + varobj_item *(*next) (struct varobj_iter *self);
> +};
> +
> +/* Returns the next varobj or NULL if it has reached the end. */
> +
> +#define varobj_iter_next(ITER) (ITER)->ops->next (ITER)
> +
> +/* Delete a varobj_iter object. */
> +
> +#define varobj_iter_delete(ITER) \
> + do \
> + { \
> + if (ITER) \
I believe we are now requested an explicit check against NULL.
> + { \
> + (ITER)->ops->dtor (ITER); \
> + xfree (ITER); \
> + } \
> + } while (0)
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 4a10617..7a8305b 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -41,6 +41,8 @@
> typedef int PyObject;
> #endif
>
> +#include "varobj-iter.h"
> +
> /* Non-zero if we want to see trace of varobj level stuff. */
>
> unsigned int varobjdebug = 0;
> @@ -139,14 +141,14 @@ struct varobj_dynamic
>
> /* The iterator returned by the printer's 'children' method, or NULL
> if not available. */
> - PyObject *child_iter;
> + struct varobj_iter *child_iter;
>
> /* We request one extra item from the iterator, so that we can
> report to the caller whether there are more items than we have
> already reported. However, we don't want to install this value
> when we read it, because that will mess up future updates. So,
> we stash it here instead. */
> - PyObject *saved_item;
> + varobj_item *saved_item;
> };
>
> struct cpstack
> @@ -255,7 +257,7 @@ is_root_p (struct varobj *var)
> #ifdef HAVE_PYTHON
> /* Helper function to install a Python environment suitable for
> use during operations on VAR. */
> -static struct cleanup *
> +struct cleanup *
> varobj_ensure_python_env (struct varobj *var)
> {
> return ensure_python_env (var->root->exp->gdbarch,
> @@ -772,6 +774,19 @@ dynamic_varobj_has_child_method (struct varobj *var)
> return result;
> }
>
> +/* Dynamic varobj's iterator factory. Returns an iterator object
> + suitable for iterating over VAR's children. */
This first bit is a tiny bit awkward. How about "A factory for creating
dynamic varobj iterators." ?
> +
> +static struct varobj_iter *
> +varobj_get_iterator (struct varobj *var)
> +{
> + if (var->dynamic->pretty_printer)
> + return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
> +
> + gdb_assert_not_reached (_("\
> +requested an interator from a non-dynamic varobj"));
^^^^^^^^^
"iterator"
> +}
> +
> #endif
>
> static int
> @@ -787,9 +802,7 @@ update_dynamic_varobj_children (struct varobj *var,
> {
> #if HAVE_PYTHON
> struct cleanup *back_to;
> - PyObject *children;
> int i;
> - PyObject *printer = var->dynamic->pretty_printer;
>
> if (!gdb_python_initialized)
> return 0;
> @@ -797,37 +810,22 @@ update_dynamic_varobj_children (struct varobj *var,
> back_to = varobj_ensure_python_env (var);
>
> *cchanged = 0;
> - if (!PyObject_HasAttr (printer, gdbpy_children_cst))
> - {
> - do_cleanups (back_to);
> - return 0;
> - }
>
> if (update_children || var->dynamic->child_iter == NULL)
> {
> - children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
> - NULL);
> + varobj_iter_delete (var->dynamic->child_iter);
> + var->dynamic->child_iter = varobj_get_iterator (var);
>
> - if (!children)
> - {
> - gdbpy_print_stack ();
> - error (_("Null value returned for children"));
> - }
> + Py_XDECREF (var->dynamic->saved_item);
> + var->dynamic->saved_item = NULL;
>
> - make_cleanup_py_decref (children);
> + i = 0;
>
> - Py_XDECREF (var->dynamic->child_iter);
> - var->dynamic->child_iter = PyObject_GetIter (children);
> if (var->dynamic->child_iter == NULL)
> {
> - gdbpy_print_stack ();
> - error (_("Could not get children iterator"));
> + do_cleanups (back_to);
> + return 0;
> }
> -
> - Py_XDECREF (var->dynamic->saved_item);
> - var->dynamic->saved_item = NULL;
> -
> - i = 0;
> }
> else
> i = VEC_length (varobj_p, var->children);
> @@ -837,7 +835,6 @@ update_dynamic_varobj_children (struct varobj *var,
> for (; to < 0 || i < to + 1; ++i)
> {
> PyObject *item;
> - int force_done = 0;
>
> /* See if there was a leftover from last time. */
> if (var->dynamic->saved_item)
> @@ -846,52 +843,17 @@ update_dynamic_varobj_children (struct varobj *var,
> var->dynamic->saved_item = NULL;
> }
> else
> - item = PyIter_Next (var->dynamic->child_iter);
> -
> - if (!item)
> {
> - /* Normal end of iteration. */
> - if (!PyErr_Occurred ())
> - break;
> -
> - /* If we got a memory error, just use the text as the
> - item. */
> - if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
> - {
> - PyObject *type, *value, *trace;
> - char *name_str, *value_str;
> -
> - PyErr_Fetch (&type, &value, &trace);
> - value_str = gdbpy_exception_to_string (type, value);
> - Py_XDECREF (type);
> - Py_XDECREF (value);
> - Py_XDECREF (trace);
> - if (!value_str)
> - {
> - gdbpy_print_stack ();
> - break;
> - }
> -
> - name_str = xstrprintf ("<error at %d>", i);
> - item = Py_BuildValue ("(ss)", name_str, value_str);
> - xfree (name_str);
> - xfree (value_str);
> - if (!item)
> - {
> - gdbpy_print_stack ();
> - break;
> - }
> -
> - force_done = 1;
> - }
> - else
> - {
> - /* Any other kind of error. */
> - gdbpy_print_stack ();
> - break;
> - }
> + item = varobj_iter_next (var->dynamic->child_iter);
> }
>
> + if (item == NULL)
> + {
> + /* Iteration is done. Remove iterator from VAR. */
> + varobj_iter_delete (var->dynamic->child_iter);
> + var->dynamic->child_iter = NULL;
> + break;
> + }
> /* We don't want to push the extra child on any report list. */
> if (to < 0 || i < to)
> {
> @@ -931,9 +893,6 @@ update_dynamic_varobj_children (struct varobj *var,
> element. */
> break;
> }
> -
> - if (force_done)
> - break;
> }
>
> if (i < VEC_length (varobj_p, var->children))
> @@ -952,8 +911,6 @@ update_dynamic_varobj_children (struct varobj *var,
> *cchanged = 1;
>
> var->num_children = VEC_length (varobj_p, var->children);
> -
> - do_cleanups (back_to);
Doesn't this cleanup still need to run?
>
> return 1;
> #else
> @@ -1244,7 +1201,7 @@ install_visualizer (struct varobj_dynamic *var, PyObject *constructor,
> Py_XDECREF (var->pretty_printer);
> var->pretty_printer = visualizer;
>
> - Py_XDECREF (var->child_iter);
> + varobj_iter_delete (var->child_iter);
> var->child_iter = NULL;
> }
>
> diff --git a/gdb/varobj.h b/gdb/varobj.h
> index 978d9b9..0f68311 100644
> --- a/gdb/varobj.h
> +++ b/gdb/varobj.h
> @@ -308,6 +308,8 @@ extern int varobj_has_more (struct varobj *var, int to);
>
> extern int varobj_pretty_printed_p (struct varobj *var);
>
> +extern struct cleanup *varobj_ensure_python_env (struct varobj *var);
> +
IIRC, we are now adding documentation near declarations, too. Many are
often as simple as "See description in varobj.c."
> extern int varobj_default_value_is_changeable_p (struct varobj *var);
> extern int varobj_value_is_changeable_p (struct varobj *var);
>
>
More information about the Gdb-patches
mailing list