Re: [PATCH 02/12] Generalize varobj iterator

[Tom Tromey <tromey@redhat.com>]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2014-02-14  Pedro Alves  <pedro@codesourcery.com>
Yao> 	    Yao Qi  <yao@codesourcery.com>
Yao> 	* Makefile.in (SUBDIR_PYTHON_OBS): Add "py-varobj.o".
Yao> 	(SUBDIR_PYTHON_SRCS): Add "python/py-varobj.c".
Yao> 	(HFILES_NO_SRCDIR): Add "varobj-iter.h".
Yao> 	(py-varobj.o): New rule.
Yao> 	* python/py-varobj.c: New file.
Yao> 	* python/python-internal.h (py_varobj_get_iterator): Declare.
Yao> 	* varobj-iter.h: New file.
Yao> 	* varobj.c: Include "varobj-iter.h"
Yao> 	(struct varobj) <child_iter>: Change its type from "PyObject *"
Yao> 	to "struct varobj_iter *".
Yao> 	<saved_item>: Likewise.
Yao> 	[HAVE_PYTHON] (varobj_ensure_python_env): Make it extern.
Yao> 	[HAVE_PYTHON] (varobj_get_iterator): New function.
Yao> 	(update_dynamic_varobj_children) [HAVE_PYTHON]: Move
Yao> 	python-specific code to python/py-varobj.c.
Yao> 	(install_visualizer): Call varobj_iter_delete instead of
Yao> 	Py_XDECREF.
Yao> 	* varobj.h (varobj_ensure_python_env): Declare.

Yao> +static void
Yao> +py_varobj_iter_dtor (struct varobj_iter *self)
Yao> +{
Yao> +  struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
Yao> +
Yao> +  Py_XDECREF (dis->iter);

I think this has to acquire the GIL before calling Py_XDECREF.

Yao> +static void
Yao> +py_varobj_iter_ctor (struct py_varobj_iter *self,
Yao> +		      struct varobj *var, PyObject *pyiter)
Yao> +{
Yao> +  self->base.var = var;
Yao> +  self->base.ops = &py_varobj_iter_ops;
Yao> +  self->base.next_raw_index = 0;
Yao> +  self->iter = pyiter;
[...]
Yao> +static struct py_varobj_iter *
Yao> +py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
Yao> +{
[...]
Yao> +  py_varobj_iter_ctor (self, var, pyiter);

I think these two functions should be documented as stealing a reference
to PYITER.  It's helpful to see such comments later.

Using CPYCHECKER_STEALS_REFERENCE_TO_ARG would also be nice, but I
appreciate that this isn't easy for everybody to test.

Yao> +  iter = PyObject_GetIter (children);
Yao> + if (iter == NULL)
Yao> +    {

The "if" looks misindented.

Yao> +struct varobj_iter;
Yao> +struct varobj;
Yao> +struct varobj_iter *py_varobj_get_iterator (struct varobj *var,
Yao> +					    PyObject *printer);
Yao>  #endif /* GDB_PYTHON_INTERNAL_H */

Please leave a blank line before the #endif.

Yao> +extern  struct cleanup *varobj_ensure_python_env (struct varobj *var);

Extra space after "extern".

Tom