Re: [RFA 03/13] Use gdbpy_ref in py-cmd.c

[Tom Tromey <tom@tromey.com>]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> 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  <tom@tromey.com>
 
+	* 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  <tom@tromey.com>
+
 	* 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<char>
-	    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."));