[PATCH 0/2] Return gdbpy_ref in a couple more places

[PATCH 0/2] Return gdbpy_ref in a couple more places

[Andrew Burgess]

I spotted a couple of places where a gdbpy_ref was released in one
function only to be recreated again later.

Thanks,
Andrew

Andrew Burgess (2):
  gdb/python: Make a function return gdbpy_ref<>
  gdb/python: Make a function return gdbpy_ref<>

 gdb/ChangeLog            | 12 ++++++++++++
 gdb/python/py-cmd.c      | 16 ++++++++--------
 gdb/python/py-function.c |  9 ++++++---
 3 files changed, 26 insertions(+), 11 deletions(-)

-- 
2.14.5

[PATCH 1/2] gdb/python: Make a function return gdbpy_ref<>

[Andrew Burgess]

Make cmdpy_completer_helper return a gdbpy_ref<> directly rather than
building a gdbpy_ref<>, releasing it, and then having a new
gdbpy_ref<> created to hold the result.

gdb/ChangeLog:

	* python/py-cmd.c (cmdpy_completer_helper): Return gdbpy_ref<>.
	(cmdpy_completer_handle_brkchars): Adjust.
	(cmdpy_completer): Adjust.
---
 gdb/ChangeLog       |  6 ++++++
 gdb/python/py-cmd.c | 16 ++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index e7eb66f515c..d560b3a44e3 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -174,10 +174,10 @@ cmdpy_function (struct cmd_list_element *command,
    and then a "complete"-completion sequentially.  Therefore, we just
    recalculate everything twice for TAB-completions.
 
-   This function returns the PyObject representing the Python method
-   call.  */
+   This function returns a reference to the PyObject representing the
+   Python method call.  */
 
-static PyObject *
+static gdbpy_ref<>
 cmdpy_completer_helper (struct cmd_list_element *command,
 			const char *text, const char *word)
 {
@@ -220,7 +220,7 @@ cmdpy_completer_helper (struct cmd_list_element *command,
       PyErr_Clear ();
     }
 
-  return resultobj.release ();
+  return resultobj;
 }
 
 /* Python function called to determine the break characters of a
@@ -235,9 +235,9 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
 {
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  /* Calling our helper to obtain the PyObject of the Python
+  /* Calling our helper to obtain a reference to the PyObject of the Python
      function.  */
-  gdbpy_ref<> 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)
@@ -278,9 +278,9 @@ cmdpy_completer (struct cmd_list_element *command,
 {
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  /* Calling our helper to obtain the PyObject of the Python
+  /* Calling our helper to obtain a reference to the PyObject of the Python
      function.  */
-  gdbpy_ref<> 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.  */
-- 
2.14.5

[PATCH 2/2] gdb/python: Make a function return gdbpy_ref<>

[Andrew Burgess]

Make convert_values_to_python return a gdbpy_ref<> directly rather
than building a gdbpy_ref<>, releasing it, and then having a new
gdbpy_ref<> created to hold the result.

I also added a header comment to convert_values_to_python.

gdb/ChangeLog:

	* python/py-function.c (convert_values_to_python): Return
	gdbpy_ref<>. Add header comment.
	(fnpy_call): Adjust.
---
 gdb/ChangeLog            | 6 ++++++
 gdb/python/py-function.c | 9 ++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/gdb/python/py-function.c b/gdb/python/py-function.c
index 1900f0ff0c0..cf5e68a0545 100644
--- a/gdb/python/py-function.c
+++ b/gdb/python/py-function.c
@@ -34,7 +34,10 @@ extern PyTypeObject fnpy_object_type
 
 \f
 
-static PyObject *
+/* Return a reference to a tuple ARGC elements long.  Each element of the
+   tuple is a PyObject converted from the corresponding element of ARGV.  */
+
+static gdbpy_ref<>
 convert_values_to_python (int argc, struct value **argv)
 {
   int i;
@@ -50,7 +53,7 @@ convert_values_to_python (int argc, struct value **argv)
 	return NULL;
       PyTuple_SetItem (result.get (), i, elt.release ());
     }
-  return result.release ();
+  return result;
 }
 
 /* Call a Python function object's invoke method.  */
@@ -64,7 +67,7 @@ fnpy_call (struct gdbarch *gdbarch, const struct language_defn *language,
   gdbpy_enter enter_py (gdbarch, language);
   struct value *value;
   gdbpy_ref<> result;
-  gdbpy_ref<> args (convert_values_to_python (argc, argv));
+  gdbpy_ref<> args = convert_values_to_python (argc, argv);
 
   /* convert_values_to_python can return NULL on error.  If we
      encounter this, do not call the function, but allow the Python ->
-- 
2.14.5

Re: [PATCH 0/2] Return gdbpy_ref in a couple more places

[Simon Marchi]

On 2018-10-24 15:12, Andrew Burgess wrote:
> I spotted a couple of places where a gdbpy_ref was released in one
> function only to be recreated again later.
> 
> Thanks,
> Andrew

Thanks, both patches LGTM.  However, please update the commit titles to 
include the function name (e.g. "gdb/python: Make cmdpy_completer_helper 
return gdbpy_ref<>").  It's clearer than having multiple commits with 
the same title.

Simon