public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA 10/14] Use gdbpy_reference in call_doc_function
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (5 preceding siblings ...)
  2016-11-07  5:48 ` [RFA 13/14] Use gdbpy_reference in py-value.c Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:48 ` [RFA 08/14] Use gdbpy_reference in py-framefilter.c Tom Tromey
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes call_doc_function to use gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-param.c (call_doc_function): Use gdbpy_reference.
---
 gdb/ChangeLog         |  4 ++++
 gdb/python/py-param.c | 11 +++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9aa7008..9544515 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-param.c (call_doc_function): Use gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-linetable.c (build_line_table_tuple_from_pcs)
 	(ltpy_get_all_source_lines): Use gdbpy_reference.
 
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index 3604f9f..dae2f7b 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -27,6 +27,7 @@
 #include "completer.h"
 #include "language.h"
 #include "arch-utils.h"
+#include "py-ref.h"
 
 /* Parameter constants and their values.  */
 struct parm_constant
@@ -331,15 +332,14 @@ static char *
 call_doc_function (PyObject *obj, PyObject *method, PyObject *arg)
 {
   char *data = NULL;
-  PyObject *result = PyObject_CallMethodObjArgs (obj, method, arg, NULL);
+  gdbpy_reference result (PyObject_CallMethodObjArgs (obj, method, arg, NULL));
 
-  if (! result)
+  if (result == NULL)
     return NULL;
 
-  if (gdbpy_is_string (result))
+  if (gdbpy_is_string (result.get ()))
     {
-      data = python_string_to_host_string (result);
-      Py_DECREF (result);
+      data = python_string_to_host_string (result.get ());
       if (! data)
 	return NULL;
     }
@@ -347,7 +347,6 @@ call_doc_function (PyObject *obj, PyObject *method, PyObject *arg)
     {
       PyErr_SetString (PyExc_RuntimeError,
 		       _("Parameter must return a string value."));
-      Py_DECREF (result);
       return NULL;
     }
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 07/14] Use gdbpy_reference in gdbpy_breakpoints
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (9 preceding siblings ...)
  2016-11-07  5:48 ` [RFA 05/14] Use gdbpy_reference in py-function.c Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:48 ` [RFA 03/14] Use gdbpy_reference in py-type.c Tom Tromey
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes gdbpy_breakpoints to use gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-breakpoint.c (gdbpy_breakpoints): Use gdbpy_reference.
---
 gdb/ChangeLog              |  4 ++++
 gdb/python/py-breakpoint.c | 18 +++++-------------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 164b568..53f2e5f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-breakpoint.c (gdbpy_breakpoints): Use gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-inferior.c (gdbpy_inferiors): Use gdbpy_reference.
 
 2016-11-06  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 656b13a..f9fbc06 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -763,28 +763,20 @@ build_bp_list (struct breakpoint *b, void *arg)
 PyObject *
 gdbpy_breakpoints (PyObject *self, PyObject *args)
 {
-  PyObject *list, *tuple;
-
   if (bppy_live == 0)
     return PyTuple_New (0);
 
-  list = PyList_New (0);
-  if (!list)
+  gdbpy_reference list (PyList_New (0));
+  if (list == NULL)
     return NULL;
 
   /* If iterate_over_breakpoints returns non NULL it signals an error
      condition.  In that case abandon building the list and return
      NULL.  */
-  if (iterate_over_breakpoints (build_bp_list, list) != NULL)
-    {
-      Py_DECREF (list);
-      return NULL;
-    }
-
-  tuple = PyList_AsTuple (list);
-  Py_DECREF (list);
+  if (iterate_over_breakpoints (build_bp_list, list.get ()) != NULL)
+    return NULL;
 
-  return tuple;
+  return PyList_AsTuple (list.get ());
 }
 
 /* Call the "stop" method (if implemented) in the breakpoint
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 01/14] Introduce py-ref.h
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (2 preceding siblings ...)
  2016-11-07  5:48 ` [RFA 14/14] Use gdbpy_reference in gdbpy_lookup_symbol Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  7:45   ` Jan Kratochvil
  2016-11-10 23:48   ` Pedro Alves
  2016-11-07  5:48 ` [RFA 04/14] Use gdbpy_reference in gdbpy_string_to_argv Tom Tromey
                   ` (11 subsequent siblings)
  15 siblings, 2 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This patch introduces class gdbpy_reference, which is a sort of smart
pointer that owns a single Python reference to a PyObject.  This class
acts a bit like unique_ptr, but also a bit like shared_ptr (in that
copies do what you might expect); I considered going solely with
unique_ptr but it seemed quite strange to have a unique_ptr that
actually manages a shared resource.

Subsequent patches use this new class to simplify logic in the Python
layer.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-ref.h: New file.
---
 gdb/ChangeLog       |   4 ++
 gdb/python/py-ref.h | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 146 insertions(+)
 create mode 100644 gdb/python/py-ref.h

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8e173a7..b46e26a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
+	* python/py-ref.h: New file.
+
 2016-10-29  Manish Goregaokar  <manish@mozilla.com>
 
     * rust-exp.y: Parse `sizeof(exp)` as `UNOP_SIZEOF`
diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
new file mode 100644
index 0000000..1b1f1d2
--- /dev/null
+++ b/gdb/python/py-ref.h
@@ -0,0 +1,142 @@
+/* Python reference-holding class
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_PYTHON_REF_H
+#define GDB_PYTHON_REF_H
+
+/* An instance of this class holds a reference to a PyObject.  When
+   the object is destroyed, the PyObject is decref'd.
+
+   Normally an instance is constructed using a PyObject*.  This sort
+   of initialization lets this class manage the lifetime of that
+   reference.
+
+   Assignment and copy construction will make a new reference as
+   appropriate.  Assignment from a plain PyObject* is disallowed to
+   avoid confusion about whether this acquires a new reference;
+   instead use the "reset" method -- which, like the PyObject*
+   constructor, transfers ownership.
+*/
+class gdbpy_reference
+{
+ public:
+
+  /* Create a new NULL instance.  */
+  gdbpy_reference ()
+    : m_obj (NULL)
+  {
+  }
+
+  /* Create a new NULL instance.  */
+  gdbpy_reference (nullptr_t)
+    : m_obj (NULL)
+  {
+  }
+
+  /* Create a new instance.  OBJ is a reference, management of which
+     is now transferred to this class.  */
+  explicit gdbpy_reference (PyObject *obj)
+    : m_obj (obj)
+  {
+  }
+
+  /* Copy another instance.  */
+  gdbpy_reference (const gdbpy_reference &other)
+    : m_obj (other.m_obj)
+  {
+    if (m_obj != NULL)
+      Py_INCREF (m_obj);
+  }
+
+  /* Transfer ownership from another instance.  */
+  gdbpy_reference (gdbpy_reference &&other)
+    : m_obj (other.m_obj)
+  {
+    other.m_obj = NULL;
+  }
+
+  /* Destroy this instance.  */
+  ~gdbpy_reference ()
+  {
+    Py_XDECREF (m_obj);
+  }
+
+  /* Copy another instance.  */
+  gdbpy_reference &operator= (const gdbpy_reference &other)
+  {
+    Py_XDECREF (m_obj);
+    m_obj = other.m_obj;
+    if (m_obj != NULL)
+      Py_INCREF (m_obj);
+    return *this;
+  }
+
+  /* Transfer ownership from another instance.  */
+  gdbpy_reference &operator= (gdbpy_reference &&other)
+  {
+    Py_XDECREF (m_obj);
+    m_obj = other.m_obj;
+    other.m_obj = NULL;
+    return *this;
+  }
+
+  /* Change this instance's referent.  OBJ is a reference, management
+     of which is now transferred to this class.  */
+  void reset (PyObject *obj)
+  {
+    Py_XDECREF (m_obj);
+    m_obj = obj;
+  }
+
+  /* Return this instance's referent.  In Python terms this is a
+     borrowed pointer.  */
+  PyObject *get ()
+  {
+    return m_obj;
+  }
+
+  /* Return this instance's referent, and stop managing this
+     reference.  The caller is now responsible for the ownership of
+     the reference.  */
+  PyObject *release ()
+  {
+    PyObject *result = m_obj;
+
+    m_obj = NULL;
+    return result;
+  }
+
+  /* Equality.  */
+  bool operator== (const PyObject *other) const
+  {
+    return m_obj == other;
+  }
+
+  /* Inequality.  */
+  bool operator!= (const PyObject *other) const
+  {
+    return m_obj != other;
+  }
+
+ private:
+
+  PyObject *m_obj;
+};
+
+#endif /* GDB_PYTHON_REF_H */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 11/14] Use gdbpy_reference in py-prettyprint.c
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (7 preceding siblings ...)
  2016-11-07  5:48 ` [RFA 08/14] Use gdbpy_reference in py-framefilter.c Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:48 ` [RFA 05/14] Use gdbpy_reference in py-function.c Tom Tromey
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes several functions in py-prettyprint.c to use
gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-prettyprint.c (search_pp_list)
	(find_pretty_printer_from_objfiles)
	(find_pretty_printer_from_progspace)
	(find_pretty_printer_from_gdb, find_pretty_printer)
	(gdbpy_get_display_hint, gdbpy_get_varobj_pretty_printer): Use
	gdbpy_reference.
---
 gdb/ChangeLog               |  9 +++++
 gdb/python/py-prettyprint.c | 96 ++++++++++++++++-----------------------------
 2 files changed, 43 insertions(+), 62 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9544515..1b49750 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-prettyprint.c (search_pp_list)
+	(find_pretty_printer_from_objfiles)
+	(find_pretty_printer_from_progspace)
+	(find_pretty_printer_from_gdb, find_pretty_printer)
+	(gdbpy_get_display_hint, gdbpy_get_varobj_pretty_printer): Use
+	gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-param.c (call_doc_function): Use gdbpy_reference.
 
 2016-11-06  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index 8834344..0183b16 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -25,6 +25,7 @@
 #include "extension-priv.h"
 #include "python.h"
 #include "python-internal.h"
+#include "py-ref.h"
 
 /* Return type of print_string_repr.  */
 
@@ -48,25 +49,23 @@ static PyObject *
 search_pp_list (PyObject *list, PyObject *value)
 {
   Py_ssize_t pp_list_size, list_index;
-  PyObject *function, *printer = NULL;
 
   pp_list_size = PyList_Size (list);
   for (list_index = 0; list_index < pp_list_size; list_index++)
     {
-      function = PyList_GetItem (list, list_index);
+      PyObject *function = PyList_GetItem (list, list_index);
       if (! function)
 	return NULL;
 
       /* Skip if disabled.  */
       if (PyObject_HasAttr (function, gdbpy_enabled_cst))
 	{
-	  PyObject *attr = PyObject_GetAttr (function, gdbpy_enabled_cst);
+	  gdbpy_reference attr (PyObject_GetAttr (function, gdbpy_enabled_cst));
 	  int cmp;
 
-	  if (!attr)
+	  if (attr == NULL)
 	    return NULL;
-	  cmp = PyObject_IsTrue (attr);
-	  Py_DECREF (attr);
+	  cmp = PyObject_IsTrue (attr.get ());
 	  if (cmp == -1)
 	    return NULL;
 
@@ -74,13 +73,12 @@ search_pp_list (PyObject *list, PyObject *value)
 	    continue;
 	}
 
-      printer = PyObject_CallFunctionObjArgs (function, value, NULL);
-      if (! printer)
+      gdbpy_reference printer (PyObject_CallFunctionObjArgs (function, value,
+							     NULL));
+      if (printer == NULL)
 	return NULL;
       else if (printer != Py_None)
-	return printer;
-
-      Py_DECREF (printer);
+	return printer.release ();
     }
 
   Py_RETURN_NONE;
@@ -95,8 +93,6 @@ search_pp_list (PyObject *list, PyObject *value)
 static PyObject *
 find_pretty_printer_from_objfiles (PyObject *value)
 {
-  PyObject *pp_list;
-  PyObject *function;
   struct objfile *obj;
 
   ALL_OBJFILES (obj)
@@ -109,18 +105,15 @@ find_pretty_printer_from_objfiles (PyObject *value)
 	continue;
       }
 
-    pp_list = objfpy_get_printers (objf, NULL);
-    function = search_pp_list (pp_list, value);
-    Py_XDECREF (pp_list);
+    gdbpy_reference pp_list (objfpy_get_printers (objf, NULL));
+    gdbpy_reference function (search_pp_list (pp_list.get (), value));
 
     /* If there is an error in any objfile list, abort the search and exit.  */
-    if (! function)
+    if (function == NULL)
       return NULL;
 
     if (function != Py_None)
-      return function;
-
-    Py_DECREF (function);
+      return function.release ();
   }
 
   Py_RETURN_NONE;
@@ -135,16 +128,12 @@ find_pretty_printer_from_objfiles (PyObject *value)
 static PyObject *
 find_pretty_printer_from_progspace (PyObject *value)
 {
-  PyObject *pp_list;
-  PyObject *function;
   PyObject *obj = pspace_to_pspace_object (current_program_space);
 
   if (!obj)
     return NULL;
-  pp_list = pspy_get_printers (obj, NULL);
-  function = search_pp_list (pp_list, value);
-  Py_XDECREF (pp_list);
-  return function;
+  gdbpy_reference pp_list (pspy_get_printers (obj, NULL));
+  return search_pp_list (pp_list.get (), value);
 }
 
 /* Subroutine of find_pretty_printer to simplify it.
@@ -156,23 +145,16 @@ find_pretty_printer_from_progspace (PyObject *value)
 static PyObject *
 find_pretty_printer_from_gdb (PyObject *value)
 {
-  PyObject *pp_list;
-  PyObject *function;
-
   /* Fetch the global pretty printer list.  */
   if (gdb_python_module == NULL
       || ! PyObject_HasAttrString (gdb_python_module, "pretty_printers"))
     Py_RETURN_NONE;
-  pp_list = PyObject_GetAttrString (gdb_python_module, "pretty_printers");
-  if (pp_list == NULL || ! PyList_Check (pp_list))
-    {
-      Py_XDECREF (pp_list);
-      Py_RETURN_NONE;
-    }
+  gdbpy_reference pp_list (PyObject_GetAttrString (gdb_python_module,
+						   "pretty_printers"));
+  if (pp_list == NULL || ! PyList_Check (pp_list.get ()))
+    Py_RETURN_NONE;
 
-  function = search_pp_list (pp_list, value);
-  Py_XDECREF (pp_list);
-  return function;
+  return search_pp_list (pp_list.get (), value);
 }
 
 /* Find the pretty-printing constructor function for VALUE.  If no
@@ -182,24 +164,19 @@ find_pretty_printer_from_gdb (PyObject *value)
 static PyObject *
 find_pretty_printer (PyObject *value)
 {
-  PyObject *function;
-
   /* Look at the pretty-printer list for each objfile
      in the current program-space.  */
-  function = find_pretty_printer_from_objfiles (value);
+  gdbpy_reference function (find_pretty_printer_from_objfiles (value));
   if (function == NULL || function != Py_None)
-    return function;
-  Py_DECREF (function);
+    return function.release ();
 
   /* Look at the pretty-printer list for the current program-space.  */
-  function = find_pretty_printer_from_progspace (value);
+  function.reset (find_pretty_printer_from_progspace (value));
   if (function == NULL || function != Py_None)
-    return function;
-  Py_DECREF (function);
+    return function.release ();
 
   /* Look at the pretty-printer list in the gdb module.  */
-  function = find_pretty_printer_from_gdb (value);
-  return function;
+  return find_pretty_printer_from_gdb (value);
 }
 
 /* Pretty-print a single value, via the printer object PRINTER.
@@ -247,22 +224,22 @@ pretty_print_one_value (PyObject *printer, struct value **out_value)
 char *
 gdbpy_get_display_hint (PyObject *printer)
 {
-  PyObject *hint;
   char *result = NULL;
 
   if (! PyObject_HasAttr (printer, gdbpy_display_hint_cst))
     return NULL;
 
-  hint = PyObject_CallMethodObjArgs (printer, gdbpy_display_hint_cst, NULL);
-  if (hint)
+  gdbpy_reference hint (PyObject_CallMethodObjArgs (printer,
+						    gdbpy_display_hint_cst,
+						    NULL));
+  if (hint != NULL)
     {
-      if (gdbpy_is_string (hint))
+      if (gdbpy_is_string (hint.get ()))
 	{
-	  result = python_string_to_host_string (hint);
+	  result = python_string_to_host_string (hint.get ());
 	  if (result == NULL)
 	    gdbpy_print_stack ();
 	}
-      Py_DECREF (hint);
     }
   else
     gdbpy_print_stack ();
@@ -818,9 +795,6 @@ apply_varobj_pretty_printer (PyObject *printer_obj,
 PyObject *
 gdbpy_get_varobj_pretty_printer (struct value *value)
 {
-  PyObject *val_obj;
-  PyObject *pretty_printer = NULL;
-
   TRY
     {
       value = value_copy (value);
@@ -831,13 +805,11 @@ gdbpy_get_varobj_pretty_printer (struct value *value)
     }
   END_CATCH
 
-  val_obj = value_to_value_object (value);
-  if (! val_obj)
+  gdbpy_reference val_obj (value_to_value_object (value));
+  if (val_obj == NULL)
     return NULL;
 
-  pretty_printer = find_pretty_printer (val_obj);
-  Py_DECREF (val_obj);
-  return pretty_printer;
+  return find_pretty_printer (val_obj.get ());
 }
 
 /* A Python function which wraps find_pretty_printer and instantiates
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 04/14] Use gdbpy_reference in gdbpy_string_to_argv
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (3 preceding siblings ...)
  2016-11-07  5:48 ` [RFA 01/14] Introduce py-ref.h Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:48 ` [RFA 13/14] Use gdbpy_reference in py-value.c Tom Tromey
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This chanes gdbpy_string_to_argv to use gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-cmd.c (gdbpy_string_to_argv): Use gdbpy_reference.
---
 gdb/ChangeLog       |  4 ++++
 gdb/python/py-cmd.c | 13 +++++--------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4475a66..1a0a21f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-cmd.c (gdbpy_string_to_argv): Use gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-type.c (convert_field, make_fielditem, typy_fields)
 	(typy_range): Use gdbpy_reference.
 
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index af6c5cf..80216e3 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -27,6 +27,7 @@
 #include "cli/cli-decode.h"
 #include "completer.h"
 #include "language.h"
+#include "py-ref.h"
 
 /* Struct representing built-in completion types.  */
 struct cmdpy_completer
@@ -785,13 +786,12 @@ PyTypeObject cmdpy_object_type =
 PyObject *
 gdbpy_string_to_argv (PyObject *self, PyObject *args)
 {
-  PyObject *py_argv;
   const char *input;
 
   if (!PyArg_ParseTuple (args, "s", &input))
     return NULL;
 
-  py_argv = PyList_New (0);
+  gdbpy_reference py_argv (PyList_New (0));
   if (py_argv == NULL)
     return NULL;
 
@@ -806,21 +806,18 @@ gdbpy_string_to_argv (PyObject *self, PyObject *args)
 
       for (i = 0; c_argv[i] != NULL; ++i)
 	{
-	  PyObject *argp = PyString_FromString (c_argv[i]);
+	  gdbpy_reference argp (PyString_FromString (c_argv[i]));
 
 	  if (argp == NULL
-	      || PyList_Append (py_argv, argp) < 0)
+	      || PyList_Append (py_argv.get (), argp.get ()) < 0)
 	    {
-	      Py_XDECREF (argp);
-	      Py_DECREF (py_argv);
 	      freeargv (c_argv);
 	      return NULL;
 	    }
-	  Py_DECREF (argp);
 	}
 
       freeargv (c_argv);
     }
 
-  return py_argv;
+  return py_argv.release ();
 }
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 08/14] Use gdbpy_reference in py-framefilter.c
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (6 preceding siblings ...)
  2016-11-07  5:48 ` [RFA 10/14] Use gdbpy_reference in call_doc_function Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:48 ` [RFA 11/14] Use gdbpy_reference in py-prettyprint.c Tom Tromey
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes some code in py-framefilter.c to use gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-framefilter.c (extract_sym, extract_value)
	(get_py_iter_from_func, bootstrap_python_frame_filters): Use
	gdbpy_reference.
---
 gdb/ChangeLog               |  6 ++++
 gdb/python/py-framefilter.c | 86 +++++++++++++++------------------------------
 2 files changed, 35 insertions(+), 57 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 53f2e5f..4921600 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-framefilter.c (extract_sym, extract_value)
+	(get_py_iter_from_func, bootstrap_python_frame_filters): Use
+	gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-breakpoint.c (gdbpy_breakpoints): Use gdbpy_reference.
 
 2016-11-06  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 6692ac5..d010408 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -30,6 +30,7 @@
 #include "demangle.h"
 #include "mi/mi-cmds.h"
 #include "python-internal.h"
+#include "py-ref.h"
 
 enum mi_print_types
 {
@@ -55,17 +56,16 @@ static enum ext_lang_bt_status
 extract_sym (PyObject *obj, char **name, struct symbol **sym,
 	     struct block **sym_block, const struct language_defn **language)
 {
-  PyObject *result = PyObject_CallMethod (obj, "symbol", NULL);
+  gdbpy_reference result (PyObject_CallMethod (obj, "symbol", NULL));
 
   if (result == NULL)
     return EXT_LANG_BT_ERROR;
 
   /* For 'symbol' callback, the function can return a symbol or a
      string.  */
-  if (gdbpy_is_string (result))
+  if (gdbpy_is_string (result.get ()))
     {
-      *name = python_string_to_host_string (result);
-      Py_DECREF (result);
+      *name = python_string_to_host_string (result.get ());
 
       if (*name == NULL)
 	return EXT_LANG_BT_ERROR;
@@ -82,15 +82,13 @@ extract_sym (PyObject *obj, char **name, struct symbol **sym,
     {
       /* This type checks 'result' during the conversion so we
 	 just call it unconditionally and check the return.  */
-      *sym = symbol_object_to_symbol (result);
+      *sym = symbol_object_to_symbol (result.get ());
       /* TODO: currently, we have no way to recover the block in which SYMBOL
 	 was found, so we have no block to return.  Trying to evaluate SYMBOL
 	 will yield an incorrect value when it's located in a FRAME and
 	 evaluated from another frame (as permitted in nested functions).  */
       *sym_block = NULL;
 
-      Py_DECREF (result);
-
       if (*sym == NULL)
 	{
 	  PyErr_SetString (PyExc_RuntimeError,
@@ -129,7 +127,7 @@ extract_value (PyObject *obj, struct value **value)
 {
   if (PyObject_HasAttrString (obj, "value"))
     {
-      PyObject *vresult = PyObject_CallMethod (obj, "value", NULL);
+      gdbpy_reference vresult (PyObject_CallMethod (obj, "value", NULL));
 
       if (vresult == NULL)
 	return EXT_LANG_BT_ERROR;
@@ -139,14 +137,12 @@ extract_value (PyObject *obj, struct value **value)
 	 value.  */
       if (vresult == Py_None)
 	{
-	  Py_DECREF (vresult);
 	  *value = NULL;
 	  return EXT_LANG_BT_OK;
 	}
       else
 	{
-	  *value = convert_value_from_python (vresult);
-	  Py_DECREF (vresult);
+	  *value = convert_value_from_python (vresult.get ());
 
 	  if (*value == NULL)
 	    return EXT_LANG_BT_ERROR;
@@ -315,20 +311,17 @@ get_py_iter_from_func (PyObject *filter, char *func)
 {
   if (PyObject_HasAttrString (filter, func))
     {
-      PyObject *result = PyObject_CallMethod (filter, func, NULL);
+      gdbpy_reference result (PyObject_CallMethod (filter, func, NULL));
 
       if (result != NULL)
 	{
 	  if (result == Py_None)
 	    {
-	      return result;
+	      return result.release ();
 	    }
 	  else
 	    {
-	      PyObject *iterator = PyObject_GetIter (result);
-
-	      Py_DECREF (result);
-	      return iterator;
+	      return PyObject_GetIter (result.get ());
 	    }
 	}
     }
@@ -1445,60 +1438,39 @@ static PyObject *
 bootstrap_python_frame_filters (struct frame_info *frame,
 				int frame_low, int frame_high)
 {
-  struct cleanup *cleanups =
-    make_cleanup (null_cleanup, NULL);
-  PyObject *module, *sort_func, *iterable, *frame_obj, *iterator;
-  PyObject *py_frame_low, *py_frame_high;
-
-  frame_obj = frame_info_to_frame_object (frame);
+  gdbpy_reference frame_obj (frame_info_to_frame_object (frame));
   if (frame_obj == NULL)
-    goto error;
-  make_cleanup_py_decref (frame_obj);
+    return NULL;
 
-  module = PyImport_ImportModule ("gdb.frames");
+  gdbpy_reference module (PyImport_ImportModule ("gdb.frames"));
   if (module == NULL)
-    goto error;
-  make_cleanup_py_decref (module);
+    return NULL;
 
-  sort_func = PyObject_GetAttrString (module, "execute_frame_filters");
+  gdbpy_reference sort_func (PyObject_GetAttrString (module.get (),
+						     "execute_frame_filters"));
   if (sort_func == NULL)
-    goto error;
-  make_cleanup_py_decref (sort_func);
+    return NULL;
 
-  py_frame_low = PyInt_FromLong (frame_low);
+  gdbpy_reference py_frame_low (PyInt_FromLong (frame_low));
   if (py_frame_low == NULL)
-    goto error;
-  make_cleanup_py_decref (py_frame_low);
+    return NULL;
 
-  py_frame_high = PyInt_FromLong (frame_high);
+  gdbpy_reference py_frame_high (PyInt_FromLong (frame_high));
   if (py_frame_high == NULL)
-    goto error;
-  make_cleanup_py_decref (py_frame_high);
+    return NULL;
 
-  iterable = PyObject_CallFunctionObjArgs (sort_func, frame_obj,
-					   py_frame_low,
-					   py_frame_high,
-					   NULL);
+  gdbpy_reference iterable (PyObject_CallFunctionObjArgs (sort_func.get (),
+							  frame_obj.get (),
+							  py_frame_low.get (),
+							  py_frame_high.get (),
+							  NULL));
   if (iterable == NULL)
-    goto error;
-
-  do_cleanups (cleanups);
+    return NULL;
 
   if (iterable != Py_None)
-    {
-      iterator = PyObject_GetIter (iterable);
-      Py_DECREF (iterable);
-    }
+    return PyObject_GetIter (iterable.get ());
   else
-    {
-      return iterable;
-    }
-
-  return iterator;
-
- error:
-  do_cleanups (cleanups);
-  return NULL;
+    return iterable.release ();
 }
 
 /*  This is the only publicly exported function in this file.  FRAME
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 03/14] Use gdbpy_reference in py-type.c
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (10 preceding siblings ...)
  2016-11-07  5:48 ` [RFA 07/14] Use gdbpy_reference in gdbpy_breakpoints Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:56 ` [RFA 02/14] Change event code to use gdbpy_reference Tom Tromey
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes py-type.c to use gdbpy_reference.
This results in simpler logic and the removal of "goto"s.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-type.c (convert_field, make_fielditem, typy_fields)
	(typy_range): Use gdbpy_reference.
---
 gdb/ChangeLog        |   5 ++
 gdb/python/py-type.c | 195 +++++++++++++++++++++------------------------------
 2 files changed, 83 insertions(+), 117 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a827e37..4475a66 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-type.c (convert_field, make_fielditem, typy_fields)
+	(typy_range): Use gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-threadevent.c (create_thread_event_object): Use
 	gdbpy_reference.
 	* python/py-stopevent.c (create_stop_event_object): Simplify.
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 03cc8d9..72efdbb 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -28,6 +28,7 @@
 #include "language.h"
 #include "vec.h"
 #include "typeprint.h"
+#include "py-ref.h"
 
 typedef struct pyty_type_object
 {
@@ -168,18 +169,16 @@ typy_get_code (PyObject *self, void *closure)
 static PyObject *
 convert_field (struct type *type, int field)
 {
-  PyObject *result = field_new ();
-  PyObject *arg;
+  gdbpy_reference result (field_new ());
 
-  if (!result)
+  if (result == NULL)
     return NULL;
 
-  arg = type_to_type_object (type);
+  gdbpy_reference arg (type_to_type_object (type));
   if (arg == NULL)
-    goto fail;
-  if (PyObject_SetAttrString (result, "parent_type", arg) < 0)
-    goto failarg;
-  Py_DECREF (arg);
+    return NULL;
+  if (PyObject_SetAttrString (result.get (), "parent_type", arg.get ()) < 0)
+    return NULL;
 
   if (!field_is_static (&TYPE_FIELD (type, field)))
     {
@@ -187,88 +186,79 @@ convert_field (struct type *type, int field)
 
       if (TYPE_CODE (type) == TYPE_CODE_ENUM)
 	{
-	  arg = gdb_py_long_from_longest (TYPE_FIELD_ENUMVAL (type, field));
+	  arg.reset (gdb_py_long_from_longest (TYPE_FIELD_ENUMVAL (type,
+								   field)));
 	  attrstring = "enumval";
 	}
       else
 	{
-	  arg = gdb_py_long_from_longest (TYPE_FIELD_BITPOS (type, field));
+	  arg.reset (gdb_py_long_from_longest (TYPE_FIELD_BITPOS (type,
+								  field)));
 	  attrstring = "bitpos";
 	}
 
-      if (!arg)
-	goto fail;
+      if (arg == NULL)
+	return NULL;
 
       /* At least python-2.4 had the second parameter non-const.  */
-      if (PyObject_SetAttrString (result, (char *) attrstring, arg) < 0)
-	goto failarg;
-      Py_DECREF (arg);
+      if (PyObject_SetAttrString (result.get (), (char *) attrstring,
+				  arg.get ()) < 0)
+	return NULL;
     }
 
-  arg = NULL;
+  arg.reset (NULL);
   if (TYPE_FIELD_NAME (type, field))
     {
       const char *field_name = TYPE_FIELD_NAME (type, field);
 
       if (field_name[0] != '\0')
 	{
-	  arg = PyString_FromString (TYPE_FIELD_NAME (type, field));
+	  arg.reset (PyString_FromString (TYPE_FIELD_NAME (type, field)));
 	  if (arg == NULL)
-	    goto fail;
+	    return NULL;
 	}
     }
   if (arg == NULL)
     {
-      arg = Py_None;
-      Py_INCREF (arg);
+      arg.reset (Py_None);
+      Py_INCREF (arg.get ());
     }
-  if (PyObject_SetAttrString (result, "name", arg) < 0)
-    goto failarg;
-  Py_DECREF (arg);
+  if (PyObject_SetAttrString (result.get (), "name", arg.get ()) < 0)
+    return NULL;
 
-  arg = TYPE_FIELD_ARTIFICIAL (type, field) ? Py_True : Py_False;
-  Py_INCREF (arg);
-  if (PyObject_SetAttrString (result, "artificial", arg) < 0)
-    goto failarg;
-  Py_DECREF (arg);
+  arg.reset (TYPE_FIELD_ARTIFICIAL (type, field) ? Py_True : Py_False);
+  Py_INCREF (arg.get ());
+  if (PyObject_SetAttrString (result.get (), "artificial", arg.get ()) < 0)
+    return NULL;
 
   if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
-    arg = field < TYPE_N_BASECLASSES (type) ? Py_True : Py_False;
+    arg.reset (field < TYPE_N_BASECLASSES (type) ? Py_True : Py_False);
   else
-    arg = Py_False;
-  Py_INCREF (arg);
-  if (PyObject_SetAttrString (result, "is_base_class", arg) < 0)
-    goto failarg;
-  Py_DECREF (arg);
-
-  arg = PyLong_FromLong (TYPE_FIELD_BITSIZE (type, field));
-  if (!arg)
-    goto fail;
-  if (PyObject_SetAttrString (result, "bitsize", arg) < 0)
-    goto failarg;
-  Py_DECREF (arg);
+    arg.reset (Py_False);
+  Py_INCREF (arg.get ());
+  if (PyObject_SetAttrString (result.get (), "is_base_class", arg.get ()) < 0)
+    return NULL;
+
+  arg.reset (PyLong_FromLong (TYPE_FIELD_BITSIZE (type, field)));
+  if (arg == NULL)
+    return NULL;
+  if (PyObject_SetAttrString (result.get (), "bitsize", arg.get ()) < 0)
+    return NULL;
 
   /* A field can have a NULL type in some situations.  */
   if (TYPE_FIELD_TYPE (type, field) == NULL)
     {
-      arg = Py_None;
-      Py_INCREF (arg);
+      arg.reset (Py_None);
+      Py_INCREF (arg.get ());
     }
   else
-    arg = type_to_type_object (TYPE_FIELD_TYPE (type, field));
-  if (!arg)
-    goto fail;
-  if (PyObject_SetAttrString (result, "type", arg) < 0)
-    goto failarg;
-  Py_DECREF (arg);
-
-  return result;
+    arg.reset (type_to_type_object (TYPE_FIELD_TYPE (type, field)));
+  if (arg == NULL)
+    return NULL;
+  if (PyObject_SetAttrString (result.get (), "type", arg.get ()) < 0)
+    return NULL;
 
- failarg:
-  Py_DECREF (arg);
- fail:
-  Py_DECREF (result);
-  return NULL;
+  return result.release ();
 }
 
 /* Helper function to return the name of a field, as a gdb.Field object.
@@ -298,39 +288,29 @@ field_name (struct type *type, int field)
 static PyObject *
 make_fielditem (struct type *type, int i, enum gdbpy_iter_kind kind)
 {
-  PyObject *item = NULL, *key = NULL, *value = NULL;
-
   switch (kind)
     {
     case iter_items:
-      key = field_name (type, i);
-      if (key == NULL)
-	goto fail;
-      value = convert_field (type, i);
-      if (value == NULL)
-	goto fail;
-      item = PyTuple_New (2);
-      if (item == NULL)
-	goto fail;
-      PyTuple_SET_ITEM (item, 0, key);
-      PyTuple_SET_ITEM (item, 1, value);
-      break;
+      {
+	gdbpy_reference key (field_name (type, i));
+	if (key == NULL)
+	  return NULL;
+	gdbpy_reference value (convert_field (type, i));
+	if (value == NULL)
+	  return NULL;
+	gdbpy_reference item (PyTuple_New (2));
+	if (item == NULL)
+	  return NULL;
+	PyTuple_SET_ITEM (item.get (), 0, key.release ());
+	PyTuple_SET_ITEM (item.get (), 1, value.release ());
+	return item.release ();
+      }
     case iter_keys:
-      item = field_name (type, i);
-      break;
+      return field_name (type, i);
     case iter_values:
-      item =  convert_field (type, i);
-      break;
-    default:
-      gdb_assert_not_reached ("invalid gdbpy_iter_kind");
+      return convert_field (type, i);
     }
-  return item;
-
- fail:
-  Py_XDECREF (key);
-  Py_XDECREF (value);
-  Py_XDECREF (item);
-  return NULL;
+  gdb_assert_not_reached ("invalid gdbpy_iter_kind");
 }
 
 /* Return a sequence of all field names, fields, or (name, field) pairs.
@@ -389,7 +369,6 @@ static PyObject *
 typy_fields (PyObject *self, PyObject *args)
 {
   struct type *type = ((type_object *) self)->type;
-  PyObject *r, *rl;
 
   if (TYPE_CODE (type) != TYPE_CODE_ARRAY)
     return typy_fields_items (self, iter_values);
@@ -397,14 +376,11 @@ typy_fields (PyObject *self, PyObject *args)
   /* Array type.  Handle this as a special case because the common
      machinery wants struct or union or enum types.  Build a list of
      one entry which is the range for the array.  */
-  r = convert_field (type, 0);
+  gdbpy_reference r (convert_field (type, 0));
   if (r == NULL)
     return NULL;
 
-  rl = Py_BuildValue ("[O]", r);
-  Py_DECREF (r);
-
-  return rl;
+  return Py_BuildValue ("[O]", r.get ());
 }
 
 /* Return a sequence of all field names.  Each field is a gdb.Field object.  */
@@ -601,8 +577,6 @@ static PyObject *
 typy_range (PyObject *self, PyObject *args)
 {
   struct type *type = ((type_object *) self)->type;
-  PyObject *result;
-  PyObject *low_bound = NULL, *high_bound = NULL;
   /* Initialize these to appease GCC warnings.  */
   LONGEST low = 0, high = 0;
 
@@ -628,35 +602,22 @@ typy_range (PyObject *self, PyObject *args)
       break;
     }
 
-  low_bound = PyLong_FromLong (low);
-  if (!low_bound)
-    goto failarg;
-
-  high_bound = PyLong_FromLong (high);
-  if (!high_bound)
-    goto failarg;
+  gdbpy_reference low_bound (PyLong_FromLong (low));
+  if (low_bound == NULL)
+    return NULL;
 
-  result = PyTuple_New (2);
-  if (!result)
-    goto failarg;
+  gdbpy_reference high_bound (PyLong_FromLong (high));
+  if (high_bound == NULL)
+    return NULL;
 
-  if (PyTuple_SetItem (result, 0, low_bound) != 0)
-    {
-      Py_DECREF (result);
-      goto failarg;
-    }
-  if (PyTuple_SetItem (result, 1, high_bound) != 0)
-    {
-      Py_DECREF (high_bound);
-      Py_DECREF (result);
-      return NULL;
-    }
-  return result;
+  gdbpy_reference result (PyTuple_New (2));
+  if (result == NULL)
+    return NULL;
 
- failarg:
-  Py_XDECREF (high_bound);
-  Py_XDECREF (low_bound);
-  return NULL;
+  if (PyTuple_SetItem (result.get (), 0, low_bound.release ()) != 0
+      || PyTuple_SetItem (result.get (), 1, high_bound.release ()) != 0)
+    return NULL;
+  return result.release ();
 }
 
 /* Return a Type object which represents a reference to SELF.  */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 12/14] Use gdbpy_reference in python.c
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
  2016-11-07  5:48 ` [RFA 06/14] Use gdbpy_reference in gdbpy_inferiors Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:48 ` [RFA 14/14] Use gdbpy_reference in gdbpy_lookup_symbol Tom Tromey
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes a couple of functions in python.c to use gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/python.c (gdbpy_progspaces, gdbpy_objfiles): Use
	gdbpy_reference.
---
 gdb/ChangeLog       |  5 +++++
 gdb/python/python.c | 29 +++++++++++------------------
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1b49750..f5392a3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/python.c (gdbpy_progspaces, gdbpy_objfiles): Use
+	gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-prettyprint.c (search_pp_list)
 	(find_pretty_printer_from_objfiles)
 	(find_pretty_printer_from_progspace)
diff --git a/gdb/python/python.c b/gdb/python/python.c
index d6bd6bf..221bb4d 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -99,6 +99,7 @@ const struct extension_language_defn extension_language_python =
 #include "gdbthread.h"
 #include "interps.h"
 #include "event-top.h"
+#include "py-ref.h"
 
 /* True if Python has been successfully initialized, false
    otherwise.  */
@@ -1276,24 +1277,20 @@ static PyObject *
 gdbpy_progspaces (PyObject *unused1, PyObject *unused2)
 {
   struct program_space *ps;
-  PyObject *list;
 
-  list = PyList_New (0);
-  if (!list)
+  gdbpy_reference list (PyList_New (0));
+  if (list == NULL)
     return NULL;
 
   ALL_PSPACES (ps)
   {
     PyObject *item = pspace_to_pspace_object (ps);
 
-    if (!item || PyList_Append (list, item) == -1)
-      {
-	Py_DECREF (list);
-	return NULL;
-      }
+    if (!item || PyList_Append (list.get (), item) == -1)
+      return NULL;
   }
 
-  return list;
+  return list.release ();
 }
 
 \f
@@ -1376,24 +1373,20 @@ static PyObject *
 gdbpy_objfiles (PyObject *unused1, PyObject *unused2)
 {
   struct objfile *objf;
-  PyObject *list;
 
-  list = PyList_New (0);
-  if (!list)
+  gdbpy_reference list (PyList_New (0));
+  if (list == NULL)
     return NULL;
 
   ALL_OBJFILES (objf)
   {
     PyObject *item = objfile_to_objfile_object (objf);
 
-    if (!item || PyList_Append (list, item) == -1)
-      {
-	Py_DECREF (list);
-	return NULL;
-      }
+    if (!item || PyList_Append (list.get (), item) == -1)
+      return NULL;
   }
 
-  return list;
+  return list.release ();
 }
 
 /* Compute the list of active python type printers and store them in
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 13/14] Use gdbpy_reference in py-value.c
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (4 preceding siblings ...)
  2016-11-07  5:48 ` [RFA 04/14] Use gdbpy_reference in gdbpy_string_to_argv Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:48 ` [RFA 10/14] Use gdbpy_reference in call_doc_function Tom Tromey
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes a few functions in py-value.c to use gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-value.c (value_has_field, get_field_flag)
	(get_field_type, valpy_getitem, convert_value_from_python): Use
	gdbpy_reference.
---
 gdb/ChangeLog         |  6 ++++++
 gdb/python/py-value.c | 44 +++++++++++++++-----------------------------
 2 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f5392a3..56fadac 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-value.c (value_has_field, get_field_flag)
+	(get_field_type, valpy_getitem, convert_value_from_python): Use
+	gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/python.c (gdbpy_progspaces, gdbpy_objfiles): Use
 	gdbpy_reference.
 
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 46683b8..7bca136 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -29,6 +29,7 @@
 #include "python.h"
 
 #include "python-internal.h"
+#include "py-ref.h"
 
 /* Even though Python scalar types directly map to host types, we use
    target types here to remain consistent with the values system in
@@ -578,14 +579,13 @@ value_has_field (struct value *v, PyObject *field)
 {
   struct type *parent_type, *val_type;
   enum type_code type_code;
-  PyObject *type_object = PyObject_GetAttrString (field, "parent_type");
+  gdbpy_reference type_object (PyObject_GetAttrString (field, "parent_type"));
   int has_field = 0;
 
   if (type_object == NULL)
     return -1;
 
-  parent_type = type_object_to_type (type_object);
-  Py_DECREF (type_object);
+  parent_type = type_object_to_type (type_object.get ());
   if (parent_type == NULL)
     {
       PyErr_SetString (PyExc_TypeError,
@@ -625,16 +625,12 @@ value_has_field (struct value *v, PyObject *field)
 static int
 get_field_flag (PyObject *field, const char *flag_name)
 {
-  int flag_value;
-  PyObject *flag_object = PyObject_GetAttrString (field, flag_name);
+  gdbpy_reference flag_object (PyObject_GetAttrString (field, flag_name));
 
   if (flag_object == NULL)
     return -1;
 
-  flag_value = PyObject_IsTrue (flag_object);
-  Py_DECREF (flag_object);
-
-  return flag_value;
+  return PyObject_IsTrue (flag_object.get ());
 }
 
 /* Return the "type" attribute of a gdb.Field object.
@@ -643,13 +639,12 @@ get_field_flag (PyObject *field, const char *flag_name)
 static struct type *
 get_field_type (PyObject *field)
 {
-  PyObject *ftype_obj = PyObject_GetAttrString (field, "type");
+  gdbpy_reference ftype_obj (PyObject_GetAttrString (field, "type"));
   struct type *ftype;
 
   if (ftype_obj == NULL)
     return NULL;
-  ftype = type_object_to_type (ftype_obj);
-  Py_DECREF (ftype_obj);
+  ftype = type_object_to_type (ftype_obj.get ());
   if (ftype == NULL)
     PyErr_SetString (PyExc_TypeError,
 		     _("'type' attribute of gdb.Field object is not a "
@@ -705,25 +700,19 @@ valpy_getitem (PyObject *self, PyObject *key)
 	}
       else
 	{
-	  PyObject *name_obj = PyObject_GetAttrString (key, "name");
+	  gdbpy_reference name_obj (PyObject_GetAttrString (key, "name"));
 
 	  if (name_obj == NULL)
 	    return NULL;
 
 	  if (name_obj != Py_None)
 	    {
-	      field = python_string_to_host_string (name_obj);
-	      Py_DECREF (name_obj);
+	      field = python_string_to_host_string (name_obj.get ());
 	      if (field == NULL)
 		return NULL;
 	    }
 	  else
 	    {
-	      PyObject *bitpos_obj;
-	      int valid;
-
-	      Py_DECREF (name_obj);
-
 	      if (!PyObject_HasAttrString (key, "bitpos"))
 		{
 		  PyErr_SetString (PyExc_AttributeError,
@@ -732,12 +721,11 @@ valpy_getitem (PyObject *self, PyObject *key)
 
 		  return NULL;
 		}
-	      bitpos_obj = PyObject_GetAttrString (key, "bitpos");
+	      gdbpy_reference bitpos_obj (PyObject_GetAttrString (key,
+								  "bitpos"));
 	      if (bitpos_obj == NULL)
 		return NULL;
-	      valid = gdb_py_int_as_long (bitpos_obj, &bitpos);
-	      Py_DECREF (bitpos_obj);
-	      if (!valid)
+	      if (!gdb_py_int_as_long (bitpos_obj.get (), &bitpos))
 		return NULL;
 
 	      field_type = get_field_type (key);
@@ -1621,13 +1609,13 @@ convert_value_from_python (PyObject *obj)
 	         ULONGEST instead.  */
 	      if (PyErr_ExceptionMatches (PyExc_OverflowError))
 		{
-		  PyObject *etype, *evalue, *etraceback, *zero;
+		  PyObject *etype, *evalue, *etraceback;
 
 		  PyErr_Fetch (&etype, &evalue, &etraceback);
-		  zero = PyInt_FromLong (0);
+		  gdbpy_reference zero (PyInt_FromLong (0));
 
 		  /* Check whether obj is positive.  */
-		  if (PyObject_RichCompareBool (obj, zero, Py_GT) > 0)
+		  if (PyObject_RichCompareBool (obj, zero.get (), Py_GT) > 0)
 		    {
 		      ULONGEST ul;
 
@@ -1638,8 +1626,6 @@ convert_value_from_python (PyObject *obj)
 		  else
 		    /* There's nothing we can do.  */
 		    PyErr_Restore (etype, evalue, etraceback);
-
-		  Py_DECREF (zero);
 		}
 	    }
 	  else
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 14/14] Use gdbpy_reference in gdbpy_lookup_symbol
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
  2016-11-07  5:48 ` [RFA 06/14] Use gdbpy_reference in gdbpy_inferiors Tom Tromey
  2016-11-07  5:48 ` [RFA 12/14] Use gdbpy_reference in python.c Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:48 ` [RFA 01/14] Introduce py-ref.h Tom Tromey
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes gdbpy_lookup_symbol to use gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-symbol.c (gdbpy_lookup_symbol): Use gdbpy_reference.
---
 gdb/ChangeLog          |  4 ++++
 gdb/python/py-symbol.c | 18 ++++++++----------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 56fadac..6499a27 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-symbol.c (gdbpy_lookup_symbol): Use gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-value.c (value_has_field, get_field_flag)
 	(get_field_type, valpy_getitem, convert_value_from_python): Use
 	gdbpy_reference.
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index e0569d2..b0d92b2 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -23,6 +23,7 @@
 #include "symtab.h"
 #include "python-internal.h"
 #include "objfiles.h"
+#include "py-ref.h"
 
 typedef struct sympy_symbol_object {
   PyObject_HEAD
@@ -374,7 +375,7 @@ gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw)
   const char *name;
   static char *keywords[] = { "name", "block", "domain", NULL };
   struct symbol *symbol = NULL;
-  PyObject *block_obj = NULL, *ret_tuple, *sym_obj, *bool_obj;
+  PyObject *block_obj = NULL, *sym_obj, *bool_obj;
   const struct block *block = NULL;
 
   if (! PyArg_ParseTupleAndKeywords (args, kw, "s|O!i", keywords, &name,
@@ -410,31 +411,28 @@ gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw)
     }
   END_CATCH
 
-  ret_tuple = PyTuple_New (2);
-  if (!ret_tuple)
+  gdbpy_reference ret_tuple (PyTuple_New (2));
+  if (ret_tuple == NULL)
     return NULL;
 
   if (symbol)
     {
       sym_obj = symbol_to_symbol_object (symbol);
       if (!sym_obj)
-	{
-	  Py_DECREF (ret_tuple);
-	  return NULL;
-	}
+	return NULL;
     }
   else
     {
       sym_obj = Py_None;
       Py_INCREF (Py_None);
     }
-  PyTuple_SET_ITEM (ret_tuple, 0, sym_obj);
+  PyTuple_SET_ITEM (ret_tuple.get (), 0, sym_obj);
 
   bool_obj = (is_a_field_of_this.type != NULL) ? Py_True : Py_False;
   Py_INCREF (bool_obj);
-  PyTuple_SET_ITEM (ret_tuple, 1, bool_obj);
+  PyTuple_SET_ITEM (ret_tuple.get (), 1, bool_obj);
 
-  return ret_tuple;
+  return ret_tuple.release ();
 }
 
 /* Implementation of
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 06/14] Use gdbpy_reference in gdbpy_inferiors
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:48 ` [RFA 12/14] Use gdbpy_reference in python.c Tom Tromey
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes gdbpy_inferiors to use gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-inferior.c (gdbpy_inferiors): Use gdbpy_reference.
---
 gdb/ChangeLog            |  4 ++++
 gdb/python/py-inferior.c | 18 +++++-------------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8271499..164b568 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-inferior.c (gdbpy_inferiors): Use gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-function.c (convert_values_to_python, fnpy_init): Use
 	gdbpy_reference.
 
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 8732b87..b343c0f 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -477,22 +477,14 @@ build_inferior_list (struct inferior *inf, void *arg)
 PyObject *
 gdbpy_inferiors (PyObject *unused, PyObject *unused2)
 {
-  PyObject *list, *tuple;
-
-  list = PyList_New (0);
-  if (!list)
+  gdbpy_reference list (PyList_New (0));
+  if (list == NULL)
     return NULL;
 
-  if (iterate_over_inferiors (build_inferior_list, list))
-    {
-      Py_DECREF (list);
-      return NULL;
-    }
-
-  tuple = PyList_AsTuple (list);
-  Py_DECREF (list);
+  if (iterate_over_inferiors (build_inferior_list, list.get ()))
+    return NULL;
 
-  return tuple;
+  return PyList_AsTuple (list.get ());
 }
 
 /* Membuf and memory manipulation.  */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 05/14] Use gdbpy_reference in py-function.c
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (8 preceding siblings ...)
  2016-11-07  5:48 ` [RFA 11/14] Use gdbpy_reference in py-prettyprint.c Tom Tromey
@ 2016-11-07  5:48 ` Tom Tromey
  2016-11-07  5:48 ` [RFA 07/14] Use gdbpy_reference in gdbpy_breakpoints Tom Tromey
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes some code in py-function.c to use gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-function.c (convert_values_to_python, fnpy_init): Use
	gdbpy_reference.
---
 gdb/ChangeLog            |  5 +++++
 gdb/python/py-function.c | 27 +++++++++++----------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1a0a21f..8271499 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-function.c (convert_values_to_python, fnpy_init): Use
+	gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-cmd.c (gdbpy_string_to_argv): Use gdbpy_reference.
 
 2016-11-06  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/py-function.c b/gdb/python/py-function.c
index d42dbde..eb7b3c7 100644
--- a/gdb/python/py-function.c
+++ b/gdb/python/py-function.c
@@ -27,6 +27,7 @@
 #include "completer.h"
 #include "expression.h"
 #include "language.h"
+#include "py-ref.h"
 
 extern PyTypeObject fnpy_object_type
     CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("PyObject");
@@ -37,22 +38,19 @@ static PyObject *
 convert_values_to_python (int argc, struct value **argv)
 {
   int i;
-  PyObject *result = PyTuple_New (argc);
+  gdbpy_reference result (PyTuple_New (argc));
 
-  if (! result)
+  if (result == NULL)
     return NULL;
 
   for (i = 0; i < argc; ++i)
     {
-      PyObject *elt = value_to_value_object (argv[i]);
-      if (! elt)
-	{
-	  Py_DECREF (result);
-	  return NULL;
-	}
-      PyTuple_SetItem (result, i, elt);
+      gdbpy_reference elt (value_to_value_object (argv[i]));
+      if (elt == NULL)
+	return NULL;
+      PyTuple_SetItem (result.get (), i, elt.release ());
     }
-  return result;
+  return result.release ();
 }
 
 /* Call a Python function object's invoke method.  */
@@ -173,21 +171,18 @@ fnpy_init (PyObject *self, PyObject *args, PyObject *kwds)
 
   if (PyObject_HasAttrString (self, "__doc__"))
     {
-      PyObject *ds_obj = PyObject_GetAttrString (self, "__doc__");
+      gdbpy_reference ds_obj (PyObject_GetAttrString (self, "__doc__"));
       if (ds_obj != NULL)
 	{
-	  if (gdbpy_is_string (ds_obj))
+	  if (gdbpy_is_string (ds_obj.get ()))
 	    {
-	      docstring = python_string_to_host_string (ds_obj);
+	      docstring = python_string_to_host_string (ds_obj.get ());
 	      if (docstring == NULL)
 		{
 		  Py_DECREF (self);
-		  Py_DECREF (ds_obj);
 		  return -1;
 		}
 	    }
-
-	  Py_DECREF (ds_obj);
 	}
     }
   if (! docstring)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 02/14] Change event code to use gdbpy_reference
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (11 preceding siblings ...)
  2016-11-07  5:48 ` [RFA 03/14] Use gdbpy_reference in py-type.c Tom Tromey
@ 2016-11-07  5:56 ` Tom Tromey
  2016-11-11  0:09   ` Pedro Alves
  2016-11-07  5:57 ` [RFA 09/14] Use gdbpy_reference in py-linetable.c Tom Tromey
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes the event code in the Python layer to use
gdbpy_reference, simplifying the logic in many places.

It also changes evpy_emit_event not to steal a reference to its
argument.  This is simpler to do now that gdbpy_reference is in use;
it's also a reasonable cleanup in its own right.  While doing this I
realized that evpy_emit_event should not be calling gdbpy_print_stack
(all the outermost callers do this if needed), so I removed this as
well.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-threadevent.c (create_thread_event_object): Use
	gdbpy_reference.
	* python/py-stopevent.c (create_stop_event_object): Simplify.
	(emit_stop_event): Use gdbpy_reference.
	* python/py-signalevent.c (create_signal_event_object): Use
	gdbpy_reference.
	* python/py-newobjfileevent.c (create_new_objfile_event_object)
	(emit_new_objfile_event, create_clear_objfiles_event_object)
	(emit_clear_objfiles_event): Use gdbpy_reference.
	* python/py-infevents.c (create_inferior_call_event_object)
	(create_register_changed_event_object)
	(create_memory_changed_event_object, emit_inferior_call_event)
	(emit_memory_changed_event, emit_register_changed_event): Use
	gdbpy_reference.
	* python/py-exitedevent.c (create_exited_event_object)
	(emit_exited_event): Use gdbpy_reference.
	* python/py-event.h (evpy_emit_event): Remove
	CPYCHECKER_STEALS_REFERENCE_TO_ARG annotation.
	* python/py-event.c (evpy_emit_event): Use gdbpy_reference.
	* python/py-continueevent.c (emit_continue_event): Use
	gdbpy_reference.
	* python/py-breakpoint.c (gdbpy_breakpoint_created)
	(gdbpy_breakpoint_deleted, gdbpy_breakpoint_modified): Use
	gdbpy_reference.
	* python/py-bpevent.c (create_breakpoint_event_object): Use
	gdbpy_reference.
---
 gdb/ChangeLog                   |  29 +++++++++
 gdb/python/py-bpevent.c         |  23 +++----
 gdb/python/py-breakpoint.c      |   7 +-
 gdb/python/py-continueevent.c   |   9 ++-
 gdb/python/py-event.c           |  29 +++------
 gdb/python/py-event.h           |   4 +-
 gdb/python/py-exitedevent.c     |  50 +++++---------
 gdb/python/py-infevents.c       | 141 ++++++++++++----------------------------
 gdb/python/py-newobjfileevent.c |  52 ++++++---------
 gdb/python/py-signalevent.c     |  27 +++-----
 gdb/python/py-stopevent.c       |  54 ++++++---------
 gdb/python/py-threadevent.c     |  19 ++----
 12 files changed, 172 insertions(+), 272 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b46e26a..a827e37 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,34 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-threadevent.c (create_thread_event_object): Use
+	gdbpy_reference.
+	* python/py-stopevent.c (create_stop_event_object): Simplify.
+	(emit_stop_event): Use gdbpy_reference.
+	* python/py-signalevent.c (create_signal_event_object): Use
+	gdbpy_reference.
+	* python/py-newobjfileevent.c (create_new_objfile_event_object)
+	(emit_new_objfile_event, create_clear_objfiles_event_object)
+	(emit_clear_objfiles_event): Use gdbpy_reference.
+	* python/py-infevents.c (create_inferior_call_event_object)
+	(create_register_changed_event_object)
+	(create_memory_changed_event_object, emit_inferior_call_event)
+	(emit_memory_changed_event, emit_register_changed_event): Use
+	gdbpy_reference.
+	* python/py-exitedevent.c (create_exited_event_object)
+	(emit_exited_event): Use gdbpy_reference.
+	* python/py-event.h (evpy_emit_event): Remove
+	CPYCHECKER_STEALS_REFERENCE_TO_ARG annotation.
+	* python/py-event.c (evpy_emit_event): Use gdbpy_reference.
+	* python/py-continueevent.c (emit_continue_event): Use
+	gdbpy_reference.
+	* python/py-breakpoint.c (gdbpy_breakpoint_created)
+	(gdbpy_breakpoint_deleted, gdbpy_breakpoint_modified): Use
+	gdbpy_reference.
+	* python/py-bpevent.c (create_breakpoint_event_object): Use
+	gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-ref.h: New file.
 
 2016-10-29  Manish Goregaokar  <manish@mozilla.com>
diff --git a/gdb/python/py-bpevent.c b/gdb/python/py-bpevent.c
index f5ff2ef..787f927 100644
--- a/gdb/python/py-bpevent.c
+++ b/gdb/python/py-bpevent.c
@@ -19,6 +19,7 @@
 
 #include "defs.h"
 #include "py-stopevent.h"
+#include "py-ref.h"
 
 extern PyTypeObject breakpoint_event_object_type
     CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("event_object");
@@ -29,26 +30,22 @@ extern PyTypeObject breakpoint_event_object_type
 PyObject *
 create_breakpoint_event_object (PyObject *breakpoint_list, PyObject *first_bp)
 {
-  PyObject *breakpoint_event_obj =
-      create_stop_event_object (&breakpoint_event_object_type);
+  gdbpy_reference breakpoint_event_obj
+    (create_stop_event_object (&breakpoint_event_object_type));
 
-  if (!breakpoint_event_obj)
-    goto fail;
+  if (breakpoint_event_obj == NULL)
+    return NULL;
 
-  if (evpy_add_attribute (breakpoint_event_obj,
+  if (evpy_add_attribute (breakpoint_event_obj.get (),
                           "breakpoint",
                           first_bp) < 0)
-    goto fail;
-  if (evpy_add_attribute (breakpoint_event_obj,
+    return NULL;
+  if (evpy_add_attribute (breakpoint_event_obj.get (),
                           "breakpoints",
                           breakpoint_list) < 0)
-    goto fail;
+    return NULL;
 
-  return breakpoint_event_obj;
-
- fail:
-  Py_XDECREF (breakpoint_event_obj);
-  return NULL;
+  return breakpoint_event_obj.release ();
 }
 
 GDBPY_NEW_EVENT_TYPE (breakpoint,
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 80f5d1f..656b13a 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -919,7 +919,6 @@ gdbpy_breakpoint_created (struct breakpoint *bp)
 
   if (!evregpy_no_listeners_p (gdb_py_events.breakpoint_created))
     {
-      Py_INCREF (newbp);
       if (evpy_emit_event ((PyObject *) newbp,
 			   gdb_py_events.breakpoint_created) < 0)
 	gdbpy_print_stack ();
@@ -947,10 +946,7 @@ gdbpy_breakpoint_deleted (struct breakpoint *b)
 	{
 	  if (!evregpy_no_listeners_p (gdb_py_events.breakpoint_deleted))
 	    {
-	      PyObject *bp_obj_alias = (PyObject *) bp_obj;
-
-	      Py_INCREF (bp_obj_alias);
-	      if (evpy_emit_event (bp_obj_alias,
+	      if (evpy_emit_event ((PyObject *) bp_obj,
 				   gdb_py_events.breakpoint_deleted) < 0)
 		gdbpy_print_stack ();
 	    }
@@ -982,7 +978,6 @@ gdbpy_breakpoint_modified (struct breakpoint *b)
 	{
 	  if (!evregpy_no_listeners_p (gdb_py_events.breakpoint_modified))
 	    {
-	      Py_INCREF (bp_obj);
 	      if (evpy_emit_event (bp_obj,
 				   gdb_py_events.breakpoint_modified) < 0)
 		gdbpy_print_stack ();
diff --git a/gdb/python/py-continueevent.c b/gdb/python/py-continueevent.c
index 2f21524..5bd0b39 100644
--- a/gdb/python/py-continueevent.c
+++ b/gdb/python/py-continueevent.c
@@ -19,6 +19,7 @@
 
 #include "defs.h"
 #include "py-event.h"
+#include "py-ref.h"
 
 extern PyTypeObject continue_event_object_type
     CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("event_object");
@@ -36,14 +37,12 @@ create_continue_event_object (void)
 int
 emit_continue_event (ptid_t ptid)
 {
-  PyObject *event;
-
   if (evregpy_no_listeners_p (gdb_py_events.cont))
     return 0;
 
-  event = create_continue_event_object ();
-  if (event)
-    return evpy_emit_event (event, gdb_py_events.cont);
+  gdbpy_reference event (create_continue_event_object ());
+  if (event != NULL)
+    return evpy_emit_event (event.get (), gdb_py_events.cont);
   return -1;
 }
 
diff --git a/gdb/python/py-event.c b/gdb/python/py-event.c
index 0dd94b4..c973337 100644
--- a/gdb/python/py-event.c
+++ b/gdb/python/py-event.c
@@ -89,26 +89,25 @@ int
 evpy_emit_event (PyObject *event,
                  eventregistry_object *registry)
 {
-  PyObject *callback_list_copy = NULL;
   Py_ssize_t i;
 
   /* Create a copy of call back list and use that for
      notifying listeners to avoid skipping callbacks
      in the case of a callback being disconnected during
      a notification.  */
-  callback_list_copy = PySequence_List (registry->callbacks);
-  if (!callback_list_copy)
-    goto fail;
+  gdbpy_reference callback_list_copy (PySequence_List (registry->callbacks));
+  if (callback_list_copy == NULL)
+    return -1;
 
-  for (i = 0; i < PyList_Size (callback_list_copy); i++)
+  for (i = 0; i < PyList_Size (callback_list_copy.get ()); i++)
     {
-      PyObject *func = PyList_GetItem (callback_list_copy, i);
-      PyObject *func_result;
+      PyObject *func = PyList_GetItem (callback_list_copy.get (), i);
 
       if (func == NULL)
-	goto fail;
+	return -1;
 
-      func_result = PyObject_CallFunctionObjArgs (func, event, NULL);
+      gdbpy_reference
+	func_result (PyObject_CallFunctionObjArgs (func, event, NULL));
 
       if (func_result == NULL)
 	{
@@ -116,21 +115,9 @@ evpy_emit_event (PyObject *event,
 	     call all of the callbacks even if one is broken.  */
 	  gdbpy_print_stack ();
 	}
-      else
-	{
-	  Py_DECREF (func_result);
-	}
     }
 
-  Py_XDECREF (callback_list_copy);
-  Py_XDECREF (event);
   return 0;
-
- fail:
-  gdbpy_print_stack ();
-  Py_XDECREF (callback_list_copy);
-  Py_XDECREF (event);
-  return -1;
 }
 
 static PyGetSetDef event_object_getset[] =
diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
index 998fc84..786a963 100644
--- a/gdb/python/py-event.h
+++ b/gdb/python/py-event.h
@@ -24,6 +24,7 @@
 #include "command.h"
 #include "python-internal.h"
 #include "inferior.h"
+#include "py-ref.h"
 
 /* This macro creates the following functions:
 
@@ -121,8 +122,7 @@ extern int emit_register_changed_event (struct frame_info *frame,
 				        int regnum);
 extern int emit_memory_changed_event (CORE_ADDR addr, ssize_t len);
 extern int evpy_emit_event (PyObject *event,
-                            eventregistry_object *registry)
-  CPYCHECKER_STEALS_REFERENCE_TO_ARG (1);
+                            eventregistry_object *registry);
 
 extern PyObject *create_event_object (PyTypeObject *py_type);
 extern PyObject *create_thread_event_object (PyTypeObject *py_type);
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 87eedf8..6000dee 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -26,42 +26,30 @@ extern PyTypeObject exited_event_object_type
 static PyObject *
 create_exited_event_object (const LONGEST *exit_code, struct inferior *inf)
 {
-  PyObject *exited_event;
-  PyObject *inf_obj = NULL;
+  gdbpy_reference exited_event
+    (create_event_object (&exited_event_object_type));
 
-  exited_event = create_event_object (&exited_event_object_type);
-
-  if (!exited_event)
-    goto fail;
+  if (exited_event == NULL)
+    return NULL;
 
   if (exit_code)
     {
-      PyObject *exit_code_obj = PyLong_FromLongLong (*exit_code);
-      int failed;
+      gdbpy_reference exit_code_obj (PyLong_FromLongLong (*exit_code));
 
       if (exit_code_obj == NULL)
-	goto fail;
-
-      failed = evpy_add_attribute (exited_event, "exit_code",
-				   exit_code_obj) < 0;
-      Py_DECREF (exit_code_obj);
-      if (failed)
-	goto fail;
+	return NULL;
+      if (evpy_add_attribute (exited_event.get (), "exit_code",
+			      exit_code_obj.get ()) < 0)
+	return NULL;
     }
 
-  inf_obj = inferior_to_inferior_object (inf);
-  if (!inf_obj || evpy_add_attribute (exited_event,
-                                      "inferior",
-                                      inf_obj) < 0)
-    goto fail;
-  Py_DECREF (inf_obj);
+  gdbpy_reference inf_obj (inferior_to_inferior_object (inf));
+  if (inf_obj == NULL || evpy_add_attribute (exited_event.get (),
+					     "inferior",
+					     inf_obj.get ()) < 0)
+    return NULL;
 
-  return exited_event;
-
- fail:
-  Py_XDECREF (inf_obj);
-  Py_XDECREF (exited_event);
-  return NULL;
+  return exited_event.release ();
 }
 
 /* Callback that is used when an exit event occurs.  This function
@@ -70,15 +58,13 @@ create_exited_event_object (const LONGEST *exit_code, struct inferior *inf)
 int
 emit_exited_event (const LONGEST *exit_code, struct inferior *inf)
 {
-  PyObject *event;
-
   if (evregpy_no_listeners_p (gdb_py_events.exited))
     return 0;
 
-  event = create_exited_event_object (exit_code, inf);
+  gdbpy_reference event (create_exited_event_object (exit_code, inf));
 
-  if (event)
-    return evpy_emit_event (event, gdb_py_events.exited);
+  if (event != NULL)
+    return evpy_emit_event (event.get (), gdb_py_events.exited);
 
   return -1;
 }
diff --git a/gdb/python/py-infevents.c b/gdb/python/py-infevents.c
index 33d994c..34b9dc9 100644
--- a/gdb/python/py-infevents.c
+++ b/gdb/python/py-infevents.c
@@ -19,6 +19,7 @@
 
 #include "defs.h"
 #include "py-event.h"
+#include "py-ref.h"
 
 extern PyTypeObject inferior_call_pre_event_object_type
     CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("event_object");
@@ -36,52 +37,36 @@ static PyObject *
 create_inferior_call_event_object (inferior_call_kind flag, ptid_t ptid,
 				   CORE_ADDR addr)
 {
-  PyObject *event;
-  PyObject *ptid_obj = NULL;
-  PyObject *addr_obj = NULL;
+  gdbpy_reference event;
   int failed;
-  struct cleanup *cleanups;
-  struct cleanup *member_cleanups;
 
   switch (flag)
     {
     case INFERIOR_CALL_PRE:
-      event = create_event_object (&inferior_call_pre_event_object_type);
+      event.reset (create_event_object (&inferior_call_pre_event_object_type));
       break;
     case INFERIOR_CALL_POST:
-      event = create_event_object (&inferior_call_post_event_object_type);
+      event.reset (create_event_object (&inferior_call_post_event_object_type));
       break;
     default:
-      return NULL;
+      gdb_assert_not_reached ("invalid inferior_call_kind");
     }
 
-  cleanups = make_cleanup_py_decref (event);
-
-  ptid_obj = gdbpy_create_ptid_object (ptid);
+  gdbpy_reference ptid_obj (gdbpy_create_ptid_object (ptid));
   if (ptid_obj == NULL)
-    goto fail;
-  member_cleanups = make_cleanup_py_decref (ptid_obj);
+    return NULL;
 
-  failed = evpy_add_attribute (event, "ptid", ptid_obj) < 0;
-  if (failed)
-    goto fail;
+  if (evpy_add_attribute (event.get (), "ptid", ptid_obj.get ()) < 0)
+    return NULL;
 
-  addr_obj = PyLong_FromLongLong (addr);
+  gdbpy_reference addr_obj (PyLong_FromLongLong (addr));
   if (addr_obj == NULL)
-    goto fail;
-  make_cleanup_py_decref (addr_obj);
-
-  failed = evpy_add_attribute (event, "address", addr_obj) < 0;
-  if (failed)
-    goto fail;
+    return NULL;
 
-  do_cleanups (member_cleanups);
-  discard_cleanups (cleanups);
-  return event;
+  if (evpy_add_attribute (event.get (), "address", addr_obj.get ()) < 0)
+    return NULL;
 
- fail:
-  do_cleanups (cleanups);
-  return NULL;
+  return event.release ();
 }
 
 /* Construct a gdb.RegisterChangedEvent containing the affected
@@ -91,44 +76,26 @@ static PyObject *
 create_register_changed_event_object (struct frame_info *frame, 
 				      int regnum)
 {
-  PyObject *event;
-  PyObject *frame_obj = NULL;
-  PyObject *regnum_obj = NULL;
-  int failed;
-  struct cleanup *cleanups;
-  struct cleanup *member_cleanups;
-
-  event = create_event_object (&register_changed_event_object_type);
+  gdbpy_reference event
+    (create_event_object (&register_changed_event_object_type));
   if (event == NULL)
     return NULL;
 
-  cleanups = make_cleanup_py_decref (event);
-
-  frame_obj = frame_info_to_frame_object (frame);
+  gdbpy_reference frame_obj (frame_info_to_frame_object (frame));
   if (frame_obj == NULL)
-    goto fail;
-  member_cleanups = make_cleanup_py_decref (frame_obj);
+    return NULL;
 
-  failed = evpy_add_attribute (event, "frame", frame_obj) < 0;
-  if (failed)
-    goto fail;
+  if (evpy_add_attribute (event.get (), "frame", frame_obj.get ()) < 0)
+    return NULL;
 
-  regnum_obj = PyLong_FromLongLong (regnum);
+  gdbpy_reference regnum_obj (PyLong_FromLongLong (regnum));
   if (regnum_obj == NULL)
-    goto fail;
-  make_cleanup_py_decref (regnum_obj);
-
-  failed = evpy_add_attribute (event, "regnum", regnum_obj) < 0;
-  if (failed)
-    goto fail;
+    return NULL;
 
-  do_cleanups (member_cleanups);
-  discard_cleanups (cleanups);
-  return event;
+  if (evpy_add_attribute (event.get (), "regnum", regnum_obj.get ()) < 0)
+    return NULL;
 
- fail:
-  do_cleanups (cleanups);
-  return NULL;
+  return event.release ();
 }
 
 /* Construct a gdb.MemoryChangedEvent describing the extent of the
@@ -137,44 +104,27 @@ create_register_changed_event_object (struct frame_info *frame,
 static PyObject *
 create_memory_changed_event_object (CORE_ADDR addr, ssize_t len)
 {
-  PyObject *event;
-  PyObject *addr_obj = NULL;
-  PyObject *len_obj = NULL;
-  int failed;
-  struct cleanup *cleanups;
-  struct cleanup *member_cleanups;
-
-  event = create_event_object (&memory_changed_event_object_type);
+  gdbpy_reference event
+    (create_event_object (&memory_changed_event_object_type));
 
   if (event == NULL)
     return NULL;
-  cleanups = make_cleanup_py_decref (event);
 
-  addr_obj = PyLong_FromLongLong (addr);
+  gdbpy_reference addr_obj (PyLong_FromLongLong (addr));
   if (addr_obj == NULL)
-    goto fail;
-  member_cleanups = make_cleanup_py_decref (addr_obj);
+    return NULL;
 
-  failed = evpy_add_attribute (event, "address", addr_obj) < 0;
-  if (failed)
-    goto fail;
+  if (evpy_add_attribute (event.get (), "address", addr_obj.get ()) < 0)
+    return NULL;
 
-  len_obj = PyLong_FromLong (len);
+  gdbpy_reference len_obj (PyLong_FromLong (len));
   if (len_obj == NULL)
-    goto fail;
-  make_cleanup_py_decref (len_obj);
-
-  failed = evpy_add_attribute (event, "length", len_obj) < 0;
-  if (failed)
-    goto fail;
+    return NULL;
 
-  do_cleanups (member_cleanups);
-  discard_cleanups (cleanups);
-  return event;
+  if (evpy_add_attribute (event.get (), "length", len_obj.get ()) < 0)
+    return NULL;
 
- fail:
-  do_cleanups (cleanups);
-  return NULL;
+  return event.release ();
 }
 
 /* Callback function which notifies observers when an event occurs which
@@ -186,14 +136,13 @@ int
 emit_inferior_call_event (inferior_call_kind flag, ptid_t thread,
 			  CORE_ADDR addr)
 {
-  PyObject *event;
-
   if (evregpy_no_listeners_p (gdb_py_events.inferior_call))
     return 0;
 
-  event = create_inferior_call_event_object (flag, thread, addr);
+  gdbpy_reference event (create_inferior_call_event_object (flag, thread,
+							    addr));
   if (event != NULL)
-    return evpy_emit_event (event, gdb_py_events.inferior_call);
+    return evpy_emit_event (event.get (), gdb_py_events.inferior_call);
   return -1;
 }
 
@@ -203,14 +152,12 @@ emit_inferior_call_event (inferior_call_kind flag, ptid_t thread,
 int
 emit_memory_changed_event (CORE_ADDR addr, ssize_t len)
 {
-  PyObject *event;
-
   if (evregpy_no_listeners_p (gdb_py_events.memory_changed))
     return 0;
 
-  event = create_memory_changed_event_object (addr, len);
+  gdbpy_reference event (create_memory_changed_event_object (addr, len));
   if (event != NULL)
-    return evpy_emit_event (event, gdb_py_events.memory_changed);
+    return evpy_emit_event (event.get (), gdb_py_events.memory_changed);
   return -1;
 }
 
@@ -220,14 +167,12 @@ emit_memory_changed_event (CORE_ADDR addr, ssize_t len)
 int
 emit_register_changed_event (struct frame_info* frame, int regnum)
 {
-  PyObject *event;
-
   if (evregpy_no_listeners_p (gdb_py_events.register_changed))
     return 0;
 
-  event = create_register_changed_event_object (frame, regnum);
+  gdbpy_reference event (create_register_changed_event_object (frame, regnum));
   if (event != NULL)
-    return evpy_emit_event (event, gdb_py_events.register_changed);
+    return evpy_emit_event (event.get (), gdb_py_events.register_changed);
   return -1;
 }
 
diff --git a/gdb/python/py-newobjfileevent.c b/gdb/python/py-newobjfileevent.c
index 3dd2430b..a96e4f0 100644
--- a/gdb/python/py-newobjfileevent.c
+++ b/gdb/python/py-newobjfileevent.c
@@ -28,26 +28,22 @@ extern PyTypeObject clear_objfiles_event_object_type
 static PyObject *
 create_new_objfile_event_object (struct objfile *objfile)
 {
-  PyObject *objfile_event;
   PyObject *py_objfile;
 
-  objfile_event = create_event_object (&new_objfile_event_object_type);
-  if (!objfile_event)
-    goto fail;
+  gdbpy_reference objfile_event
+    (create_event_object (&new_objfile_event_object_type));
+  if (objfile_event == NULL)
+    return NULL;
 
   /* Note that objfile_to_objfile_object returns a borrowed reference,
      so we don't need a decref here.  */
   py_objfile = objfile_to_objfile_object (objfile);
-  if (!py_objfile || evpy_add_attribute (objfile_event,
+  if (!py_objfile || evpy_add_attribute (objfile_event.get (),
                                          "new_objfile",
                                          py_objfile) < 0)
-    goto fail;
+    return NULL;
 
-  return objfile_event;
-
- fail:
-  Py_XDECREF (objfile_event);
-  return NULL;
+  return objfile_event.release ();
 }
 
 /* Callback function which notifies observers when a new objfile event occurs.
@@ -57,14 +53,12 @@ create_new_objfile_event_object (struct objfile *objfile)
 int
 emit_new_objfile_event (struct objfile *objfile)
 {
-  PyObject *event;
-
   if (evregpy_no_listeners_p (gdb_py_events.new_objfile))
     return 0;
 
-  event = create_new_objfile_event_object (objfile);
-  if (event)
-    return evpy_emit_event (event, gdb_py_events.new_objfile);
+  gdbpy_reference event (create_new_objfile_event_object (objfile));
+  if (event != NULL)
+    return evpy_emit_event (event.get (), gdb_py_events.new_objfile);
   return -1;
 }
 
@@ -79,26 +73,22 @@ GDBPY_NEW_EVENT_TYPE (new_objfile,
 static PyObject *
 create_clear_objfiles_event_object (void)
 {
-  PyObject *objfile_event;
   PyObject *py_progspace;
 
-  objfile_event = create_event_object (&clear_objfiles_event_object_type);
-  if (!objfile_event)
-    goto fail;
+  gdbpy_reference objfile_event
+    (create_event_object (&clear_objfiles_event_object_type));
+  if (objfile_event == NULL)
+    return NULL;
 
   /* Note that pspace_to_pspace_object returns a borrowed reference,
      so we don't need a decref here.  */
   py_progspace = pspace_to_pspace_object (current_program_space);
-  if (!py_progspace || evpy_add_attribute (objfile_event,
+  if (!py_progspace || evpy_add_attribute (objfile_event.get (),
 					   "progspace",
 					   py_progspace) < 0)
-    goto fail;
+    return NULL;
 
-  return objfile_event;
-
- fail:
-  Py_XDECREF (objfile_event);
-  return NULL;
+  return objfile_event.release ();
 }
 
 /* Callback function which notifies observers when the "clear objfiles"
@@ -109,14 +99,12 @@ create_clear_objfiles_event_object (void)
 int
 emit_clear_objfiles_event (void)
 {
-  PyObject *event;
-
   if (evregpy_no_listeners_p (gdb_py_events.clear_objfiles))
     return 0;
 
-  event = create_clear_objfiles_event_object ();
-  if (event)
-    return evpy_emit_event (event, gdb_py_events.clear_objfiles);
+  gdbpy_reference event (create_clear_objfiles_event_object ());
+  if (event != NULL)
+    return evpy_emit_event (event.get (), gdb_py_events.clear_objfiles);
   return -1;
 }
 
diff --git a/gdb/python/py-signalevent.c b/gdb/python/py-signalevent.c
index cd5f72c..d3d030a 100644
--- a/gdb/python/py-signalevent.c
+++ b/gdb/python/py-signalevent.c
@@ -27,30 +27,23 @@ PyObject *
 create_signal_event_object (enum gdb_signal stop_signal)
 {
   const char *signal_name;
-  PyObject *signal_name_obj = NULL;
-  PyObject *signal_event_obj =
-      create_stop_event_object (&signal_event_object_type);
+  gdbpy_reference signal_event_obj
+    (create_stop_event_object (&signal_event_object_type));
 
-  if (!signal_event_obj)
-    goto fail;
+  if (signal_event_obj == NULL)
+    return NULL;
 
   signal_name = gdb_signal_to_name (stop_signal);
 
-  signal_name_obj = PyString_FromString (signal_name);
+  gdbpy_reference signal_name_obj (PyString_FromString (signal_name));
   if (signal_name_obj == NULL)
-    goto fail;
-  if (evpy_add_attribute (signal_event_obj,
+    return NULL;
+  if (evpy_add_attribute (signal_event_obj.get (),
                           "stop_signal",
-                          signal_name_obj) < 0)
-    goto fail;
-  Py_DECREF (signal_name_obj);
+                          signal_name_obj.get ()) < 0)
+    return NULL;
 
-  return signal_event_obj;
-
- fail:
-  Py_XDECREF (signal_name_obj);
-  Py_XDECREF (signal_event_obj);
-  return NULL;
+  return signal_event_obj.release ();
 }
 
 GDBPY_NEW_EVENT_TYPE (signal,
diff --git a/gdb/python/py-stopevent.c b/gdb/python/py-stopevent.c
index 47592c1..0516012 100644
--- a/gdb/python/py-stopevent.c
+++ b/gdb/python/py-stopevent.c
@@ -23,16 +23,7 @@
 PyObject *
 create_stop_event_object (PyTypeObject *py_type)
 {
-  PyObject *stop_event_obj = create_thread_event_object (py_type);
-
-  if (!stop_event_obj)
-    goto fail;
-
-  return stop_event_obj;
-
-  fail:
-   Py_XDECREF (stop_event_obj);
-   return NULL;
+  return create_thread_event_object (py_type);
 }
 
 /* Callback observers when a stop event occurs.  This function will create a
@@ -45,8 +36,8 @@ create_stop_event_object (PyTypeObject *py_type)
 int
 emit_stop_event (struct bpstats *bs, enum gdb_signal stop_signal)
 {
-  PyObject *stop_event_obj = NULL; /* Appease GCC warning.  */
-  PyObject *list = NULL;
+  gdbpy_reference stop_event_obj;
+  gdbpy_reference list;
   PyObject *first_bp = NULL;
   struct bpstats *current_bs;
 
@@ -64,13 +55,13 @@ emit_stop_event (struct bpstats *bs, enum gdb_signal stop_signal)
 
           if (list == NULL)
             {
-              list = PyList_New (0);
-              if (!list)
-                goto fail;
+              list.reset (PyList_New (0));
+              if (list == NULL)
+		return -1;
             }
 
-          if (PyList_Append (list, current_py_bp))
-            goto fail;
+          if (PyList_Append (list.get (), current_py_bp))
+	    return -1;
 
           if (first_bp == NULL)
             first_bp = current_py_bp;
@@ -79,36 +70,31 @@ emit_stop_event (struct bpstats *bs, enum gdb_signal stop_signal)
 
   if (list != NULL)
     {
-      stop_event_obj = create_breakpoint_event_object (list, first_bp);
-      if (!stop_event_obj)
-        goto fail;
-      Py_DECREF (list);
+      stop_event_obj.reset (create_breakpoint_event_object (list.get (),
+							    first_bp));
+      if (stop_event_obj == NULL)
+	return -1;
     }
 
   /* Check if the signal is "Signal 0" or "Trace/breakpoint trap".  */
   if (stop_signal != GDB_SIGNAL_0
       && stop_signal != GDB_SIGNAL_TRAP)
     {
-      stop_event_obj =
-	  create_signal_event_object (stop_signal);
-      if (!stop_event_obj)
-	goto fail;
+      stop_event_obj.reset (create_signal_event_object (stop_signal));
+      if (stop_event_obj == NULL)
+	return -1;
     }
 
   /* If all fails emit an unknown stop event.  All event types should
      be known and this should eventually be unused.  */
-  if (!stop_event_obj)
+  if (stop_event_obj == NULL)
     {
-      stop_event_obj = create_stop_event_object (&stop_event_object_type);
-      if (!stop_event_obj)
-        goto fail;
+      stop_event_obj.reset (create_stop_event_object (&stop_event_object_type));
+      if (stop_event_obj == NULL)
+	return -1;
     }
 
-  return evpy_emit_event (stop_event_obj, gdb_py_events.stop);
-
- fail:
-  Py_XDECREF (list);
-  return -1;
+  return evpy_emit_event (stop_event_obj.get (), gdb_py_events.stop);
 }
 
 GDBPY_NEW_EVENT_TYPE (stop,
diff --git a/gdb/python/py-threadevent.c b/gdb/python/py-threadevent.c
index 53291d3..e4c0afb 100644
--- a/gdb/python/py-threadevent.c
+++ b/gdb/python/py-threadevent.c
@@ -51,26 +51,21 @@ PyObject *
 create_thread_event_object (PyTypeObject *py_type)
 {
   PyObject *thread = NULL;
-  PyObject *thread_event_obj = NULL;
 
-  thread_event_obj = create_event_object (py_type);
-  if (!thread_event_obj)
-    goto fail;
+  gdbpy_reference thread_event_obj (create_event_object (py_type));
+  if (thread_event_obj == NULL)
+    return NULL;
 
   thread = get_event_thread ();
   if (!thread)
-    goto fail;
+    return NULL;
 
-  if (evpy_add_attribute (thread_event_obj,
+  if (evpy_add_attribute (thread_event_obj.get (),
                           "inferior_thread",
                           thread) < 0)
-    goto fail;
+    return NULL;
 
-  return thread_event_obj;
-
-  fail:
-   Py_XDECREF (thread_event_obj);
-   return NULL;
+  return thread_event_obj.release ();
 }
 
 GDBPY_NEW_EVENT_TYPE (thread,
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 09/14] Use gdbpy_reference in py-linetable.c
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (12 preceding siblings ...)
  2016-11-07  5:56 ` [RFA 02/14] Change event code to use gdbpy_reference Tom Tromey
@ 2016-11-07  5:57 ` Tom Tromey
  2016-11-08  4:07 ` [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
  2016-11-11  1:18 ` Pedro Alves
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  5:57 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes some code in py-linetable.c to use gdbpy_reference.

2016-11-06  Tom Tromey  <tom@tromey.com>

	* python/py-linetable.c (build_line_table_tuple_from_pcs)
	(ltpy_get_all_source_lines): Use gdbpy_reference.
---
 gdb/ChangeLog             |  5 +++++
 gdb/python/py-linetable.c | 51 +++++++++++++----------------------------------
 2 files changed, 19 insertions(+), 37 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4921600..9aa7008 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-06  Tom Tromey  <tom@tromey.com>
 
+	* python/py-linetable.c (build_line_table_tuple_from_pcs)
+	(ltpy_get_all_source_lines): Use gdbpy_reference.
+
+2016-11-06  Tom Tromey  <tom@tromey.com>
+
 	* python/py-framefilter.c (extract_sym, extract_value)
 	(get_py_iter_from_func, bootstrap_python_frame_filters): Use
 	gdbpy_reference.
diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index 662ae88..8be7e0e 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -19,6 +19,7 @@
 
 #include "defs.h"
 #include "python-internal.h"
+#include "py-ref.h"
 
 typedef struct {
   PyObject_HEAD
@@ -124,7 +125,6 @@ static PyObject *
 build_line_table_tuple_from_pcs (int line, VEC (CORE_ADDR) *vec)
 {
   int vec_len = 0;
-  PyObject *tuple;
   CORE_ADDR pc;
   int i;
 
@@ -132,31 +132,22 @@ build_line_table_tuple_from_pcs (int line, VEC (CORE_ADDR) *vec)
   if (vec_len < 1)
     Py_RETURN_NONE;
 
-  tuple = PyTuple_New (vec_len);
+  gdbpy_reference tuple (PyTuple_New (vec_len));
 
   if (tuple == NULL)
     return NULL;
 
   for (i = 0; VEC_iterate (CORE_ADDR, vec, i, pc); ++i)
     {
-      PyObject *obj = build_linetable_entry (line, pc);
+      gdbpy_reference obj (build_linetable_entry (line, pc));
 
       if (obj == NULL)
-	{
-	  Py_DECREF (tuple);
-	  tuple = NULL;
-	  break;
-	}
-      else if (PyTuple_SetItem (tuple, i, obj) != 0)
-	{
-	  Py_DECREF (obj);
-	  Py_DECREF (tuple);
-	  tuple = NULL;
-	  break;
-	}
+	return NULL;
+      else if (PyTuple_SetItem (tuple.get (), i, obj.release ()) != 0)
+	return NULL;
     }
 
-  return tuple;
+  return tuple.release ();
 }
 
 /* Implementation of gdb.LineTable.line (self) -> Tuple.  Returns a
@@ -236,7 +227,6 @@ ltpy_get_all_source_lines (PyObject *self, PyObject *args)
 {
   struct symtab *symtab;
   Py_ssize_t index;
-  PyObject *source_list, *source_dict, *line;
   struct linetable_entry *item;
 
   LTPY_REQUIRE_VALID (self, symtab);
@@ -248,7 +238,7 @@ ltpy_get_all_source_lines (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  source_dict = PyDict_New ();
+  gdbpy_reference source_dict (PyDict_New ());
   if (source_dict == NULL)
     return NULL;
 
@@ -260,30 +250,17 @@ ltpy_get_all_source_lines (PyObject *self, PyObject *args)
 	 include in the source set. */
       if (item->line > 0)
 	{
-	  line = gdb_py_object_from_longest (item->line);
+	  gdbpy_reference line (gdb_py_object_from_longest (item->line));
 
 	  if (line == NULL)
-	    {
-	      Py_DECREF (source_dict);
-	      return NULL;
-	    }
-
-	  if (PyDict_SetItem (source_dict, line, Py_None) == -1)
-	    {
-	      Py_DECREF (line);
-	      Py_DECREF (source_dict);
-	      return NULL;
-	    }
-
-	  Py_DECREF (line);
+	    return NULL;
+
+	  if (PyDict_SetItem (source_dict.get (), line.get (), Py_None) == -1)
+	    return NULL;
 	}
     }
 
-
-  source_list = PyDict_Keys (source_dict);
-  Py_DECREF (source_dict);
-
-  return source_list;
+  return PyDict_Keys (source_dict.get ());
 }
 
 /* Implementation of gdb.LineTable.is_valid (self) -> Boolean.
-- 
2.7.4

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFA 00/14] add a smart pointer for PyObject*
@ 2016-11-07  6:00 Tom Tromey
  2016-11-07  5:48 ` [RFA 06/14] Use gdbpy_reference in gdbpy_inferiors Tom Tromey
                   ` (15 more replies)
  0 siblings, 16 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07  6:00 UTC (permalink / raw)
  To: gdb-patches

This patch introduces a new smart pointer that manages a Python
reference, and then changes parts of the Python layer to use this new
wrapper.

Dealing with reference counting in the Python layer has been a
consistent source of bugs.  It's relatively easy to introduce a memory
leak, especially when dealing with an error case.  This new class
simplifies typical Python-facing code, reducing the chance of such
errors.

Many spots in gdb's Python layer deal with error-handling by "goto"
with a "fail" label; this series removes many such "goto"s.  This
series provides other sorts of cleanups as well.

I avoided introducing changes when an ensure_python_env cleanup was in
use locally; throwing an exception across one of these when a
gdbpy_reference is in used could change the order of operations,
resulting in a Py_DECREF when the GIL is not held.  I'm not sure
whether this happens in practice but it seemed best not to find out.

This is a net code reduction even including the new code and the
ChangeLog entries.

In the longer run I'd like to remove make_cleanup_py_decref and
ensure_python_env.

I've built on x86-64 Fedora 24 and run the gdb.python tests locally.
I'm also currently sending this through the buildbot.

Tom

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 01/14] Introduce py-ref.h
  2016-11-07  5:48 ` [RFA 01/14] Introduce py-ref.h Tom Tromey
@ 2016-11-07  7:45   ` Jan Kratochvil
  2016-11-07 15:48     ` Tom Tromey
  2016-11-10 23:48   ` Pedro Alves
  1 sibling, 1 reply; 30+ messages in thread
From: Jan Kratochvil @ 2016-11-07  7:45 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Mon, 07 Nov 2016 06:47:23 +0100, Tom Tromey wrote:
> +  /* Copy another instance.  */
> +  gdbpy_reference &operator= (const gdbpy_reference &other)
> +  {
> +    Py_XDECREF (m_obj);
> +    m_obj = other.m_obj;
> +    if (m_obj != NULL)
> +      Py_INCREF (m_obj);
> +    return *this;
> +  }
> +
> +  /* Transfer ownership from another instance.  */
> +  gdbpy_reference &operator= (gdbpy_reference &&other)
> +  {
> +    Py_XDECREF (m_obj);
> +    m_obj = other.m_obj;
> +    other.m_obj = NULL;
> +    return *this;
> +  }

These two methods stale-reference / deallocate during self-assignments:
  gdbpyref = gdbpyref;
  gdbpyref = std::move (gdbpyref);

std::unique_ptr<> and std::shared_ptr<> behave correctly in such cases.


Jan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 01/14] Introduce py-ref.h
  2016-11-07  7:45   ` Jan Kratochvil
@ 2016-11-07 15:48     ` Tom Tromey
  0 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-07 15:48 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Tom Tromey, gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> These two methods stale-reference / deallocate during self-assignments:
Jan>   gdbpyref = gdbpyref;
Jan>   gdbpyref = std::move (gdbpyref);

Jan> std::unique_ptr<> and std::shared_ptr<> behave correctly in such cases.

I'll fix them.
Or maybe remove them, I can't remember if I ended up needing these.

Tom

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 00/14] add a smart pointer for PyObject*
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (13 preceding siblings ...)
  2016-11-07  5:57 ` [RFA 09/14] Use gdbpy_reference in py-linetable.c Tom Tromey
@ 2016-11-08  4:07 ` Tom Tromey
  2016-11-11  1:18 ` Pedro Alves
  15 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-08  4:07 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> I've built on x86-64 Fedora 24 and run the gdb.python tests locally.
Tom> I'm also currently sending this through the buildbot.

The buildbot run was clean as well.

Tom

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 01/14] Introduce py-ref.h
  2016-11-07  5:48 ` [RFA 01/14] Introduce py-ref.h Tom Tromey
  2016-11-07  7:45   ` Jan Kratochvil
@ 2016-11-10 23:48   ` Pedro Alves
  2016-11-12 17:31     ` Tom Tromey
  1 sibling, 1 reply; 30+ messages in thread
From: Pedro Alves @ 2016-11-10 23:48 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 11/07/2016 05:47 AM, Tom Tromey wrote:

> +/* An instance of this class holds a reference to a PyObject.  When
> +   the object is destroyed, the PyObject is decref'd.
> +
> +   Normally an instance is constructed using a PyObject*.  This sort
> +   of initialization lets this class manage the lifetime of that
> +   reference.
> +
> +   Assignment and copy construction will make a new reference as
> +   appropriate.  Assignment from a plain PyObject* is disallowed to
> +   avoid confusion about whether this acquires a new reference;
> +   instead use the "reset" method -- which, like the PyObject*
> +   constructor, transfers ownership.

I think this should say something about supporting explicitly
wrapping a NULL PyObject *, and how to check whether there's
a managed object.

> +*/
> +class gdbpy_reference
> +{
> + public:
> +
> +  /* Create a new NULL instance.  */
> +  gdbpy_reference ()
> +    : m_obj (NULL)
> +  {
> +  }
> +
> +  /* Create a new NULL instance.  */
> +  gdbpy_reference (nullptr_t)
> +    : m_obj (NULL)
> +  {
> +  }

I don't think this overload is needed, since there's no
ambiguity without it.  If you don't provide it, initializing
with nullptr selects the one below, with the same result.

Otherwise, shouldn't this overload be explicit too?

> +
> +  /* Create a new instance.  OBJ is a reference, management of which
> +     is now transferred to this class.  */
> +  explicit gdbpy_reference (PyObject *obj)
> +    : m_obj (obj)
> +  {
> +  }

> +  /* Equality.  */
> +  bool operator== (const PyObject *other) const
> +  {
> +    return m_obj == other;
> +  }
> +
> +  /* Inequality.  */
> +  bool operator!= (const PyObject *other) const
> +  {
> +    return m_obj != other;
> +  }

This supports:

  gdbpy_reference == PyObject *

Seems to me we should support these too:

  gdbpy_reference == gdbpy_reference
  PyObject *      == gdbpy_reference

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 02/14] Change event code to use gdbpy_reference
  2016-11-07  5:56 ` [RFA 02/14] Change event code to use gdbpy_reference Tom Tromey
@ 2016-11-11  0:09   ` Pedro Alves
  2016-11-11  0:51     ` Pedro Alves
  0 siblings, 1 reply; 30+ messages in thread
From: Pedro Alves @ 2016-11-11  0:09 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 11/07/2016 05:47 AM, Tom Tromey wrote:
>  PyObject *
>  create_breakpoint_event_object (PyObject *breakpoint_list, PyObject *first_bp)
>  {
> -  PyObject *breakpoint_event_obj =
> -      create_stop_event_object (&breakpoint_event_object_type);
> +  gdbpy_reference breakpoint_event_obj
> +    (create_stop_event_object (&breakpoint_event_object_type));
>  

> - fail:
> -  Py_XDECREF (breakpoint_event_obj);
> -  return NULL;
> +  return breakpoint_event_obj.release ();
>  }

Wouldn't it better to make these create_foo functions return a
gdbpy_reference ?  Then at the call site you'd use std::move instead
of reset (when assigning to an existing lval):

      stop_event_obj = std::move (create_breakpoint_event_object (list.get (),
							          first_bp));

The advantage of following such a principle is that it makes it impossible
for callers to forget to manage the result with a gdbpy_reference, since
assigning the return to a raw PyObject * won't compile.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 02/14] Change event code to use gdbpy_reference
  2016-11-11  0:09   ` Pedro Alves
@ 2016-11-11  0:51     ` Pedro Alves
  0 siblings, 0 replies; 30+ messages in thread
From: Pedro Alves @ 2016-11-11  0:51 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 11/11/2016 12:09 AM, Pedro Alves wrote:
> On 11/07/2016 05:47 AM, Tom Tromey wrote:
>>  PyObject *
>>  create_breakpoint_event_object (PyObject *breakpoint_list, PyObject *first_bp)
>>  {
>> -  PyObject *breakpoint_event_obj =
>> -      create_stop_event_object (&breakpoint_event_object_type);
>> +  gdbpy_reference breakpoint_event_obj
>> +    (create_stop_event_object (&breakpoint_event_object_type));
>>  
> 
>> - fail:
>> -  Py_XDECREF (breakpoint_event_obj);
>> -  return NULL;
>> +  return breakpoint_event_obj.release ();
>>  }
> 
> Wouldn't it better to make these create_foo functions return a
> gdbpy_reference ?  Then at the call site you'd use std::move instead
> of reset (when assigning to an existing lval):
> 
>       stop_event_obj = std::move (create_breakpoint_event_object (list.get (),
> 							          first_bp));

Err, not even std::move would be necessary, of course, 
since the function's return would be an rvalue in this context already.

So you'd just do:

  stop_event_obj = create_breakpoint_event_object (list.get (), first_bp));

That could be done after this series lands, in any case.

> The advantage of following such a principle is that it makes it impossible
> for callers to forget to manage the result with a gdbpy_reference, since
> assigning the return to a raw PyObject * won't compile.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 00/14] add a smart pointer for PyObject*
  2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
                   ` (14 preceding siblings ...)
  2016-11-08  4:07 ` [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
@ 2016-11-11  1:18 ` Pedro Alves
  2016-11-11  3:34   ` Tom Tromey
  2016-11-12 17:11   ` Tom Tromey
  15 siblings, 2 replies; 30+ messages in thread
From: Pedro Alves @ 2016-11-11  1:18 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

Hi Tom,

Great to see this finally happen.  Congrats.  :-)

I read the whole series now, and it all looks great to me.  Once
patch #1 is fixed, it's fine with me to put it all in.

I did wonder about shortening the name to "gdbpy_ref".  That's how I end
up reading "gdbpy_reference" in my mind after seeing it so many
times.  :-)  You already picked a shorter name as header file
name -- py-ref.h -- and that's typed way less often.  :-)
Anyway, certainly not very important.  Just mentioning in case
you had already considered but didn't know what others would think.

I wonder whether it'd be desirable to add more methods to 
gdbpy_reference, so you'd have code like:

  gdbpy_reference sort_func = module.GetAttrString ("execute_frame_filters");

instead of:

  gdbpy_reference sort_func (PyObject_GetAttrString (module.get (),
						     "execute_frame_filters"));

etc.

And maybe add some C++ classes that inherit from gdbpy_reference for
specific Python types (e.g python tuple or list), which would provide
methods that would only make sense for those types.

Did you consider things like these?  Or maybe you did but thought
they'd obfuscate?

Also, somewhat related, I briefly looked at making our custom Python
types C++ classes before, so they could themselves hold other
C++ classes, std::string, etc., though I didn't find how to
coerce Python to use operator new with PyTypeObject types.
There must be some way though.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 00/14] add a smart pointer for PyObject*
  2016-11-11  1:18 ` Pedro Alves
@ 2016-11-11  3:34   ` Tom Tromey
  2016-11-11  4:03     ` Pedro Alves
  2016-11-12 17:11   ` Tom Tromey
  1 sibling, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2016-11-11  3:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I did wonder about shortening the name to "gdbpy_ref".  That's how I end
Pedro> up reading "gdbpy_reference" in my mind after seeing it so many
Pedro> times.  :-)  You already picked a shorter name as header file
Pedro> name -- py-ref.h -- and that's typed way less often.  :-)
Pedro> Anyway, certainly not very important.  Just mentioning in case
Pedro> you had already considered but didn't know what others would think.

I wouldn't mind.  I just erred on the side of verbosity.

Pedro> I wonder whether it'd be desirable to add more methods to 
Pedro> gdbpy_reference, so you'd have code like:

Pedro>   gdbpy_reference sort_func = module.GetAttrString ("execute_frame_filters");

Pedro> instead of:

Pedro>   gdbpy_reference sort_func (PyObject_GetAttrString (module.get (),
Pedro> 						     "execute_frame_filters"));

Pedro> etc.

This particular one I chose not to do, on the theory that assignment
from another gdbpy_reference does a Py_INCREF.  That is, this addition
would mean that one assignment operator claims ownership of an existing
reference, while another assignment operator would create a new
reference.

So, to avoid the confusion I made the PyObject* constructor explicit,
and I required the use of .reset(...) to claim ownership.

I'm open to basically any spelling of any of this.  I just thought this
approach was most likely to avoid bugs.

Pedro> And maybe add some C++ classes that inherit from gdbpy_reference for
Pedro> specific Python types (e.g python tuple or list), which would provide
Pedro> methods that would only make sense for those types.

Pedro> Did you consider things like these?  Or maybe you did but thought
Pedro> they'd obfuscate?

I did consider subclassing and also adding some convenience methods.
Other similar ideas came to mind as well, like providing wrapper
functions for all the Python APIs to try to ensure ownership sanity --
basically implementing David Malcolm's reference-checker work as a bunch
of templates or so.

I do think this would help; while the current approach definitely
improves the situation, it also leaves some bug-prone behavior.  For
example, PyTuple_SET_ITEM steals a reference, so calls end up looking
like:

  PyTuple_SET_ITEM (py_arg_tuple.get (), 0, py_value_obj.release ());

... using .get() rather than .release() would cause a bug (probably a
crash).

If we had a wrapper we could make calls look like:

  mumble_tuple_set_item (py_arg_tuple, 0, py_value_obj);

... with the first argument being unwrapped using .get() and the third
with an implicit incref (or require std::move there).  (An equivalent
using member functions could also be done.)


Anyway, I didn't do this just because it requires more thought; and
there were a lot of patches already; and I figured that it could be done
later if desirable.

Pedro> Also, somewhat related, I briefly looked at making our custom Python
Pedro> types C++ classes before, so they could themselves hold other
Pedro> C++ classes, std::string, etc., though I didn't find how to
Pedro> coerce Python to use operator new with PyTypeObject types.
Pedro> There must be some way though.

I haven't looked at all into prior art for C++ Python APIs; which might
be worth doing.  One "easy" way to do it would be to store a pointer to
a C++ object which would be "delete"d in the Python object-destruction
callback (gross due to extra allocation of course).  Another possible
way would be placement new.  But maybe there's something even nicer.

Tom

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 00/14] add a smart pointer for PyObject*
  2016-11-11  3:34   ` Tom Tromey
@ 2016-11-11  4:03     ` Pedro Alves
  2016-11-11  5:49       ` Tom Tromey
  0 siblings, 1 reply; 30+ messages in thread
From: Pedro Alves @ 2016-11-11  4:03 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 11/11/2016 03:34 AM, Tom Tromey wrote:

> Pedro>   gdbpy_reference sort_func = module.GetAttrString ("execute_frame_filters");
> 
> Pedro> instead of:
> 
> Pedro>   gdbpy_reference sort_func (PyObject_GetAttrString (module.get (),
> Pedro> 						     "execute_frame_filters"));
> 
> Pedro> etc.
> 
> This particular one I chose not to do, on the theory that assignment
> from another gdbpy_reference does a Py_INCREF.  That is, this addition
> would mean that one assignment operator claims ownership of an existing
> reference, while another assignment operator would create a new
> reference.

I think that theory is incorrect.  If module.GetAttrString() returns a 
gdbpy_reference, then RVO kicks in and no copy is made at all.  If RVO
doesn't kick in, because the compiler fails to optimize, then it's the
move constructor that is called, since the return of
module.GetAttrString() is an rvalue in this context.

Likewise, when assigning to an existing variable:

 gdbpy_reference sort_func;

 ...

 sort_func = module.GetAttrString ("execute_frame_filters");

then it's the move assignment operator that is called.

Thus there should be no increfs here at all.

No chance of introducing bugs.  I had that in mind here too,
but didn't spell it out, sorry about that.


This is like returning std::unique_ptr from functions (including
C++14 std::make_unique):

  extern std::unique_ptr<...> make_something ();

  std::unique_ptr<...> foo = make_something ();

The assignment only works because that's actually a move.  Returning
std::unique_ptr makes sure that you don't try to assign the result of
the function call to a raw pointer directly.


The copy assignment operator is only called when you copy from
an lvalue, like e.g.:

 gdbpy_reference ref1, ref2;
 ...
 ref1 = ref2;

And in this case clearly you do want to incref.

There's another safety advantage to having functions return
gdbpy_references directly.  Consider usages like:

  if (module.GetAttrString ("execute_frame_filters") == NULL)

(I made that up, I assume there are cases like this, but I
didn't check.)

If GetAttrString returns a PyObject *, then this leaks.  If instead
it returns a gdbpy_reference, it does not.

> Anyway, I didn't do this just because it requires more thought; and
> there were a lot of patches already; and I figured that it could be done
> later if desirable.

I hope I didn't came across as suggesting you'd do this now.

> 
> Pedro> Also, somewhat related, I briefly looked at making our custom Python
> Pedro> types C++ classes before, so they could themselves hold other
> Pedro> C++ classes, std::string, etc., though I didn't find how to
> Pedro> coerce Python to use operator new with PyTypeObject types.
> Pedro> There must be some way though.
> 
> I haven't looked at all into prior art for C++ Python APIs; which might
> be worth doing.  One "easy" way to do it would be to store a pointer to
> a C++ object which would be "delete"d in the Python object-destruction
> callback (gross due to extra allocation of course).  Another possible
> way would be placement new.  But maybe there's something even nicer.

Yeah, definitely worth looking at what other APIs are doing.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 00/14] add a smart pointer for PyObject*
  2016-11-11  4:03     ` Pedro Alves
@ 2016-11-11  5:49       ` Tom Tromey
  0 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-11  5:49 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

> gdbpy_reference sort_func = module.GetAttrString
> ("execute_frame_filters");

Pedro> I think that theory is incorrect.  If module.GetAttrString() returns a 
Pedro> gdbpy_reference, then RVO kicks in and no copy is made at all.

What happened is that I misread your example as just changing an
initializer to an assignment -- not seeing that in fact the first one
was also positing that the function would be returning a
gdbpy_reference.  Which I should have realized since you explicitly said
that somewhere...

Pedro> There's another safety advantage to having functions return
Pedro> gdbpy_references directly.

I completely agree.

Tom

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 00/14] add a smart pointer for PyObject*
  2016-11-11  1:18 ` Pedro Alves
  2016-11-11  3:34   ` Tom Tromey
@ 2016-11-12 17:11   ` Tom Tromey
  1 sibling, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2016-11-12 17:11 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I read the whole series now, and it all looks great to me.  Once
Pedro> patch #1 is fixed, it's fine with me to put it all in.

Pedro> I did wonder about shortening the name to "gdbpy_ref".

I went ahead and did this.

Tom

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 01/14] Introduce py-ref.h
  2016-11-10 23:48   ` Pedro Alves
@ 2016-11-12 17:31     ` Tom Tromey
  2016-11-15 14:32       ` Pedro Alves
  0 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2016-11-12 17:31 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I think this should say something about supporting explicitly
Pedro> wrapping a NULL PyObject *, and how to check whether there's
Pedro> a managed object.
[... other review comments...]

Here's a new version of this patch, which I think addresses all the
comments, from both you and others.

Tom

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 99edafc..da23621 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
+	* python/py-ref.h: New file.
+
 2016-11-11  Yao Qi  <yao.qi@linaro.org>
 
 	* spu-tdep.c (spu_software_single_step): Don't call
diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
new file mode 100644
index 0000000..ae3b5cc
--- /dev/null
+++ b/gdb/python/py-ref.h
@@ -0,0 +1,158 @@
+/* Python reference-holding class
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_PYTHON_REF_H
+#define GDB_PYTHON_REF_H
+
+/* An instance of this class either holds a reference to a PyObject,
+   or is "NULL".  If it holds a reference, then when the object is
+   destroyed, the PyObject is decref'd.
+
+   Normally an instance is constructed using a PyObject*.  This sort
+   of initialization lets this class manage the lifetime of that
+   reference.
+
+   Assignment and copy construction will make a new reference as
+   appropriate.  Assignment from a plain PyObject* is disallowed to
+   avoid confusion about whether this acquires a new reference;
+   instead use the "reset" method -- which, like the PyObject*
+   constructor, transfers ownership.
+*/
+class gdbpy_ref
+{
+ public:
+
+  /* Create a new NULL instance.  */
+  gdbpy_ref ()
+    : m_obj (NULL)
+  {
+  }
+
+  /* Create a new instance.  OBJ is a reference, management of which
+     is now transferred to this class.  */
+  explicit gdbpy_ref (PyObject *obj)
+    : m_obj (obj)
+  {
+  }
+
+  /* Copy another instance.  */
+  gdbpy_ref (const gdbpy_ref &other)
+    : m_obj (other.m_obj)
+  {
+    if (m_obj != NULL)
+      Py_INCREF (m_obj);
+  }
+
+  /* Transfer ownership from OTHER.  */
+  gdbpy_ref (gdbpy_ref &&other)
+    : m_obj (other.m_obj)
+  {
+    other.m_obj = NULL;
+  }
+
+  /* Destroy this instance.  */
+  ~gdbpy_ref ()
+  {
+    Py_XDECREF (m_obj);
+  }
+
+  /* Copy another instance.  */
+  gdbpy_ref &operator= (const gdbpy_ref &other)
+  {
+    /* Do nothing on self-assignment.  */
+    if (this != &other)
+      {
+	Py_XDECREF (m_obj);
+	m_obj = other.m_obj;
+	if (m_obj != NULL)
+	  Py_INCREF (m_obj);
+      }
+    return *this;
+  }
+
+  /* Transfer ownership from OTHER.  */
+  gdbpy_ref &operator= (gdbpy_ref &&other)
+  {
+    /* Note that this handles self-assignment properly.  */
+    std::swap (m_obj, other.m_obj);
+    return *this;
+  }
+
+  /* Change this instance's referent.  OBJ is a reference, management
+     of which is now transferred to this class.  */
+  void reset (PyObject *obj)
+  {
+    Py_XDECREF (m_obj);
+    m_obj = obj;
+  }
+
+  /* Return this instance's referent.  In Python terms this is a
+     borrowed pointer.  */
+  PyObject *get () const
+  {
+    return m_obj;
+  }
+
+  /* Return this instance's referent, and stop managing this
+     reference.  The caller is now responsible for the ownership of
+     the reference.  */
+  PyObject *release ()
+  {
+    PyObject *result = m_obj;
+
+    m_obj = NULL;
+    return result;
+  }
+
+ private:
+
+  PyObject *m_obj;
+};
+
+inline bool operator== (const gdbpy_ref &self, const gdbpy_ref &other)
+{
+  return self.get () == other.get ();
+}
+
+inline bool operator== (const gdbpy_ref &self, const PyObject *other)
+{
+  return self.get () == other;
+}
+
+inline bool operator== (const PyObject *self, const gdbpy_ref &other)
+{
+  return self == other.get ();
+}
+
+inline bool operator!= (const gdbpy_ref &self, const gdbpy_ref &other)
+{
+  return self.get () != other.get ();
+}
+
+inline bool operator!= (const gdbpy_ref &self, const PyObject *other)
+{
+  return self.get () != other;
+}
+
+inline bool operator!= (const PyObject *self, const gdbpy_ref &other)
+{
+  return self != other.get ();
+}
+
+#endif /* GDB_PYTHON_REF_H */

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 01/14] Introduce py-ref.h
  2016-11-12 17:31     ` Tom Tromey
@ 2016-11-15 14:32       ` Pedro Alves
  2016-11-16 23:18         ` Tom Tromey
  0 siblings, 1 reply; 30+ messages in thread
From: Pedro Alves @ 2016-11-15 14:32 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Hi Tom,

On 11/12/2016 05:31 PM, Tom Tromey wrote:

> +
> +  /* Transfer ownership from OTHER.  */
> +  gdbpy_ref &operator= (gdbpy_ref &&other)
> +  {
> +    /* Note that this handles self-assignment properly.  */
> +    std::swap (m_obj, other.m_obj);

Since there's no need to worry about noexcept garantees here
(destructing a gdbpy_ref can't throw), I'd rather that
move would not be implemented as a swap, as that extends the
lifetime of m_obj in OTHER.

I.e., bugs like these will go unnoticed longer:

gdbpy_ref bar = make_whatever (....);
gdbpy_ref foo = make_whatever (....);

if (condition)
{
  bar = std::move (foo);
  some_function (foo.get ()); // whoops, passed bar.get () instead of NULL and crashing early.
}

I'd be happy with the idiom of implementing move-in-term-of-swap
by first move-constructing, and then swapping that temporary:

  gdbpy_ref &operator= (gdbpy_ref &&other)
  {
    gdbpy_ref (std::move (other)).swap (*this);
    return *this;
  }

Or perhaps simpler, go with something like what you had in the
first version:

+  /* Transfer ownership from another instance.  */
+  gdbpy_reference &operator= (gdbpy_reference &&other)
+  {
+    Py_XDECREF (m_obj);
+    m_obj = other.m_obj;
+    other.m_obj = NULL;
+    return *this;
+  }
+

But add a "if (this != &other)" check.  I'd tweak it to be
in terms of reset() while at it:

  /* Transfer ownership from another instance.  */
  gdbpy_reference &operator= (gdbpy_reference &&other)
  {
    if (this != &other)
      {
        reset (other.m_obj);
        other.m_obj = NULL;
      }
    return *this;
  }

Otherwise LGTM.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 01/14] Introduce py-ref.h
  2016-11-15 14:32       ` Pedro Alves
@ 2016-11-16 23:18         ` Tom Tromey
  2016-11-16 23:34           ` Pedro Alves
  0 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2016-11-16 23:18 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Since there's no need to worry about noexcept garantees here
Pedro> (destructing a gdbpy_ref can't throw), I'd rather that
Pedro> move would not be implemented as a swap, as that extends the
Pedro> lifetime of m_obj in OTHER.
[...]
Pedro> Or perhaps simpler, go with something like what you had in the
Pedro> first version:

I made this change.

Pedro> But add a "if (this != &other)" check.  I'd tweak it to be
Pedro> in terms of reset() while at it:

I also changed the other operator= to use reset(), and I changed the
code to use Py_XINCREF rather than a NULL check.

Tom

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFA 01/14] Introduce py-ref.h
  2016-11-16 23:18         ` Tom Tromey
@ 2016-11-16 23:34           ` Pedro Alves
  0 siblings, 0 replies; 30+ messages in thread
From: Pedro Alves @ 2016-11-16 23:34 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 11/16/2016 11:18 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> Since there's no need to worry about noexcept garantees here
> Pedro> (destructing a gdbpy_ref can't throw), I'd rather that
> Pedro> move would not be implemented as a swap, as that extends the
> Pedro> lifetime of m_obj in OTHER.
> [...]
> Pedro> Or perhaps simpler, go with something like what you had in the
> Pedro> first version:
> 
> I made this change.
> 
> Pedro> But add a "if (this != &other)" check.  I'd tweak it to be
> Pedro> in terms of reset() while at it:
> 
> I also changed the other operator= to use reset(), and I changed the
> code to use Py_XINCREF rather than a NULL check.

Sounds good.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2016-11-16 23:34 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-07  6:00 [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
2016-11-07  5:48 ` [RFA 06/14] Use gdbpy_reference in gdbpy_inferiors Tom Tromey
2016-11-07  5:48 ` [RFA 12/14] Use gdbpy_reference in python.c Tom Tromey
2016-11-07  5:48 ` [RFA 14/14] Use gdbpy_reference in gdbpy_lookup_symbol Tom Tromey
2016-11-07  5:48 ` [RFA 01/14] Introduce py-ref.h Tom Tromey
2016-11-07  7:45   ` Jan Kratochvil
2016-11-07 15:48     ` Tom Tromey
2016-11-10 23:48   ` Pedro Alves
2016-11-12 17:31     ` Tom Tromey
2016-11-15 14:32       ` Pedro Alves
2016-11-16 23:18         ` Tom Tromey
2016-11-16 23:34           ` Pedro Alves
2016-11-07  5:48 ` [RFA 04/14] Use gdbpy_reference in gdbpy_string_to_argv Tom Tromey
2016-11-07  5:48 ` [RFA 13/14] Use gdbpy_reference in py-value.c Tom Tromey
2016-11-07  5:48 ` [RFA 10/14] Use gdbpy_reference in call_doc_function Tom Tromey
2016-11-07  5:48 ` [RFA 08/14] Use gdbpy_reference in py-framefilter.c Tom Tromey
2016-11-07  5:48 ` [RFA 11/14] Use gdbpy_reference in py-prettyprint.c Tom Tromey
2016-11-07  5:48 ` [RFA 05/14] Use gdbpy_reference in py-function.c Tom Tromey
2016-11-07  5:48 ` [RFA 07/14] Use gdbpy_reference in gdbpy_breakpoints Tom Tromey
2016-11-07  5:48 ` [RFA 03/14] Use gdbpy_reference in py-type.c Tom Tromey
2016-11-07  5:56 ` [RFA 02/14] Change event code to use gdbpy_reference Tom Tromey
2016-11-11  0:09   ` Pedro Alves
2016-11-11  0:51     ` Pedro Alves
2016-11-07  5:57 ` [RFA 09/14] Use gdbpy_reference in py-linetable.c Tom Tromey
2016-11-08  4:07 ` [RFA 00/14] add a smart pointer for PyObject* Tom Tromey
2016-11-11  1:18 ` Pedro Alves
2016-11-11  3:34   ` Tom Tromey
2016-11-11  4:03     ` Pedro Alves
2016-11-11  5:49       ` Tom Tromey
2016-11-12 17:11   ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).