From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14614 invoked by alias); 21 May 2014 17:51:18 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 14530 invoked by uid 89); 21 May 2014 17:51:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 21 May 2014 17:51:16 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4LHpDoe021042 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 21 May 2014 13:51:13 -0400 Received: from barimba (ovpn-113-182.phx2.redhat.com [10.3.113.182]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s4LHpCOw013112 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Wed, 21 May 2014 13:51:12 -0400 From: Tom Tromey To: Yao Qi Cc: Subject: Re: [PATCH 02/12] Generalize varobj iterator References: <1392367471-13527-1-git-send-email-yao@codesourcery.com> <1392367471-13527-3-git-send-email-yao@codesourcery.com> Date: Wed, 21 May 2014 17:51:00 -0000 In-Reply-To: <1392367471-13527-3-git-send-email-yao@codesourcery.com> (Yao Qi's message of "Fri, 14 Feb 2014 16:44:21 +0800") Message-ID: <874n0jkokw.fsf@fleche.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2014-05/txt/msg00493.txt.bz2 >>>>> "Yao" == Yao Qi writes: Yao> 2014-02-14 Pedro Alves Yao> Yao Qi 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) : Change its type from "PyObject *" Yao> to "struct varobj_iter *". Yao> : 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