From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13428 invoked by alias); 21 Nov 2016 22:46:55 -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 13284 invoked by uid 89); 21 Nov 2016 22:46:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Calling, invocation, our X-HELO: gproxy6-pub.mail.unifiedlayer.com Received: from gproxy6-pub.mail.unifiedlayer.com (HELO gproxy6-pub.mail.unifiedlayer.com) (67.222.39.168) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with SMTP; Mon, 21 Nov 2016 22:46:44 +0000 Received: (qmail 2634 invoked by uid 0); 21 Nov 2016 22:46:42 -0000 Received: from unknown (HELO cmgw2) (10.0.90.83) by gproxy6.mail.unifiedlayer.com with SMTP; 21 Nov 2016 22:46:42 -0000 Received: from box522.bluehost.com ([74.220.219.122]) by cmgw2 with id Ammd1u00d2f2jeq01mmgnX; Mon, 21 Nov 2016 15:46:41 -0700 X-Authority-Analysis: v=2.1 cv=YNIMl32x c=1 sm=1 tr=0 a=GsOEXm/OWkKvwdLVJsfwcA==:117 a=GsOEXm/OWkKvwdLVJsfwcA==:17 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=L24OOQBejmoA:10 a=zstS-IiYAAAA:8 a=dmZDO-HLVF1i167SSi4A:9 a=4G6NA9xxw8l3yy4pmD5M:22 Received: from 174-16-143-211.hlrn.qwest.net ([174.16.143.211]:37202 helo=bapiya) by box522.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.86_1) (envelope-from ) id 1c8xMF-0002sz-28; Mon, 21 Nov 2016 15:46:39 -0700 From: Tom Tromey To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [RFA 03/13] Use gdbpy_ref in py-cmd.c References: <1479674496-14000-1-git-send-email-tom@tromey.com> <1479674496-14000-4-git-send-email-tom@tromey.com> Date: Mon, 21 Nov 2016 22:46:00 -0000 In-Reply-To: <1479674496-14000-4-git-send-email-tom@tromey.com> (Tom Tromey's message of "Sun, 20 Nov 2016 13:41:26 -0700") Message-ID: <87k2bwxxh0.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-BWhitelist: no X-Exim-ID: 1c8xMF-0002sz-28 X-Source-Sender: 174-16-143-211.hlrn.qwest.net (bapiya) [174.16.143.211]:37202 X-Source-Auth: tom+tromey.com X-Email-Count: 2 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTIyLmJsdWVob3N0LmNvbQ== X-SW-Source: 2016-11/txt/msg00620.txt.bz2 >>>>> "Tom" == Tom Tromey writes: Tom> This changes py-cmd.c to use gdbpy_ref in more places. This also Tom> fixes a latent memory leak in cmdpy_completer_helper, which Tom> unnecessarily increfs the result of PyObject_CallMethodObjArgs. This Tom> is not needed because that function returns a new reference. One of the buildbot compilers noticed that this patch was incomplete -- it was still passing a gdbpy_ref to a varargs function. I assume this wasn't caught in local testing because (1) my version of gcc doesn't warn, and (2) the object is effectively passed as a pointer, making it "work". I've appended the corrected patch. Tom diff --git a/gdb/ChangeLog b/gdb/ChangeLog index ebb4d71..c4a1f71 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,12 @@ 2016-11-20 Tom Tromey + * python/py-cmd.c (cmdpy_completer_helper): Use gdbpy_ref. Remove + extra incref. + (cmdpy_completer_handle_brkchars, cmdpy_completer, cmdpy_init): + Use gdbpy_ref. + +2016-11-20 Tom Tromey + * python/py-breakpoint.c (gdbpy_breakpoint_cond_says_stop): Use gdbpy_ref. diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c index 17daaee..202c60d 100644 --- a/gdb/python/py-cmd.c +++ b/gdb/python/py-cmd.c @@ -232,8 +232,6 @@ cmdpy_completer_helper (struct cmd_list_element *command, const char *text, const char *word) { cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command); - PyObject *textobj, *wordobj; - PyObject *resultobj; if (obj == NULL) error (_("Invalid invocation of Python command object.")); @@ -243,29 +241,26 @@ cmdpy_completer_helper (struct cmd_list_element *command, return NULL; } - textobj = PyUnicode_Decode (text, strlen (text), host_charset (), NULL); + gdbpy_ref textobj (PyUnicode_Decode (text, strlen (text), host_charset (), + NULL)); if (textobj == NULL) error (_("Could not convert argument to Python string.")); - wordobj = PyUnicode_Decode (word, strlen (word), host_charset (), NULL); + gdbpy_ref wordobj (PyUnicode_Decode (word, strlen (word), host_charset (), + NULL)); if (wordobj == NULL) - { - Py_DECREF (textobj); - error (_("Could not convert argument to Python string.")); - } + error (_("Could not convert argument to Python string.")); - resultobj = PyObject_CallMethodObjArgs ((PyObject *) obj, complete_cst, - textobj, wordobj, NULL); - Py_DECREF (textobj); - Py_DECREF (wordobj); - if (!resultobj) + gdbpy_ref resultobj (PyObject_CallMethodObjArgs ((PyObject *) obj, + complete_cst, + textobj.get (), + wordobj.get (), NULL)); + if (resultobj == NULL) { /* Just swallow errors here. */ PyErr_Clear (); } - Py_XINCREF (resultobj); - - return resultobj; + return resultobj.release (); } /* Python function called to determine the break characters of a @@ -277,26 +272,24 @@ static void cmdpy_completer_handle_brkchars (struct cmd_list_element *command, const char *text, const char *word) { - PyObject *resultobj = NULL; - gdbpy_enter enter_py (get_current_arch (), current_language); /* Calling our helper to obtain the PyObject of the Python function. */ - resultobj = cmdpy_completer_helper (command, text, word); + gdbpy_ref resultobj (cmdpy_completer_helper (command, text, word)); /* Check if there was an error. */ if (resultobj == NULL) - goto done; + return; - if (PyInt_Check (resultobj)) + if (PyInt_Check (resultobj.get ())) { /* User code may also return one of the completion constants, thus requesting that sort of completion. We are only interested in this kind of return. */ long value; - if (!gdb_py_int_as_long (resultobj, &value)) + if (!gdb_py_int_as_long (resultobj.get (), &value)) { /* Ignore. */ PyErr_Clear (); @@ -310,10 +303,6 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command, (completers[value].completer); } } - - done: - - Py_XDECREF (resultobj); } /* Called by gdb for command completion. */ @@ -322,29 +311,28 @@ static VEC (char_ptr) * cmdpy_completer (struct cmd_list_element *command, const char *text, const char *word) { - PyObject *resultobj = NULL; VEC (char_ptr) *result = NULL; gdbpy_enter enter_py (get_current_arch (), current_language); /* Calling our helper to obtain the PyObject of the Python function. */ - resultobj = cmdpy_completer_helper (command, text, word); + gdbpy_ref resultobj (cmdpy_completer_helper (command, text, word)); /* If the result object of calling the Python function is NULL, it means that there was an error. In this case, just give up and return NULL. */ if (resultobj == NULL) - goto done; + return NULL; result = NULL; - if (PyInt_Check (resultobj)) + if (PyInt_Check (resultobj.get ())) { /* User code may also return one of the completion constants, thus requesting that sort of completion. */ long value; - if (! gdb_py_int_as_long (resultobj, &value)) + if (! gdb_py_int_as_long (resultobj.get (), &value)) { /* Ignore. */ PyErr_Clear (); @@ -354,24 +342,24 @@ cmdpy_completer (struct cmd_list_element *command, } else { - PyObject *iter = PyObject_GetIter (resultobj); - PyObject *elt; + gdbpy_ref iter (PyObject_GetIter (resultobj.get ())); if (iter == NULL) - goto done; + return NULL; - while ((elt = PyIter_Next (iter)) != NULL) + while (true) { + gdbpy_ref elt (PyIter_Next (iter.get ())); + if (elt == NULL) + break; - if (! gdbpy_is_string (elt)) + if (! gdbpy_is_string (elt.get ())) { /* Skip problem elements. */ - Py_DECREF (elt); continue; } gdb::unique_xmalloc_ptr - item (python_string_to_host_string (elt)); - Py_DECREF (elt); + item (python_string_to_host_string (elt.get ())); if (item == NULL) { /* Skip problem elements. */ @@ -381,18 +369,12 @@ cmdpy_completer (struct cmd_list_element *command, VEC_safe_push (char_ptr, result, item.release ()); } - Py_DECREF (iter); - /* If we got some results, ignore problems. Otherwise, report the problem. */ if (result != NULL && PyErr_Occurred ()) PyErr_Clear (); } - done: - - Py_XDECREF (resultobj); - return result; } @@ -587,21 +569,18 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw) } if (PyObject_HasAttr (self, gdbpy_doc_cst)) { - PyObject *ds_obj = PyObject_GetAttr (self, gdbpy_doc_cst); + gdbpy_ref ds_obj (PyObject_GetAttr (self, gdbpy_doc_cst)); - if (ds_obj && gdbpy_is_string (ds_obj)) + if (ds_obj != NULL && gdbpy_is_string (ds_obj.get ())) { - docstring = python_string_to_host_string (ds_obj).release (); + docstring = python_string_to_host_string (ds_obj.get ()).release (); if (docstring == NULL) { xfree (cmd_name); xfree (pfx_name); - Py_DECREF (ds_obj); return -1; } } - - Py_XDECREF (ds_obj); } if (! docstring) docstring = xstrdup (_("This command is not documented."));