[RFA 01/13] Use gdbpy_ref in archpy_disassemble

[RFA 01/13] Use gdbpy_ref in archpy_disassemble

[Tom Tromey]

This changes archpy_disassemble to use gdbpy_ref.  It also fixes a
latent bug where archpy_disassemble was decref'ing the results of a
all to PyArg_ParseTupleAndKeywords.  This is incorrect because
PyArg_ParseTupleAndKeywords returns borrowed references.

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

	* python/py-arch.c (archpy_disassemble): Use gdbpy_ref.  Don't
	decref results of PyArg_ParseTupleAndKeywords.
---
 gdb/ChangeLog        |  5 +++++
 gdb/python/py-arch.c | 29 +++++++++--------------------
 2 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 49c226b..527053f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
+	* python/py-arch.c (archpy_disassemble): Use gdbpy_ref.  Don't
+	decref results of PyArg_ParseTupleAndKeywords.
+
 2016-11-12  Tom Tromey  <tom@tromey.com>
 
 	* python/python.c (python_run_simple_file): Use
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index 60cc5a9..216a668 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -22,6 +22,7 @@
 #include "arch-utils.h"
 #include "disasm.h"
 #include "python-internal.h"
+#include "py-ref.h"
 
 typedef struct arch_object_type_object {
   PyObject_HEAD
@@ -120,7 +121,7 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
   CORE_ADDR pc;
   gdb_py_ulongest start_temp;
   long count = 0, i;
-  PyObject *result_list, *end_obj = NULL, *count_obj = NULL;
+  PyObject *end_obj = NULL, *count_obj = NULL;
   struct gdbarch *gdbarch = NULL;
 
   ARCHPY_REQUIRE_VALID (self, gdbarch);
@@ -149,8 +150,6 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
 #endif
       else
         {
-          Py_DECREF (end_obj);
-          Py_XDECREF (count_obj);
           PyErr_SetString (PyExc_TypeError,
                            _("Argument 'end_pc' should be a (long) integer."));
 
@@ -159,8 +158,6 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
 
       if (end < start)
         {
-          Py_DECREF (end_obj);
-          Py_XDECREF (count_obj);
           PyErr_SetString (PyExc_ValueError,
                            _("Argument 'end_pc' should be greater than or "
                              "equal to the argument 'start_pc'."));
@@ -173,8 +170,6 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
       count = PyInt_AsLong (count_obj);
       if (PyErr_Occurred () || count < 0)
         {
-          Py_DECREF (count_obj);
-          Py_XDECREF (end_obj);
           PyErr_SetString (PyExc_TypeError,
                            _("Argument 'count' should be an non-negative "
                              "integer."));
@@ -183,7 +178,7 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
         }
     }
 
-  result_list = PyList_New (0);
+  gdbpy_ref result_list (PyList_New (0));
   if (result_list == NULL)
     return NULL;
 
@@ -199,19 +194,16 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
     {
       int insn_len = 0;
       struct ui_file *memfile = mem_fileopen ();
-      PyObject *insn_dict = PyDict_New ();
+      gdbpy_ref insn_dict (PyDict_New ());
 
       if (insn_dict == NULL)
         {
-          Py_DECREF (result_list);
           ui_file_delete (memfile);
 
           return NULL;
         }
-      if (PyList_Append (result_list, insn_dict))
+      if (PyList_Append (result_list.get (), insn_dict.get ()))
         {
-          Py_DECREF (result_list);
-          Py_DECREF (insn_dict);
           ui_file_delete (memfile);
 
           return NULL;  /* PyList_Append Sets the exception.  */
@@ -223,7 +215,6 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
         }
       CATCH (except, RETURN_MASK_ALL)
         {
-          Py_DECREF (result_list);
           ui_file_delete (memfile);
 
 	  gdbpy_convert_exception (except);
@@ -233,17 +224,15 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
 
       std::string as = ui_file_as_string (memfile);
 
-      if (PyDict_SetItemString (insn_dict, "addr",
+      if (PyDict_SetItemString (insn_dict.get (), "addr",
                                 gdb_py_long_from_ulongest (pc))
-          || PyDict_SetItemString (insn_dict, "asm",
+          || PyDict_SetItemString (insn_dict.get (), "asm",
                                    PyString_FromString (!as.empty ()
 							? as.c_str ()
 							: "<unknown>"))
-          || PyDict_SetItemString (insn_dict, "length",
+          || PyDict_SetItemString (insn_dict.get (), "length",
                                    PyInt_FromLong (insn_len)))
         {
-          Py_DECREF (result_list);
-
           ui_file_delete (memfile);
 
           return NULL;
@@ -254,7 +243,7 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
       ui_file_delete (memfile);
     }
 
-  return result_list;
+  return result_list.release ();
 }
 
 /* Initializes the Architecture class in the gdb module.  */
-- 
2.7.4

[RFA 02/13] Use gdbpy_ref in gdbpy_breakpoint_cond_says_stop

[Tom Tromey]

This changes gdbpy_breakpoint_cond_says_stop to use gdbpy_ref rather
than explicit reference management.

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

	* python/py-breakpoint.c (gdbpy_breakpoint_cond_says_stop): Use
	gdbpy_ref.
---
 gdb/ChangeLog              | 5 +++++
 gdb/python/py-breakpoint.c | 8 +++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 527053f..ebb4d71 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-breakpoint.c (gdbpy_breakpoint_cond_says_stop): Use
+	gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-arch.c (archpy_disassemble): Use gdbpy_ref.  Don't
 	decref results of PyArg_ParseTupleAndKeywords.
 
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 198669b..418b637 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -805,12 +805,12 @@ gdbpy_breakpoint_cond_says_stop (const struct extension_language_defn *extlang,
 
   if (PyObject_HasAttrString (py_bp, stop_func))
     {
-      PyObject *result = PyObject_CallMethod (py_bp, stop_func, NULL);
+      gdbpy_ref result (PyObject_CallMethod (py_bp, stop_func, NULL));
 
       stop = 1;
-      if (result)
+      if (result != NULL)
 	{
-	  int evaluate = PyObject_IsTrue (result);
+	  int evaluate = PyObject_IsTrue (result.get ());
 
 	  if (evaluate == -1)
 	    gdbpy_print_stack ();
@@ -819,8 +819,6 @@ gdbpy_breakpoint_cond_says_stop (const struct extension_language_defn *extlang,
 	     the Python breakpoint wants GDB to continue.  */
 	  if (! evaluate)
 	    stop = 0;
-
-	  Py_DECREF (result);
 	}
       else
 	gdbpy_print_stack ();
-- 
2.7.4

[RFA 05/13] Use gdbpy_ref in py_print_frame

[Tom Tromey]

This changes py_print_frame to use gdbpy_ref in more places.

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

	* python/py-framefilter.c (py_print_frame): Use gdbpy_ref.
---
 gdb/ChangeLog               |  4 ++++
 gdb/python/py-framefilter.c | 14 ++++----------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 83ad836..545dbde 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-framefilter.c (py_print_frame): Use gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-finishbreakpoint.c (bpfinishpy_out_of_scope): Use
 	gdbpy_ref.
 
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index c0c7cde..ff1a068 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1009,7 +1009,6 @@ py_print_frame (PyObject *filter, int flags,
   struct frame_info *frame = NULL;
   struct cleanup *cleanup_stack;
   struct value_print_options opts;
-  PyObject *py_inf_frame;
   int print_level, print_frame_info, print_args, print_locals;
   gdb::unique_xmalloc_ptr<char> function_to_free;
 
@@ -1024,14 +1023,11 @@ py_print_frame (PyObject *filter, int flags,
   /* Get the underlying frame.  This is needed to determine GDB
   architecture, and also, in the cases of frame variables/arguments to
   read them if they returned filter object requires us to do so.  */
-  py_inf_frame = PyObject_CallMethod (filter, "inferior_frame", NULL);
+  gdbpy_ref py_inf_frame (PyObject_CallMethod (filter, "inferior_frame", NULL));
   if (py_inf_frame == NULL)
     return EXT_LANG_BT_ERROR;
 
-  frame = frame_object_to_frame_info (py_inf_frame);;
-
-  Py_DECREF (py_inf_frame);
-
+  frame = frame_object_to_frame_info (py_inf_frame.get ());
   if (frame == NULL)
     return EXT_LANG_BT_ERROR;
 
@@ -1085,7 +1081,7 @@ py_print_frame (PyObject *filter, int flags,
 	 address printing.  */
       if (PyObject_HasAttrString (filter, "address"))
 	{
-	  PyObject *paddr = PyObject_CallMethod (filter, "address", NULL);
+	  gdbpy_ref paddr (PyObject_CallMethod (filter, "address", NULL));
 
 	  if (paddr == NULL)
 	    {
@@ -1095,16 +1091,14 @@ py_print_frame (PyObject *filter, int flags,
 
 	  if (paddr != Py_None)
 	    {
-	      if (get_addr_from_python (paddr, &address) < 0)
+	      if (get_addr_from_python (paddr.get (), &address) < 0)
 		{
-		  Py_DECREF (paddr);
 		  do_cleanups (cleanup_stack);
 		  return EXT_LANG_BT_ERROR;
 		}
 
 	      has_addr = 1;
 	    }
-	  Py_DECREF (paddr);
 	}
     }
 
-- 
2.7.4

[RFA 06/13] Use gdbpy_ref in py-inferior.c

[Tom Tromey]

This changes py-inferior.c to use gdbpy_ref in more places.

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

	* python/py-inferior.c (find_thread_object, build_inferior_list):
	Use gdbpy_ref.
---
 gdb/ChangeLog            |  5 +++++
 gdb/python/py-inferior.c | 23 ++++++-----------------
 2 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 545dbde..28e427e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-inferior.c (find_thread_object, build_inferior_list):
+	Use gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-framefilter.c (py_print_frame): Use gdbpy_ref.
 
 2016-11-20  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index a9993a8..4ba20ce 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -251,19 +251,17 @@ find_thread_object (ptid_t ptid)
 {
   int pid;
   struct threadlist_entry *thread;
-  PyObject *inf_obj;
   thread_object *found = NULL;
 
   pid = ptid_get_pid (ptid);
   if (pid == 0)
     return NULL;
 
-  inf_obj = find_inferior_object (pid);
-
-  if (! inf_obj)
+  gdbpy_ref inf_obj (find_inferior_object (pid));
+  if (inf_obj == NULL)
     return NULL;
 
-  for (thread = ((inferior_object *)inf_obj)->threads; thread;
+  for (thread = ((inferior_object *)(inf_obj.get ()))->threads; thread;
        thread = thread->next)
     if (ptid_equal (thread->thread_obj->thread->ptid, ptid))
       {
@@ -271,8 +269,6 @@ find_thread_object (ptid_t ptid)
 	break;
       }
 
-  Py_DECREF (inf_obj);
-
   if (found)
     return found;
 
@@ -416,19 +412,12 @@ static int
 build_inferior_list (struct inferior *inf, void *arg)
 {
   PyObject *list = (PyObject *) arg;
-  PyObject *inferior = inferior_to_inferior_object (inf);
-  int success = 0;
+  gdbpy_ref inferior (inferior_to_inferior_object (inf));
 
-  if (! inferior)
+  if (inferior  == NULL)
     return 0;
 
-  success = PyList_Append (list, inferior);
-  Py_DECREF (inferior);
-
-  if (success)
-    return 1;
-
-  return 0;
+  return PyList_Append (list, inferior.get ()) ? 1 : 0;
 }
 
 /* Implementation of gdb.inferiors () -> (gdb.Inferior, ...).
-- 
2.7.4

[RFA 10/13] Use gdbpy_ref in py-utils.c

[Tom Tromey]

This changes more places in py-utils.c to use gdbpy_ref.

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

	* python/py-utils.c (unicode_to_encoded_string)
	(python_string_to_target_string)
	(python_string_to_target_python_string)
	(python_string_to_host_string, gdbpy_obj_to_string)
	(get_addr_from_python): Use gdbpy_ref.
---
 gdb/ChangeLog         |  8 ++++++++
 gdb/python/py-utils.c | 49 +++++++++++++++----------------------------------
 2 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3016d3a..4f8447c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-utils.c (unicode_to_encoded_string)
+	(python_string_to_target_string)
+	(python_string_to_target_python_string)
+	(python_string_to_host_string, gdbpy_obj_to_string)
+	(get_addr_from_python): Use gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-unwind.c (pyuw_object_attribute_to_pointer): Use
 	gdbpy_ref.
 
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 9f99e29..6b7540c 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -21,7 +21,7 @@
 #include "charset.h"
 #include "value.h"
 #include "python-internal.h"
-
+#include "py-ref.h"
 
 /* This is a cleanup function which decrements the refcount on a
    Python object.  */
@@ -111,21 +111,18 @@ static gdb::unique_xmalloc_ptr<char>
 unicode_to_encoded_string (PyObject *unicode_str, const char *charset)
 {
   gdb::unique_xmalloc_ptr<char> result;
-  PyObject *string;
 
   /* Translate string to named charset.  */
-  string = PyUnicode_AsEncodedString (unicode_str, charset, NULL);
+  gdbpy_ref string (PyUnicode_AsEncodedString (unicode_str, charset, NULL));
   if (string == NULL)
     return NULL;
 
 #ifdef IS_PY3K
-  result.reset (xstrdup (PyBytes_AsString (string)));
+  result.reset (xstrdup (PyBytes_AsString (string.get ())));
 #else
-  result.reset (xstrdup (PyString_AsString (string)));
+  result.reset (xstrdup (PyString_AsString (string.get ())));
 #endif
 
-  Py_DECREF (string);
-
   return result;
 }
 
@@ -168,15 +165,11 @@ unicode_to_target_python_string (PyObject *unicode_str)
 gdb::unique_xmalloc_ptr<char>
 python_string_to_target_string (PyObject *obj)
 {
-  PyObject *str;
-
-  str = python_string_to_unicode (obj);
+  gdbpy_ref str (python_string_to_unicode (obj));
   if (str == NULL)
     return NULL;
 
-  gdb::unique_xmalloc_ptr<char> result (unicode_to_target_string (str));
-  Py_DECREF (str);
-  return result;
+  return unicode_to_target_string (str.get ());
 }
 
 /* Converts a python string (8-bit or unicode) to a target string in the
@@ -187,16 +180,11 @@ python_string_to_target_string (PyObject *obj)
 PyObject *
 python_string_to_target_python_string (PyObject *obj)
 {
-  PyObject *str;
-  PyObject *result;
-
-  str = python_string_to_unicode (obj);
+  gdbpy_ref str (python_string_to_unicode (obj));
   if (str == NULL)
     return NULL;
 
-  result = unicode_to_target_python_string (str);
-  Py_DECREF (str);
-  return result;
+  return unicode_to_target_python_string (str.get ());
 }
 
 /* Converts a python string (8-bit or unicode) to a target string in
@@ -205,16 +193,11 @@ python_string_to_target_python_string (PyObject *obj)
 gdb::unique_xmalloc_ptr<char>
 python_string_to_host_string (PyObject *obj)
 {
-  PyObject *str;
-
-  str = python_string_to_unicode (obj);
+  gdbpy_ref str (python_string_to_unicode (obj));
   if (str == NULL)
     return NULL;
 
-  gdb::unique_xmalloc_ptr<char>
-    result (unicode_to_encoded_string (str, host_charset ()));
-  Py_DECREF (str);
-  return result;
+  return unicode_to_encoded_string (str.get (), host_charset ());
 }
 
 /* Convert a host string to a python string.  */
@@ -244,19 +227,18 @@ gdbpy_is_string (PyObject *obj)
 gdb::unique_xmalloc_ptr<char>
 gdbpy_obj_to_string (PyObject *obj)
 {
-  PyObject *str_obj = PyObject_Str (obj);
+  gdbpy_ref str_obj (PyObject_Str (obj));
 
   if (str_obj != NULL)
     {
       gdb::unique_xmalloc_ptr<char> msg;
 
 #ifdef IS_PY3K
-      msg = python_string_to_host_string (str_obj);
+      msg = python_string_to_host_string (str_obj.get ());
 #else
-      msg.reset (xstrdup (PyString_AsString (str_obj)));
+      msg.reset (xstrdup (PyString_AsString (str_obj.get ())));
 #endif
 
-      Py_DECREF (str_obj);
       return msg;
     }
 
@@ -329,14 +311,13 @@ get_addr_from_python (PyObject *obj, CORE_ADDR *addr)
     }
   else
     {
-      PyObject *num = PyNumber_Long (obj);
+      gdbpy_ref num (PyNumber_Long (obj));
       gdb_py_ulongest val;
 
       if (num == NULL)
 	return -1;
 
-      val = gdb_py_long_as_ulongest (num);
-      Py_XDECREF (num);
+      val = gdb_py_long_as_ulongest (num.get ());
       if (PyErr_Occurred ())
 	return -1;
 
-- 
2.7.4

[RFA 00/13] series #4 of c++ in python

[Tom Tromey]

This is another series addressing the use of C++ in the Python layer.
For this series, I looked at two things: explicit decrefs and use of
make_cleanup_py_decref.

After this series most of the explicit decrefs are done in Python
class destructors.  So, this is probably the last such series.  It is
based on the earlier series.

This series fixes a few latent bugs.  It culminates in the removal of
make_cleanup_py_decref and make_cleanup_py_xdecref.

I've built and tested it locally against Python 2 and Python 3, but
I'm also sending it through the buildbot.

Tom

[RFA 13/13] Remove make_cleanup_py_decref and make_cleanup_py_xdecref

[Tom Tromey]

make_cleanup_py_decref and make_cleanup_py_xdecref are now unused, so
this patch removes themm.  Future Python changes should use gdbpy_ref
instead.

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

	* python/python-internal.h (make_cleanup_py_decref)
	(make_cleanup_py_xdecref): Don't declare.
	* python/py-utils.c (py_decref, make_cleanup_py_decref)
	(py_xdecref, make_cleanup_py_xdecref): Remove.
---
 gdb/ChangeLog                |  7 +++++++
 gdb/python/py-utils.c        | 42 ------------------------------------------
 gdb/python/python-internal.h |  3 ---
 3 files changed, 7 insertions(+), 45 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b974442..c6edec2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/python-internal.h (make_cleanup_py_decref)
+	(make_cleanup_py_xdecref): Don't declare.
+	* python/py-utils.c (py_decref, make_cleanup_py_decref)
+	(py_xdecref, make_cleanup_py_xdecref): Remove.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-framefilter.c (py_mi_print_variables): Use gdbpy_ref.
 	(py_print_locals, enumerate_locals, py_print_args): Use gdbpy_ref.
 
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 6b7540c..7ca8b39 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -23,48 +23,6 @@
 #include "python-internal.h"
 #include "py-ref.h"
 
-/* This is a cleanup function which decrements the refcount on a
-   Python object.  */
-
-static void
-py_decref (void *p)
-{
-  PyObject *py = (PyObject *) p;
-
-  Py_DECREF (py);
-}
-
-/* Return a new cleanup which will decrement the Python object's
-   refcount when run.  */
-
-struct cleanup *
-make_cleanup_py_decref (PyObject *py)
-{
-  return make_cleanup (py_decref, (void *) py);
-}
-
-/* This is a cleanup function which decrements the refcount on a
-   Python object.  This function accounts appropriately for NULL
-   references.  */
-
-static void
-py_xdecref (void *p)
-{
-  PyObject *py = (PyObject *) p;
-
-  Py_XDECREF (py);
-}
-
-/* Return a new cleanup which will decrement the Python object's
-   refcount when run.  Account for and operate on NULL references
-   correctly.  */
-
-struct cleanup *
-make_cleanup_py_xdecref (PyObject *py)
-{
-  return make_cleanup (py_xdecref, py);
-}
-
 /* Converts a Python 8-bit string to a unicode string object.  Assumes the
    8-bit string is in the host charset.  If an error occurs during conversion,
    returns NULL with a python exception set.
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index e63ceed..4ff6ac2 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -498,9 +498,6 @@ int gdbpy_initialize_xmethods (void)
 int gdbpy_initialize_unwind (void)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 
-struct cleanup *make_cleanup_py_decref (PyObject *py);
-struct cleanup *make_cleanup_py_xdecref (PyObject *py);
-
 /* Called before entering the Python interpreter to install the
    current language and architecture to be used for Python values.
    Also set the active extension language for GDB so that SIGINT's
-- 
2.7.4

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

[Tom Tromey]

This changes py-cmd.c to use gdbpy_ref in more places.  This also
fixes a latent memory leak in cmdpy_completer_helper, which
unnecessarily increfs the result of PyObject_CallMethodObjArgs.  This
is not needed because that function returns a new reference.

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

	* python/py-cmd.c (cmdpy_completer_helper): Use gdbpy_ref.  Remove
	extra incref.
	(cmdpy_completer_handle_brkchars, cmdpy_completer, cmdpy_init):
	Use gdbpy_ref.
---
 gdb/ChangeLog       |  7 +++++
 gdb/python/py-cmd.c | 80 +++++++++++++++++++----------------------------------
 2 files changed, 36 insertions(+), 51 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ebb4d71..c4a1f71 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-cmd.c (cmdpy_completer_helper): Use gdbpy_ref.  Remove
+	extra incref.
+	(cmdpy_completer_handle_brkchars, cmdpy_completer, cmdpy_init):
+	Use gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-breakpoint.c (gdbpy_breakpoint_cond_says_stop): Use
 	gdbpy_ref.
 
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 17daaee..4f605c4 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -232,8 +232,6 @@ cmdpy_completer_helper (struct cmd_list_element *command,
 			const char *text, const char *word)
 {
   cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
-  PyObject *textobj, *wordobj;
-  PyObject *resultobj;
 
   if (obj == NULL)
     error (_("Invalid invocation of Python command object."));
@@ -243,29 +241,25 @@ cmdpy_completer_helper (struct cmd_list_element *command,
       return NULL;
     }
 
-  textobj = PyUnicode_Decode (text, strlen (text), host_charset (), NULL);
+  gdbpy_ref textobj (PyUnicode_Decode (text, strlen (text), host_charset (),
+				       NULL));
   if (textobj == NULL)
     error (_("Could not convert argument to Python string."));
-  wordobj = PyUnicode_Decode (word, strlen (word), host_charset (), NULL);
+  gdbpy_ref wordobj (PyUnicode_Decode (word, strlen (word), host_charset (),
+				       NULL));
   if (wordobj == NULL)
-    {
-      Py_DECREF (textobj);
-      error (_("Could not convert argument to Python string."));
-    }
+    error (_("Could not convert argument to Python string."));
 
-  resultobj = PyObject_CallMethodObjArgs ((PyObject *) obj, complete_cst,
-					  textobj, wordobj, NULL);
-  Py_DECREF (textobj);
-  Py_DECREF (wordobj);
-  if (!resultobj)
+  gdbpy_ref resultobj (PyObject_CallMethodObjArgs ((PyObject *) obj,
+						   complete_cst,
+						   textobj, wordobj, NULL));
+  if (resultobj == NULL)
     {
       /* Just swallow errors here.  */
       PyErr_Clear ();
     }
 
-  Py_XINCREF (resultobj);
-
-  return resultobj;
+  return resultobj.release ();
 }
 
 /* Python function called to determine the break characters of a
@@ -277,26 +271,24 @@ static void
 cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
 				 const char *text, const char *word)
 {
-  PyObject *resultobj = NULL;
-
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
   /* Calling our helper to obtain the PyObject of the Python
      function.  */
-  resultobj = cmdpy_completer_helper (command, text, word);
+  gdbpy_ref resultobj (cmdpy_completer_helper (command, text, word));
 
   /* Check if there was an error.  */
   if (resultobj == NULL)
-    goto done;
+    return;
 
-  if (PyInt_Check (resultobj))
+  if (PyInt_Check (resultobj.get ()))
     {
       /* User code may also return one of the completion constants,
 	 thus requesting that sort of completion.  We are only
 	 interested in this kind of return.  */
       long value;
 
-      if (!gdb_py_int_as_long (resultobj, &value))
+      if (!gdb_py_int_as_long (resultobj.get (), &value))
 	{
 	  /* Ignore.  */
 	  PyErr_Clear ();
@@ -310,10 +302,6 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
 	    (completers[value].completer);
 	}
     }
-
- done:
-
-  Py_XDECREF (resultobj);
 }
 
 /* Called by gdb for command completion.  */
@@ -322,29 +310,28 @@ static VEC (char_ptr) *
 cmdpy_completer (struct cmd_list_element *command,
 		 const char *text, const char *word)
 {
-  PyObject *resultobj = NULL;
   VEC (char_ptr) *result = NULL;
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
   /* Calling our helper to obtain the PyObject of the Python
      function.  */
-  resultobj = cmdpy_completer_helper (command, text, word);
+  gdbpy_ref resultobj (cmdpy_completer_helper (command, text, word));
 
   /* If the result object of calling the Python function is NULL, it
      means that there was an error.  In this case, just give up and
      return NULL.  */
   if (resultobj == NULL)
-    goto done;
+    return NULL;
 
   result = NULL;
-  if (PyInt_Check (resultobj))
+  if (PyInt_Check (resultobj.get ()))
     {
       /* User code may also return one of the completion constants,
 	 thus requesting that sort of completion.  */
       long value;
 
-      if (! gdb_py_int_as_long (resultobj, &value))
+      if (! gdb_py_int_as_long (resultobj.get (), &value))
 	{
 	  /* Ignore.  */
 	  PyErr_Clear ();
@@ -354,24 +341,24 @@ cmdpy_completer (struct cmd_list_element *command,
     }
   else
     {
-      PyObject *iter = PyObject_GetIter (resultobj);
-      PyObject *elt;
+      gdbpy_ref iter (PyObject_GetIter (resultobj.get ()));
 
       if (iter == NULL)
-	goto done;
+	return NULL;
 
-      while ((elt = PyIter_Next (iter)) != NULL)
+      while (true)
 	{
+	  gdbpy_ref elt (PyIter_Next (iter.get ()));
+	  if (elt == NULL)
+	    break;
 
-	  if (! gdbpy_is_string (elt))
+	  if (! gdbpy_is_string (elt.get ()))
 	    {
 	      /* Skip problem elements.  */
-	      Py_DECREF (elt);
 	      continue;
 	    }
 	  gdb::unique_xmalloc_ptr<char>
-	    item (python_string_to_host_string (elt));
-	  Py_DECREF (elt);
+	    item (python_string_to_host_string (elt.get ()));
 	  if (item == NULL)
 	    {
 	      /* Skip problem elements.  */
@@ -381,18 +368,12 @@ cmdpy_completer (struct cmd_list_element *command,
 	  VEC_safe_push (char_ptr, result, item.release ());
 	}
 
-      Py_DECREF (iter);
-
       /* If we got some results, ignore problems.  Otherwise, report
 	 the problem.  */
       if (result != NULL && PyErr_Occurred ())
 	PyErr_Clear ();
     }
 
- done:
-
-  Py_XDECREF (resultobj);
-
   return result;
 }
 
@@ -587,21 +568,18 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
     }
   if (PyObject_HasAttr (self, gdbpy_doc_cst))
     {
-      PyObject *ds_obj = PyObject_GetAttr (self, gdbpy_doc_cst);
+      gdbpy_ref ds_obj (PyObject_GetAttr (self, gdbpy_doc_cst));
 
-      if (ds_obj && gdbpy_is_string (ds_obj))
+      if (ds_obj != NULL && gdbpy_is_string (ds_obj.get ()))
 	{
-	  docstring = python_string_to_host_string (ds_obj).release ();
+	  docstring = python_string_to_host_string (ds_obj.get ()).release ();
 	  if (docstring == NULL)
 	    {
 	      xfree (cmd_name);
 	      xfree (pfx_name);
-	      Py_DECREF (ds_obj);
 	      return -1;
 	    }
 	}
-
-      Py_XDECREF (ds_obj);
     }
   if (! docstring)
     docstring = xstrdup (_("This command is not documented."));
-- 
2.7.4

[RFA 12/13] Use gdbpy_ref rather than make_cleanup_py_decref

[Tom Tromey]

This changes some spots in py-framefilter.c to use gdbpy_ref rather
than make_cleanup_py_decref or make_cleanup_py_xdecref.

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

	* python/py-framefilter.c (py_mi_print_variables): Use gdbpy_ref.
	(py_print_locals, enumerate_locals, py_print_args): Use gdbpy_ref.
---
 gdb/ChangeLog               |  5 ++++
 gdb/python/py-framefilter.c | 59 +++++++++++++++++++++------------------------
 2 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c9671de..b974442 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-framefilter.c (py_mi_print_variables): Use gdbpy_ref.
+	(py_print_locals, enumerate_locals, py_print_args): Use gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-framefilter.c (enumerate_args): Use gdbpy_ref.
 
 2016-11-20  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index dc41790..07176cc 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -693,13 +693,12 @@ enumerate_locals (PyObject *iter,
 		  int print_args_field,
 		  struct frame_info *frame)
 {
-  PyObject *item;
   struct value_print_options opts;
 
   get_user_print_options (&opts);
   opts.deref_ref = 1;
 
-  while ((item = PyIter_Next (iter)))
+  while (true)
     {
       const struct language_defn *language;
       gdb::unique_xmalloc_ptr<char> sym_name;
@@ -710,16 +709,21 @@ enumerate_locals (PyObject *iter,
       int local_indent = 8 + (8 * indent);
       struct cleanup *locals_cleanups;
 
-      locals_cleanups = make_cleanup_py_decref (item);
+      gdbpy_ref item (PyIter_Next (iter));
+      if (item == NULL)
+	break;
+
+      locals_cleanups = make_cleanup (null_cleanup, NULL);
 
-      success = extract_sym (item, &sym_name, &sym, &sym_block, &language);
+      success = extract_sym (item.get (), &sym_name, &sym, &sym_block,
+			     &language);
       if (success == EXT_LANG_BT_ERROR)
 	{
 	  do_cleanups (locals_cleanups);
 	  goto error;
 	}
 
-      success = extract_value (item, &val);
+      success = extract_value (item.get (), &val);
       if (success == EXT_LANG_BT_ERROR)
 	{
 	  do_cleanups (locals_cleanups);
@@ -827,10 +831,8 @@ enumerate_locals (PyObject *iter,
       END_CATCH
     }
 
-  if (item == NULL && PyErr_Occurred ())
-    goto error;
-
-  return EXT_LANG_BT_OK;
+  if (!PyErr_Occurred ())
+    return EXT_LANG_BT_OK;
 
  error:
   return EXT_LANG_BT_ERROR;
@@ -846,28 +848,24 @@ py_mi_print_variables (PyObject *filter, struct ui_out *out,
 		       struct frame_info *frame)
 {
   struct cleanup *old_chain;
-  PyObject *args_iter;
-  PyObject *locals_iter;
 
-  args_iter = get_py_iter_from_func (filter, "frame_args");
-  old_chain = make_cleanup_py_xdecref (args_iter);
+  gdbpy_ref args_iter (get_py_iter_from_func (filter, "frame_args"));
   if (args_iter == NULL)
-    goto error;
+    return EXT_LANG_BT_ERROR;
 
-  locals_iter = get_py_iter_from_func (filter, "frame_locals");
+  gdbpy_ref locals_iter (get_py_iter_from_func (filter, "frame_locals"));
   if (locals_iter == NULL)
-    goto error;
+    return EXT_LANG_BT_ERROR;
 
-  make_cleanup_py_decref (locals_iter);
-  make_cleanup_ui_out_list_begin_end (out, "variables");
+  old_chain = make_cleanup_ui_out_list_begin_end (out, "variables");
 
   if (args_iter != Py_None)
-    if (enumerate_args (args_iter, out, args_type, 1, frame)
+    if (enumerate_args (args_iter.get (), out, args_type, 1, frame)
 	== EXT_LANG_BT_ERROR)
       goto error;
 
   if (locals_iter != Py_None)
-    if (enumerate_locals (locals_iter, out, 1, args_type, 1, frame)
+    if (enumerate_locals (locals_iter.get (), out, 1, args_type, 1, frame)
 	== EXT_LANG_BT_ERROR)
       goto error;
 
@@ -890,17 +888,16 @@ py_print_locals (PyObject *filter,
 		 int indent,
 		 struct frame_info *frame)
 {
-  PyObject *locals_iter = get_py_iter_from_func (filter,
-						 "frame_locals");
-  struct cleanup *old_chain = make_cleanup_py_xdecref (locals_iter);
+  struct cleanup *old_chain;
 
+  gdbpy_ref locals_iter (get_py_iter_from_func (filter, "frame_locals"));
   if (locals_iter == NULL)
-    goto locals_error;
+    return EXT_LANG_BT_ERROR;
 
-  make_cleanup_ui_out_list_begin_end (out, "locals");
+  old_chain = make_cleanup_ui_out_list_begin_end (out, "locals");
 
   if (locals_iter != Py_None)
-    if (enumerate_locals (locals_iter, out, indent, args_type,
+    if (enumerate_locals (locals_iter.get (), out, indent, args_type,
 			  0, frame) == EXT_LANG_BT_ERROR)
       goto locals_error;
 
@@ -923,13 +920,13 @@ py_print_args (PyObject *filter,
 	       enum ext_lang_frame_args args_type,
 	       struct frame_info *frame)
 {
-  PyObject *args_iter  = get_py_iter_from_func (filter, "frame_args");
-  struct cleanup *old_chain = make_cleanup_py_xdecref (args_iter);
+  struct cleanup *old_chain;
 
+  gdbpy_ref args_iter (get_py_iter_from_func (filter, "frame_args"));
   if (args_iter == NULL)
-    goto args_error;
+    return EXT_LANG_BT_ERROR;
 
-  make_cleanup_ui_out_list_begin_end (out, "args");
+  old_chain = make_cleanup_ui_out_list_begin_end (out, "args");
 
   TRY
     {
@@ -945,7 +942,7 @@ py_print_args (PyObject *filter,
   END_CATCH
 
   if (args_iter != Py_None)
-    if (enumerate_args (args_iter, out, args_type, 0, frame)
+    if (enumerate_args (args_iter.get (), out, args_type, 0, frame)
 	== EXT_LANG_BT_ERROR)
       goto args_error;
 
-- 
2.7.4

[RFA 08/13] Use gdbpy_ref in python.c

[Tom Tromey]

This changes more places in python.c to use gdbpy_ref.

Additionally, previously gdbpy_apply_type_printers would return
EXT_LANG_RC_ERROR if a type printer returned None.  However, that
doesn't seem correct to me; this patch changes it to return
EXT_LANG_RC_NOP in this case.

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

	* python/python.c (eval_python_command, gdbpy_decode_line)
	(gdbpy_run_events, gdbpy_start_type_printers)
	(gdbpy_apply_type_printers): Use gdbpy_ref.
---
 gdb/ChangeLog       |   6 +++
 gdb/python/python.c | 120 +++++++++++++++++++++++-----------------------------
 2 files changed, 59 insertions(+), 67 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 92fee8c..ab0cefb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/python.c (eval_python_command, gdbpy_decode_line)
+	(gdbpy_run_events, gdbpy_start_type_printers)
+	(gdbpy_apply_type_printers): Use gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-param.c (get_doc_string, compute_enum_values): Use
 	gdbpy_ref.
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 7a38ca2..83b9805 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -264,7 +264,7 @@ gdbpy_check_quit_flag (const struct extension_language_defn *extlang)
 static int
 eval_python_command (const char *command)
 {
-  PyObject *m, *d, *v;
+  PyObject *m, *d;
 
   m = PyImport_AddModule ("__main__");
   if (m == NULL)
@@ -273,11 +273,10 @@ eval_python_command (const char *command)
   d = PyModule_GetDict (m);
   if (d == NULL)
     return -1;
-  v = PyRun_StringFlags (command, Py_single_input, d, d, NULL);
+  gdbpy_ref v (PyRun_StringFlags (command, Py_single_input, d, d, NULL));
   if (v == NULL)
     return -1;
 
-  Py_DECREF (v);
 #ifndef IS_PY3K
   if (Py_FlushLine ())
     PyErr_Clear ();
@@ -672,9 +671,8 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
   struct symtab_and_line sal;
   char *arg = NULL;
   struct cleanup *cleanups;
-  PyObject *result = NULL;
-  PyObject *return_result = NULL;
-  PyObject *unparsed = NULL;
+  gdbpy_ref result;
+  gdbpy_ref unparsed;
   struct event_location *location = NULL;
 
   if (! PyArg_ParseTuple (args, "|s", &arg))
@@ -723,9 +721,12 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
     {
       int i;
 
-      result = PyTuple_New (sals.nelts);
-      if (! result)
-	goto error;
+      result.reset (PyTuple_New (sals.nelts));
+      if (result == NULL)
+	{
+	  do_cleanups (cleanups);
+	  return NULL;
+	}
       for (i = 0; i < sals.nelts; ++i)
 	{
 	  PyObject *obj;
@@ -733,50 +734,47 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
 	  obj = symtab_and_line_to_sal_object (sals.sals[i]);
 	  if (! obj)
 	    {
-	      Py_DECREF (result);
-	      goto error;
+	      do_cleanups (cleanups);
+	      return NULL;
 	    }
 
-	  PyTuple_SetItem (result, i, obj);
+	  PyTuple_SetItem (result.get (), i, obj);
 	}
     }
   else
     {
-      result = Py_None;
+      result.reset (Py_None);
       Py_INCREF (Py_None);
     }
 
-  return_result = PyTuple_New (2);
-  if (! return_result)
+  gdbpy_ref return_result (PyTuple_New (2));
+  if (return_result == NULL)
     {
-      Py_DECREF (result);
-      goto error;
+      do_cleanups (cleanups);
+      return NULL;
     }
 
   if (arg != NULL && strlen (arg) > 0)
     {
-      unparsed = PyString_FromString (arg);
+      unparsed.reset (PyString_FromString (arg));
       if (unparsed == NULL)
 	{
-	  Py_DECREF (result);
-	  Py_DECREF (return_result);
-	  return_result = NULL;
-	  goto error;
+	  do_cleanups (cleanups);
+	  return NULL;
 	}
     }
   else
     {
-      unparsed = Py_None;
+      unparsed.reset (Py_None);
       Py_INCREF (Py_None);
     }
 
-  PyTuple_SetItem (return_result, 0, unparsed);
-  PyTuple_SetItem (return_result, 1, result);
+  PyTuple_SetItem (return_result.get (), 0, unparsed.release ());
+  PyTuple_SetItem (return_result.get (), 1, result.release ());
 
- error:
   do_cleanups (cleanups);
 
-  return return_result;
+  return return_result.release ();
 }
 
 /* Parse a string and evaluate it as an expression.  */
@@ -893,8 +891,6 @@ gdbpy_run_events (int error, gdb_client_data client_data)
 
   while (gdbpy_event_list)
     {
-      PyObject *call_result;
-
       /* Dispatching the event might push a new element onto the event
 	 loop, so we update here "atomically enough".  */
       struct gdbpy_event *item = gdbpy_event_list;
@@ -903,11 +899,10 @@ gdbpy_run_events (int error, gdb_client_data client_data)
 	gdbpy_event_list_end = &gdbpy_event_list;
 
       /* Ignore errors.  */
-      call_result = PyObject_CallObject (item->event, NULL);
+      gdbpy_ref call_result (PyObject_CallObject (item->event, NULL));
       if (call_result == NULL)
 	PyErr_Clear ();
 
-      Py_XDECREF (call_result);
       Py_DECREF (item->event);
       xfree (item);
     }
@@ -1327,36 +1322,33 @@ static void
 gdbpy_start_type_printers (const struct extension_language_defn *extlang,
 			   struct ext_lang_type_printers *ext_printers)
 {
-  PyObject *type_module, *func = NULL, *printers_obj = NULL;
+  PyObject *printers_obj = NULL;
 
   if (!gdb_python_initialized)
     return;
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  type_module = PyImport_ImportModule ("gdb.types");
+  gdbpy_ref type_module (PyImport_ImportModule ("gdb.types"));
   if (type_module == NULL)
     {
       gdbpy_print_stack ();
-      goto done;
+      return;
     }
 
-  func = PyObject_GetAttrString (type_module, "get_type_recognizers");
+  gdbpy_ref func (PyObject_GetAttrString (type_module.get (),
+					  "get_type_recognizers"));
   if (func == NULL)
     {
       gdbpy_print_stack ();
-      goto done;
+      return;
     }
 
-  printers_obj = PyObject_CallFunctionObjArgs (func, (char *) NULL);
+  printers_obj = PyObject_CallFunctionObjArgs (func.get (), (char *) NULL);
   if (printers_obj == NULL)
     gdbpy_print_stack ();
   else
     ext_printers->py_type_printers = printers_obj;
-
- done:
-  Py_XDECREF (type_module);
-  Py_XDECREF (func);
 }
 
 /* If TYPE is recognized by some type printer, store in *PRETTIED_TYPE
@@ -1371,8 +1363,6 @@ gdbpy_apply_type_printers (const struct extension_language_defn *extlang,
 			   const struct ext_lang_type_printers *ext_printers,
 			   struct type *type, char **prettied_type)
 {
-  PyObject *type_obj, *type_module = NULL, *func = NULL;
-  PyObject *result_obj = NULL;
   PyObject *printers_obj = (PyObject *) ext_printers->py_type_printers;
   gdb::unique_xmalloc_ptr<char> result;
 
@@ -1384,53 +1374,49 @@ gdbpy_apply_type_printers (const struct extension_language_defn *extlang,
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  type_obj = type_to_type_object (type);
+  gdbpy_ref type_obj (type_to_type_object (type));
   if (type_obj == NULL)
     {
       gdbpy_print_stack ();
-      goto done;
+      return EXT_LANG_RC_ERROR;
     }
 
-  type_module = PyImport_ImportModule ("gdb.types");
+  gdbpy_ref type_module (PyImport_ImportModule ("gdb.types"));
   if (type_module == NULL)
     {
       gdbpy_print_stack ();
-      goto done;
+      return EXT_LANG_RC_ERROR;
     }
 
-  func = PyObject_GetAttrString (type_module, "apply_type_recognizers");
+  gdbpy_ref func (PyObject_GetAttrString (type_module.get (),
+					  "apply_type_recognizers"));
   if (func == NULL)
     {
       gdbpy_print_stack ();
-      goto done;
+      return EXT_LANG_RC_ERROR;
     }
 
-  result_obj = PyObject_CallFunctionObjArgs (func, printers_obj,
-					     type_obj, (char *) NULL);
+  gdbpy_ref result_obj (PyObject_CallFunctionObjArgs (func.get (), printers_obj,
+						      type_obj.get (),
+						      (char *) NULL));
   if (result_obj == NULL)
     {
       gdbpy_print_stack ();
-      goto done;
+      return EXT_LANG_RC_ERROR;
     }
 
-  if (result_obj != Py_None)
-    {
-      result = python_string_to_host_string (result_obj);
-      if (result == NULL)
-	gdbpy_print_stack ();
-    }
+  if (result_obj == Py_None)
+    return EXT_LANG_RC_NOP;
 
- done:
-  Py_XDECREF (type_obj);
-  Py_XDECREF (type_module);
-  Py_XDECREF (func);
-  Py_XDECREF (result_obj);
-  if (result != NULL)
+  result = python_string_to_host_string (result_obj.get ());
+  if (result == NULL)
     {
-      *prettied_type = result.release ();
-      return EXT_LANG_RC_OK;
+      gdbpy_print_stack ();
+      return EXT_LANG_RC_ERROR;
     }
-  return EXT_LANG_RC_ERROR;
+
+  *prettied_type = result.release ();
+  return EXT_LANG_RC_OK;
 }
 
 /* Free the result of start_type_printers.
-- 
2.7.4

[RFA 07/13] Use gdbpy_ref in py-param.c

[Tom Tromey]

This changes py-param.c to use gdbpy_ref in a couple more spots.

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

	* python/py-param.c (get_doc_string, compute_enum_values): Use
	gdbpy_ref.
---
 gdb/ChangeLog         |  5 +++++
 gdb/python/py-param.c | 18 ++++++++----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 28e427e..92fee8c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-param.c (get_doc_string, compute_enum_values): Use
+	gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-inferior.c (find_thread_object, build_inferior_list):
 	Use gdbpy_ref.
 
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index d402928..de42f46 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -307,15 +307,14 @@ get_doc_string (PyObject *object, PyObject *attr)
 
   if (PyObject_HasAttr (object, attr))
     {
-      PyObject *ds_obj = PyObject_GetAttr (object, attr);
+      gdbpy_ref ds_obj (PyObject_GetAttr (object, attr));
 
-      if (ds_obj && gdbpy_is_string (ds_obj))
+      if (ds_obj != NULL && gdbpy_is_string (ds_obj.get ()))
 	{
-	  result = python_string_to_host_string (ds_obj);
+	  result = python_string_to_host_string (ds_obj.get ());
 	  if (result == NULL)
 	    gdbpy_print_stack ();
 	}
-      Py_XDECREF (ds_obj);
     }
   if (! result)
     result.reset (xstrdup (_("This command is not documented.")));
@@ -587,23 +586,22 @@ compute_enum_values (parmpy_object *self, PyObject *enum_values)
 
   for (i = 0; i < size; ++i)
     {
-      PyObject *item = PySequence_GetItem (enum_values, i);
+      gdbpy_ref item (PySequence_GetItem (enum_values, i));
 
-      if (! item)
+      if (item == NULL)
 	{
 	  do_cleanups (back_to);
 	  return 0;
 	}
-      if (! gdbpy_is_string (item))
+      if (! gdbpy_is_string (item.get ()))
 	{
-	  Py_DECREF (item);
 	  do_cleanups (back_to);
 	  PyErr_SetString (PyExc_RuntimeError,
 			   _("The enumeration item not a string."));
 	  return 0;
 	}
-      self->enumeration[i] = python_string_to_host_string (item).release ();
-      Py_DECREF (item);
+      self->enumeration[i]
+	= python_string_to_host_string (item.get ()).release ();
       if (self->enumeration[i] == NULL)
 	{
 	  do_cleanups (back_to);
-- 
2.7.4

[RFA 09/13] Use gdbpy_ref in pyuw_object_attribute_to_pointer

[Tom Tromey]

This changes pyuw_object_attribute_to_pointer to use gdbpy_ref.

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

	* python/py-unwind.c (pyuw_object_attribute_to_pointer): Use
	gdbpy_ref.
---
 gdb/ChangeLog          | 5 +++++
 gdb/python/py-unwind.c | 5 ++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ab0cefb..3016d3a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-unwind.c (pyuw_object_attribute_to_pointer): Use
+	gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/python.c (eval_python_command, gdbpy_decode_line)
 	(gdbpy_run_events, gdbpy_start_type_printers)
 	(gdbpy_apply_type_printers): Use gdbpy_ref.
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 416223c..2c75e1d 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -177,18 +177,17 @@ pyuw_object_attribute_to_pointer (PyObject *pyo, const char *attr_name,
 
   if (PyObject_HasAttrString (pyo, attr_name))
     {
-      PyObject *pyo_value = PyObject_GetAttrString (pyo, attr_name);
+      gdbpy_ref pyo_value (PyObject_GetAttrString (pyo, attr_name));
 
       if (pyo_value != NULL && pyo_value != Py_None)
         {
-          rc = pyuw_value_obj_to_pointer (pyo_value, addr);
+          rc = pyuw_value_obj_to_pointer (pyo_value.get (), addr);
           if (!rc)
             PyErr_Format (
                 PyExc_ValueError,
                 _("The value of the '%s' attribute is not a pointer."),
                 attr_name);
         }
-      Py_XDECREF (pyo_value);
     }
   return rc;
 }
-- 
2.7.4

[RFA 04/13] Use gdbpy_ref in bpfinishpy_out_of_scope

[Tom Tromey]

This changes bpfinishpy_out_of_scope to use gdbpy_ref.

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

	* python/py-finishbreakpoint.c (bpfinishpy_out_of_scope): Use
	gdbpy_ref.
---
 gdb/ChangeLog                    | 5 +++++
 gdb/python/py-finishbreakpoint.c | 7 +++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c4a1f71..83ad836 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-finishbreakpoint.c (bpfinishpy_out_of_scope): Use
+	gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-cmd.c (cmdpy_completer_helper): Use gdbpy_ref.  Remove
 	extra incref.
 	(cmdpy_completer_handle_brkchars, cmdpy_completer, cmdpy_init):
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index fdac696..391e735 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -30,6 +30,7 @@
 #include "inferior.h"
 #include "block.h"
 #include "location.h"
+#include "py-ref.h"
 
 /* Function that is called when a Python finish bp is found out of scope.  */
 static char * const outofscope_func = "out_of_scope";
@@ -337,12 +338,10 @@ bpfinishpy_out_of_scope (struct finish_breakpoint_object *bpfinish_obj)
   if (bpfinish_obj->py_bp.bp->enable_state == bp_enabled
       && PyObject_HasAttrString (py_obj, outofscope_func))
     {
-      PyObject *meth_result;
-
-      meth_result = PyObject_CallMethod (py_obj, outofscope_func, NULL);
+      gdbpy_ref meth_result (PyObject_CallMethod (py_obj, outofscope_func,
+						  NULL));
       if (meth_result == NULL)
 	gdbpy_print_stack ();
-      Py_XDECREF (meth_result);
     }
 
   delete_breakpoint (bpfinish_obj->py_bp.bp);
-- 
2.7.4

[RFA 11/13] Use gdbpy_ref in enumerate_args

[Tom Tromey]

This changes enumerate_args to use gdbpy_ref, and gets rid of many
gotos.

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

	* python/py-framefilter.c (enumerate_args): Use gdbpy_ref.
---
 gdb/ChangeLog               |  4 ++++
 gdb/python/py-framefilter.c | 56 +++++++++++++++++----------------------------
 2 files changed, 25 insertions(+), 35 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4f8447c..c9671de 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-framefilter.c (enumerate_args): Use gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-utils.c (unicode_to_encoded_string)
 	(python_string_to_target_string)
 	(python_string_to_target_python_string)
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index ff1a068..dc41790 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -497,7 +497,6 @@ enumerate_args (PyObject *iter,
 		int print_args_field,
 		struct frame_info *frame)
 {
-  PyObject *item;
   struct value_print_options opts;
 
   get_user_print_options (&opts);
@@ -517,7 +516,7 @@ enumerate_args (PyObject *iter,
   CATCH (except, RETURN_MASK_ALL)
     {
       gdbpy_convert_exception (except);
-      goto error;
+      return EXT_LANG_BT_ERROR;
     }
   END_CATCH
 
@@ -525,11 +524,11 @@ enumerate_args (PyObject *iter,
       commas in the argument output is correct.  At the end of the
       loop block collect another item from the iterator, and, if it is
       not null emit a comma.  */
-  item = PyIter_Next (iter);
+  gdbpy_ref item (PyIter_Next (iter));
   if (item == NULL && PyErr_Occurred ())
-    goto error;
+    return EXT_LANG_BT_ERROR;
 
-  while (item)
+  while (item != NULL)
     {
       const struct language_defn *language;
       gdb::unique_xmalloc_ptr<char> sym_name;
@@ -538,22 +537,14 @@ enumerate_args (PyObject *iter,
       struct value *val;
       enum ext_lang_bt_status success = EXT_LANG_BT_ERROR;
 
-      success = extract_sym (item, &sym_name, &sym, &sym_block, &language);
+      success = extract_sym (item.get (), &sym_name, &sym, &sym_block,
+			     &language);
       if (success == EXT_LANG_BT_ERROR)
-	{
-	  Py_DECREF (item);
-	  goto error;
-	}
+	return EXT_LANG_BT_ERROR;
 
-      success = extract_value (item, &val);
+      success = extract_value (item.get (), &val);
       if (success == EXT_LANG_BT_ERROR)
-	{
-	  Py_DECREF (item);
-	  goto error;
-	}
-
-      Py_DECREF (item);
-      item = NULL;
+	return EXT_LANG_BT_ERROR;
 
       if (sym && ui_out_is_mi_like_p (out)
 	  && ! mi_should_print (sym, MI_PRINT_ARGS))
@@ -571,7 +562,7 @@ enumerate_args (PyObject *iter,
 	    {
 	      PyErr_SetString (PyExc_RuntimeError,
 			       _("No symbol or value provided."));
-	      goto error;
+	      return EXT_LANG_BT_ERROR;
 	    }
 
 	  TRY
@@ -581,7 +572,7 @@ enumerate_args (PyObject *iter,
 	  CATCH (except, RETURN_MASK_ALL)
 	    {
 	      gdbpy_convert_exception (except);
-	      goto error;
+	      return EXT_LANG_BT_ERROR;
 	    }
 	  END_CATCH
 
@@ -599,7 +590,7 @@ enumerate_args (PyObject *iter,
 		{
 		  xfree (arg.error);
 		  xfree (entryarg.error);
-		  goto error;
+		  return EXT_LANG_BT_ERROR;
 		}
 	    }
 
@@ -617,7 +608,7 @@ enumerate_args (PyObject *iter,
 		      xfree (arg.error);
 		      xfree (entryarg.error);
 		      gdbpy_convert_exception (except);
-		      goto error;
+		      return EXT_LANG_BT_ERROR;
 		    }
 		  END_CATCH
 		}
@@ -626,9 +617,9 @@ enumerate_args (PyObject *iter,
 				       args_type, print_args_field, NULL)
 		  == EXT_LANG_BT_ERROR)
 		{
-		      xfree (arg.error);
-		      xfree (entryarg.error);
-		      goto error;
+		  xfree (arg.error);
+		  xfree (entryarg.error);
+		  return EXT_LANG_BT_ERROR;
 		}
 	    }
 
@@ -643,14 +634,14 @@ enumerate_args (PyObject *iter,
 	      if (py_print_single_arg (out, sym_name.get (), NULL, val, &opts,
 				       args_type, print_args_field,
 				       language) == EXT_LANG_BT_ERROR)
-		goto error;
+		return EXT_LANG_BT_ERROR;
 	    }
 	}
 
       /* Collect the next item from the iterator.  If
 	 this is the last item, do not print the
 	 comma.  */
-      item = PyIter_Next (iter);
+      item.reset (PyIter_Next (iter));
       if (item != NULL)
 	{
 	  TRY
@@ -659,14 +650,13 @@ enumerate_args (PyObject *iter,
 	    }
 	  CATCH (except, RETURN_MASK_ALL)
 	    {
-	      Py_DECREF (item);
 	      gdbpy_convert_exception (except);
-	      goto error;
+	      return EXT_LANG_BT_ERROR;
 	    }
 	  END_CATCH
 	}
       else if (PyErr_Occurred ())
-	goto error;
+	return EXT_LANG_BT_ERROR;
 
       TRY
 	{
@@ -674,17 +664,13 @@ enumerate_args (PyObject *iter,
 	}
       CATCH (except, RETURN_MASK_ALL)
 	{
-	  Py_DECREF (item);
 	  gdbpy_convert_exception (except);
-	  goto error;
+	  return EXT_LANG_BT_ERROR;
 	}
       END_CATCH
     }
 
   return EXT_LANG_BT_OK;
-
- error:
-  return EXT_LANG_BT_ERROR;
 }
 
 
-- 
2.7.4

Re: [RFA 00/13] series #4 of c++ in python

[Tom Tromey]

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

Tom> I've built and tested it locally against Python 2 and Python 3, but
Tom> I'm also sending it through the buildbot.

The buildbot helpfully pointed out a bug in the py-cmd.c change; I'll
send an updated patch.

A subsequent run, with the new change installed, was clean.

Tom

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

[Tom Tromey]

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

Tom> This changes py-cmd.c to use gdbpy_ref in more places.  This also
Tom> fixes a latent memory leak in cmdpy_completer_helper, which
Tom> unnecessarily increfs the result of PyObject_CallMethodObjArgs.  This
Tom> is not needed because that function returns a new reference.

One of the buildbot compilers noticed that this patch was incomplete --
it was still passing a gdbpy_ref to a varargs function.  I assume this
wasn't caught in local testing because (1) my version of gcc doesn't
warn, and (2) the object is effectively passed as a pointer, making it
"work".

I've appended the corrected patch.

Tom

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ebb4d71..c4a1f71 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2016-11-20  Tom Tromey  <tom@tromey.com>
 
+	* python/py-cmd.c (cmdpy_completer_helper): Use gdbpy_ref.  Remove
+	extra incref.
+	(cmdpy_completer_handle_brkchars, cmdpy_completer, cmdpy_init):
+	Use gdbpy_ref.
+
+2016-11-20  Tom Tromey  <tom@tromey.com>
+
 	* python/py-breakpoint.c (gdbpy_breakpoint_cond_says_stop): Use
 	gdbpy_ref.
 
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 17daaee..202c60d 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -232,8 +232,6 @@ cmdpy_completer_helper (struct cmd_list_element *command,
 			const char *text, const char *word)
 {
   cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
-  PyObject *textobj, *wordobj;
-  PyObject *resultobj;
 
   if (obj == NULL)
     error (_("Invalid invocation of Python command object."));
@@ -243,29 +241,26 @@ cmdpy_completer_helper (struct cmd_list_element *command,
       return NULL;
     }
 
-  textobj = PyUnicode_Decode (text, strlen (text), host_charset (), NULL);
+  gdbpy_ref textobj (PyUnicode_Decode (text, strlen (text), host_charset (),
+				       NULL));
   if (textobj == NULL)
     error (_("Could not convert argument to Python string."));
-  wordobj = PyUnicode_Decode (word, strlen (word), host_charset (), NULL);
+  gdbpy_ref wordobj (PyUnicode_Decode (word, strlen (word), host_charset (),
+				       NULL));
   if (wordobj == NULL)
-    {
-      Py_DECREF (textobj);
-      error (_("Could not convert argument to Python string."));
-    }
+    error (_("Could not convert argument to Python string."));
 
-  resultobj = PyObject_CallMethodObjArgs ((PyObject *) obj, complete_cst,
-					  textobj, wordobj, NULL);
-  Py_DECREF (textobj);
-  Py_DECREF (wordobj);
-  if (!resultobj)
+  gdbpy_ref resultobj (PyObject_CallMethodObjArgs ((PyObject *) obj,
+						   complete_cst,
+						   textobj.get (),
+						   wordobj.get (), NULL));
+  if (resultobj == NULL)
     {
       /* Just swallow errors here.  */
       PyErr_Clear ();
     }
 
-  Py_XINCREF (resultobj);
-
-  return resultobj;
+  return resultobj.release ();
 }
 
 /* Python function called to determine the break characters of a
@@ -277,26 +272,24 @@ static void
 cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
 				 const char *text, const char *word)
 {
-  PyObject *resultobj = NULL;
-
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
   /* Calling our helper to obtain the PyObject of the Python
      function.  */
-  resultobj = cmdpy_completer_helper (command, text, word);
+  gdbpy_ref resultobj (cmdpy_completer_helper (command, text, word));
 
   /* Check if there was an error.  */
   if (resultobj == NULL)
-    goto done;
+    return;
 
-  if (PyInt_Check (resultobj))
+  if (PyInt_Check (resultobj.get ()))
     {
       /* User code may also return one of the completion constants,
 	 thus requesting that sort of completion.  We are only
 	 interested in this kind of return.  */
       long value;
 
-      if (!gdb_py_int_as_long (resultobj, &value))
+      if (!gdb_py_int_as_long (resultobj.get (), &value))
 	{
 	  /* Ignore.  */
 	  PyErr_Clear ();
@@ -310,10 +303,6 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
 	    (completers[value].completer);
 	}
     }
-
- done:
-
-  Py_XDECREF (resultobj);
 }
 
 /* Called by gdb for command completion.  */
@@ -322,29 +311,28 @@ static VEC (char_ptr) *
 cmdpy_completer (struct cmd_list_element *command,
 		 const char *text, const char *word)
 {
-  PyObject *resultobj = NULL;
   VEC (char_ptr) *result = NULL;
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
   /* Calling our helper to obtain the PyObject of the Python
      function.  */
-  resultobj = cmdpy_completer_helper (command, text, word);
+  gdbpy_ref resultobj (cmdpy_completer_helper (command, text, word));
 
   /* If the result object of calling the Python function is NULL, it
      means that there was an error.  In this case, just give up and
      return NULL.  */
   if (resultobj == NULL)
-    goto done;
+    return NULL;
 
   result = NULL;
-  if (PyInt_Check (resultobj))
+  if (PyInt_Check (resultobj.get ()))
     {
       /* User code may also return one of the completion constants,
 	 thus requesting that sort of completion.  */
       long value;
 
-      if (! gdb_py_int_as_long (resultobj, &value))
+      if (! gdb_py_int_as_long (resultobj.get (), &value))
 	{
 	  /* Ignore.  */
 	  PyErr_Clear ();
@@ -354,24 +342,24 @@ cmdpy_completer (struct cmd_list_element *command,
     }
   else
     {
-      PyObject *iter = PyObject_GetIter (resultobj);
-      PyObject *elt;
+      gdbpy_ref iter (PyObject_GetIter (resultobj.get ()));
 
       if (iter == NULL)
-	goto done;
+	return NULL;
 
-      while ((elt = PyIter_Next (iter)) != NULL)
+      while (true)
 	{
+	  gdbpy_ref elt (PyIter_Next (iter.get ()));
+	  if (elt == NULL)
+	    break;
 
-	  if (! gdbpy_is_string (elt))
+	  if (! gdbpy_is_string (elt.get ()))
 	    {
 	      /* Skip problem elements.  */
-	      Py_DECREF (elt);
 	      continue;
 	    }
 	  gdb::unique_xmalloc_ptr<char>
-	    item (python_string_to_host_string (elt));
-	  Py_DECREF (elt);
+	    item (python_string_to_host_string (elt.get ()));
 	  if (item == NULL)
 	    {
 	      /* Skip problem elements.  */
@@ -381,18 +369,12 @@ cmdpy_completer (struct cmd_list_element *command,
 	  VEC_safe_push (char_ptr, result, item.release ());
 	}
 
-      Py_DECREF (iter);
-
       /* If we got some results, ignore problems.  Otherwise, report
 	 the problem.  */
       if (result != NULL && PyErr_Occurred ())
 	PyErr_Clear ();
     }
 
- done:
-
-  Py_XDECREF (resultobj);
-
   return result;
 }
 
@@ -587,21 +569,18 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
     }
   if (PyObject_HasAttr (self, gdbpy_doc_cst))
     {
-      PyObject *ds_obj = PyObject_GetAttr (self, gdbpy_doc_cst);
+      gdbpy_ref ds_obj (PyObject_GetAttr (self, gdbpy_doc_cst));
 
-      if (ds_obj && gdbpy_is_string (ds_obj))
+      if (ds_obj != NULL && gdbpy_is_string (ds_obj.get ()))
 	{
-	  docstring = python_string_to_host_string (ds_obj).release ();
+	  docstring = python_string_to_host_string (ds_obj.get ()).release ();
 	  if (docstring == NULL)
 	    {
 	      xfree (cmd_name);
 	      xfree (pfx_name);
-	      Py_DECREF (ds_obj);
 	      return -1;
 	    }
 	}
-
-      Py_XDECREF (ds_obj);
     }
   if (! docstring)
     docstring = xstrdup (_("This command is not documented."));

Re: [RFA 08/13] Use gdbpy_ref in python.c

[Pedro Alves]

On 11/20/2016 08:41 PM, Tom Tromey wrote:
> This changes more places in python.c to use gdbpy_ref.
> 
> Additionally, previously gdbpy_apply_type_printers would return
> EXT_LANG_RC_ERROR if a type printer returned None.  However, that
> doesn't seem correct to me; this patch changes it to return
> EXT_LANG_RC_NOP in this case.

Agreed, this is what the value printers do, AFAICT from
gdbpy_apply_val_pretty_printer.

Does that result in user/script-visible behavior?  Should
this be covered by some test?

Anyway, LGTM.

Thanks,
Pedro Alves

Re: [RFA 13/13] Remove make_cleanup_py_decref and make_cleanup_py_xdecref

[Pedro Alves]

On 11/20/2016 08:41 PM, Tom Tromey wrote:
> make_cleanup_py_decref and make_cleanup_py_xdecref are now unused, so
> this patch removes themm.  Future Python changes should use gdbpy_ref
> instead.
> 
> 2016-11-20  Tom Tromey  <tom@tromey.com>
> 
> 	* python/python-internal.h (make_cleanup_py_decref)
> 	(make_cleanup_py_xdecref): Don't declare.
> 	* python/py-utils.c (py_decref, make_cleanup_py_decref)
> 	(py_xdecref, make_cleanup_py_xdecref): Remove.

Hurray!  LGTM.

Thanks,
Pedro Alves

Re: [RFA 00/13] series #4 of c++ in python

[Pedro Alves]

On 11/20/2016 08:41 PM, Tom Tromey wrote:
> This is another series addressing the use of C++ in the Python layer.
> For this series, I looked at two things: explicit decrefs and use of
> make_cleanup_py_decref.
> 
> After this series most of the explicit decrefs are done in Python
> class destructors.  So, this is probably the last such series.  It is
> based on the earlier series.
> 
> This series fixes a few latent bugs.  It culminates in the removal of
> make_cleanup_py_decref and make_cleanup_py_xdecref.
> 
> I've built and tested it locally against Python 2 and Python 3, but
> I'm also sending it through the buildbot.

This all LGTM.  Please push.

Thanks,
Pedro Alves

Re: [RFA 08/13] Use gdbpy_ref in python.c

[Tom Tromey]

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

>> Additionally, previously gdbpy_apply_type_printers would return
>> EXT_LANG_RC_ERROR if a type printer returned None.  However, that
>> doesn't seem correct to me; this patch changes it to return
>> EXT_LANG_RC_NOP in this case.

Pedro> Agreed, this is what the value printers do, AFAICT from
Pedro> gdbpy_apply_val_pretty_printer.

Pedro> Does that result in user/script-visible behavior?  Should
Pedro> this be covered by some test?

The caller is apply_ext_lang_type_printers and it does:

  ALL_ENABLED_EXTENSION_LANGUAGES (i, extlang)
...
      switch (rc)
	{
...
	case EXT_LANG_RC_ERROR:
	  return NULL;
	case EXT_LANG_RC_NOP:
	  break;

So, error means all the subsequent printer will be skipped, but nop
means they will be considered.

I think a test case would be reasonable, but I would rather not write
it.  I'm going to push this series in, but if you think the test is
required, I will write a follow-up patch to just revert this bit to the
status quo ante.

Tom