From patchwork Mon Jun 2 04:20:41 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 1229 Received: (qmail 14702 invoked by alias); 2 Jun 2014 04:21:04 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 14682 invoked by uid 89); 2 Jun 2014 04:21:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pb0-f48.google.com Received: from mail-pb0-f48.google.com (HELO mail-pb0-f48.google.com) (209.85.160.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 02 Jun 2014 04:21:00 +0000 Received: by mail-pb0-f48.google.com with SMTP id rr13so3782909pbb.21 for ; Sun, 01 Jun 2014 21:20:59 -0700 (PDT) X-Received: by 10.66.97.67 with SMTP id dy3mr24584163pab.24.1401682859100; Sun, 01 Jun 2014 21:20:59 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-50-sfba.hfc.comcastbusiness.net. [173.13.178.50]) by mx.google.com with ESMTPSA id nf5sm17934883pbc.77.2014.06.01.21.20.58 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 01 Jun 2014 21:20:58 -0700 (PDT) From: Doug Evans To: Siva Chandra Cc: gdb-patches Subject: Re: [Patch v19 4/4] Add xmethod support to the Python API References: Date: Sun, 01 Jun 2014 21:20:41 -0700 In-Reply-To: (Siva Chandra's message of "Fri, 30 May 2014 16:04:49 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Siva Chandra writes: > The attached patch addresses Doug's comments on v18. Well, almost all of them ... > ChangeLog > > 2014-05-30 Siva Chandra Reddy > > * python/py-xmethods.c: New file. > * python/py-objfile.c (objfile_object): New field 'xmethods'. > (objfpy_dealloc): XDECREF on the new xmethods field. > (objfpy_new, objfile_to_objfile_object): Initialize xmethods > field. > (objfpy_get_xmethods): New function. > (objfile_getset): New entry 'xmethods'. > * python/py-progspace.c (pspace_object): New field 'xmethods'. > (pspy_dealloc): XDECREF on the new xmethods field. > (pspy_new, pspace_to_pspace_object): Initialize xmethods > field. > (pspy_get_xmethods): New function. > (pspace_getset): New entry 'xmethods'. > * python/python-internal.h: Add declarations for new functions. > * python/python.c (_initialize_python): Invoke > gdbpy_initialize_xmethods. > * python/lib/gdb/__init__.py (xmethods): New > attribute. > * python/lib/gdb/xmethod.py: New file. > * python/lib/gdb/command/xmethods.py: New file. > > testuite/ > * gdb.python/py-xmethods.cc: New testcase to test xmethods. > * gdb.python/py-xmethods.exp: New tests to test xmethods. > * gdb.python/py-xmethods.py: Python script supporting the > new testcase and tests. > > diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c > new file mode 100644 > index 0000000..a4d6d91 > --- /dev/null > +++ b/gdb/python/py-xmethods.c > +/* Implementation of free_xmethod_worker_data for Python. */ > + > +void > +gdbpy_free_xmethod_worker_data (const struct extension_language_defn *extlang, > + void *data) > +{ > + struct gdbpy_worker_data *worker_data = data; > + > + gdb_assert (worker_data->worker != NULL && worker_data->this_type != NULL); > + > + Py_DECREF (worker_data->worker); > + Py_DECREF (worker_data->this_type); > + xfree (worker_data); > +} > + > +/* Implementation of clone_xmethod_worker_data for Python. */ > + > +void * > +gdbpy_clone_xmethod_worker_data (const struct extension_language_defn *extlang, > + void *data) > +{ > + struct gdbpy_worker_data *worker_data = data, *new_data; > + > + gdb_assert (worker_data->worker != NULL && worker_data->this_type != NULL); > + > + new_data = XCNEW (struct gdbpy_worker_data); > + new_data->worker = worker_data->worker; > + new_data->this_type = worker_data->this_type; > + Py_INCREF (new_data->worker); > + Py_INCREF (new_data->this_type); > + > + return new_data; > +} I ran the testsuite and gdb was crashing. These two requested changes were missed. Also, this request was missed too. @@ -480,6 +492,7 @@ gdbpy_get_xmethod_arg_types (const struct extension_language_defn *extlang, /* Add the type of 'this' as the first argument. */ obj_type = type_object_to_type (worker_data->this_type); + /* FIXME: Add comment explaining why passing 1 for "is const" is ok here. */ type_array[0] = make_cv_type (1, 0, lookup_pointer_type (obj_type), NULL); *nargs = i; *arg_types = type_array; As a reader I look at the call to make_cv_type and ask myself "why is that always necessarily ok?" A comment explaining why would prevent me, the reader, from having to put in time digging deeper. Ok with those changes. diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c index a4d6d91..44bb460 100644 --- a/gdb/python/py-xmethods.c +++ b/gdb/python/py-xmethods.c @@ -53,12 +53,18 @@ gdbpy_free_xmethod_worker_data (const struct extension_language_defn *extlang, void *data) { struct gdbpy_worker_data *worker_data = data; + struct cleanup *cleanups; gdb_assert (worker_data->worker != NULL && worker_data->this_type != NULL); + /* We don't do much here, but we still need the GIL. */ + cleanups = ensure_python_env (get_current_arch (), current_language); + Py_DECREF (worker_data->worker); Py_DECREF (worker_data->this_type); xfree (worker_data); + + do_cleanups (cleanups); } /* Implementation of clone_xmethod_worker_data for Python. */ @@ -68,15 +74,21 @@ gdbpy_clone_xmethod_worker_data (const struct extension_language_defn *extlang, void *data) { struct gdbpy_worker_data *worker_data = data, *new_data; + struct cleanup *cleanups; gdb_assert (worker_data->worker != NULL && worker_data->this_type != NULL); + /* We don't do much here, but we still need the GIL. */ + cleanups = ensure_python_env (get_current_arch (), current_language); + new_data = XCNEW (struct gdbpy_worker_data); new_data->worker = worker_data->worker; new_data->this_type = worker_data->this_type; Py_INCREF (new_data->worker); Py_INCREF (new_data->this_type); + do_cleanups (cleanups); + return new_data; }