[PATCH] python: Make two functions return gdbpy_ref<>

[PATCH] python: Make two functions return gdbpy_ref<>

[Simon Marchi]

I noticed that we release a gdbpy_ref in pretty_print_one_value only to
create it again later.  This patch fills the gap by returning a
gdbpy_ref all the way.

gdb/ChangeLog:

	* python/py-prettyprint.c (pretty_print_one_value): Return
	gdbpy_ref<>.
	(print_string_repr): Adjust.
	(apply_varobj_pretty_printer): Return gdbpy_ref<>.
	* python/python-internal.h (apply_varobj_pretty_printer): Return
	gdbpy_ref<>.
	* varobj.c (varobj_value_get_print_value): Adjust.
---
 gdb/python/py-prettyprint.c  | 12 +++++-------
 gdb/python/python-internal.h |  7 ++++---
 gdb/varobj.c                 |  6 +++---
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index a8a8489..3c4c8cf 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -187,7 +187,7 @@ find_pretty_printer (PyObject *value)
    is returned.  On error, *OUT_VALUE is set to NULL, NULL is
    returned, with a python exception set.  */
 
-static PyObject *
+static gdbpy_ref<>
 pretty_print_one_value (PyObject *printer, struct value **out_value)
 {
   gdbpy_ref<> result;
@@ -215,7 +215,7 @@ pretty_print_one_value (PyObject *printer, struct value **out_value)
     }
   END_CATCH
 
-  return result.release ();
+  return result;
 }
 
 /* Return the display hint for the object printer, PRINTER.  Return
@@ -288,7 +288,7 @@ print_string_repr (PyObject *printer, const char *hint,
   struct value *replacement = NULL;
   enum string_repr_result result = string_repr_ok;
 
-  gdbpy_ref<> py_str (pretty_print_one_value (printer, &replacement));
+  gdbpy_ref<> py_str = pretty_print_one_value (printer, &replacement);
   if (py_str != NULL)
     {
       if (py_str == Py_None)
@@ -721,15 +721,13 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
    set to the replacement value and this function returns NULL.  On
    error, *REPLACEMENT is set to NULL and this function also returns
    NULL.  */
-PyObject *
+gdbpy_ref<>
 apply_varobj_pretty_printer (PyObject *printer_obj,
 			     struct value **replacement,
 			     struct ui_file *stream)
 {
-  PyObject *py_str = NULL;
-
   *replacement = NULL;
-  py_str = pretty_print_one_value (printer_obj, replacement);
+  gdbpy_ref<> py_str = pretty_print_one_value (printer_obj, replacement);
 
   if (*replacement == NULL && py_str == NULL)
     print_stack_unless_memory_error (stream);
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index c4f0e21..3874fdc 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -99,6 +99,7 @@
    from including our python/python.h header file.  */
 #include <Python.h>
 #include <frameobject.h>
+#include "py-ref.h"
 
 #if PY_MAJOR_VERSION >= 3
 #define IS_PY3K 1
@@ -691,9 +692,9 @@ int gdbpy_is_value_object (PyObject *obj);
 
 /* Note that these are declared here, and not in python.h with the
    other pretty-printer functions, because they refer to PyObject.  */
-PyObject *apply_varobj_pretty_printer (PyObject *print_obj,
-				       struct value **replacement,
-				       struct ui_file *stream);
+gdbpy_ref<> apply_varobj_pretty_printer (PyObject *print_obj,
+					 struct value **replacement,
+					 struct ui_file *stream);
 PyObject *gdbpy_get_varobj_pretty_printer (struct value *value);
 gdb::unique_xmalloc_ptr<char> gdbpy_get_display_hint (PyObject *printer);
 PyObject *gdbpy_default_visualizer (PyObject *self, PyObject *args);
diff --git a/gdb/varobj.c b/gdb/varobj.c
index af60796..e109926 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -2318,9 +2318,9 @@ varobj_value_get_print_value (struct value *value,
 	    {
 	      struct value *replacement;
 
-	      gdbpy_ref<> output (apply_varobj_pretty_printer (value_formatter,
-							       &replacement,
-							       &stb));
+	      gdbpy_ref<> output = apply_varobj_pretty_printer (value_formatter,
+								&replacement,
+								&stb);
 
 	      /* If we have string like output ...  */
 	      if (output != NULL)
-- 
2.7.4

Re: [PATCH] python: Make two functions return gdbpy_ref<>

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> I noticed that we release a gdbpy_ref in pretty_print_one_value only to
Simon> create it again later.  This patch fills the gap by returning a
Simon> gdbpy_ref all the way.

Simon> gdb/ChangeLog:

Simon> 	* python/py-prettyprint.c (pretty_print_one_value): Return
Simon> 	gdbpy_ref<>.
Simon> 	(print_string_repr): Adjust.
Simon> 	(apply_varobj_pretty_printer): Return gdbpy_ref<>.
Simon> 	* python/python-internal.h (apply_varobj_pretty_printer): Return
Simon> 	gdbpy_ref<>.
Simon> 	* varobj.c (varobj_value_get_print_value): Adjust.

Thanks, this seems like a good improvement to me.
I'm sure there are more of these lurking.

Tom

Re: [PATCH] python: Make two functions return gdbpy_ref<>

[Simon Marchi]

On 2018-09-09 03:55, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> I noticed that we release a gdbpy_ref in pretty_print_one_value 
> only to
> Simon> create it again later.  This patch fills the gap by returning a
> Simon> gdbpy_ref all the way.
> 
> Simon> gdb/ChangeLog:
> 
> Simon> 	* python/py-prettyprint.c (pretty_print_one_value): Return
> Simon> 	gdbpy_ref<>.
> Simon> 	(print_string_repr): Adjust.
> Simon> 	(apply_varobj_pretty_printer): Return gdbpy_ref<>.
> Simon> 	* python/python-internal.h (apply_varobj_pretty_printer): 
> Return
> Simon> 	gdbpy_ref<>.
> Simon> 	* varobj.c (varobj_value_get_print_value): Adjust.
> 
> Thanks, this seems like a good improvement to me.
> I'm sure there are more of these lurking.

Thanks, I pushed it.

Simon