[RFA 00/20] more use of C++ in the Python layer

[RFA 00/20] more use of C++ in the Python layer

[Tom Tromey]

This patch series removes many more cleanups from the Python layer.
It mostly replaces ensure_python_env, but also fixes a few spots that
were missing in the earlier series.  You may also want to take notice
of the htab_up patch, which adds a unique_ptr typedef for managing
libiberty hash tables.

These patches require my earlier gdbpy_reference series.

I split this series up quite a bit to ease review.  The earlier
patches are all relatively straightforward translations.  Later
patches involve multiple changes in a function: if a function used
ensure_python_env and also had other cleanups (specifically
Python-related ones), I've tried to transform the entire function at
once, to avoid any ordering possible issues.

A few uses of ensure_python_env remain after this series.  I will
address them later.  My end goal is to remove all cleanups from the
Python layer, and ideally all the goto-based control flow as well;
this isn't totally achievable until much of the rest of gdb has moved
away from cleanups, but it's certainly possible to get close.

I've built and tested this locally.  I will run it through the
buildbot as well -- long story, but I have to wait until one upstream
patch lands to reset the baseline before I can do this.

Tom

[RFA 03/20] Use gdbpy_enter in py-cmd.c

[Tom Tromey]

Change py-cmd.c to use gdbpy_enter.

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

	* python/py-cmd.c (cmdpy_destroyer)
	(cmdpy_completer_handle_brkchars, cmdpy_completer): Use
	gdbpy_enter.
---
 gdb/ChangeLog       |  6 ++++++
 gdb/python/py-cmd.c | 13 +++----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ac27ee7..2478e7e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-cmd.c (cmdpy_destroyer)
+	(cmdpy_completer_handle_brkchars, cmdpy_completer): Use
+	gdbpy_enter.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-breakpoint.c (gdbpy_breakpoint_cond_says_stop): Use
 	gdbpy_enter.
 	(gdbpy_breakpoint_has_cond): Likewise.
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 5fbd25c..3b82788 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -96,9 +96,8 @@ static void
 cmdpy_destroyer (struct cmd_list_element *self, void *context)
 {
   cmdpy_object *cmd;
-  struct cleanup *cleanup;
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   /* Release our hold on the command object.  */
   cmd = (cmdpy_object *) context;
@@ -110,8 +109,6 @@ cmdpy_destroyer (struct cmd_list_element *self, void *context)
   xfree ((char *) self->name);
   xfree ((char *) self->doc);
   xfree ((char *) self->prefixname);
-
-  do_cleanups (cleanup);
 }
 
 /* Called by gdb to invoke the command.  */
@@ -287,9 +284,8 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
 				 const char *text, const char *word)
 {
   PyObject *resultobj = NULL;
-  struct cleanup *cleanup;
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   /* Calling our helper to obtain the PyObject of the Python
      function.  */
@@ -324,7 +320,6 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
  done:
 
   Py_XDECREF (resultobj);
-  do_cleanups (cleanup);
 }
 
 /* Called by gdb for command completion.  */
@@ -335,9 +330,8 @@ cmdpy_completer (struct cmd_list_element *command,
 {
   PyObject *resultobj = NULL;
   VEC (char_ptr) *result = NULL;
-  struct cleanup *cleanup;
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   /* Calling our helper to obtain the PyObject of the Python
      function.  */
@@ -404,7 +398,6 @@ cmdpy_completer (struct cmd_list_element *command,
  done:
 
   Py_XDECREF (resultobj);
-  do_cleanups (cleanup);
 
   return result;
 }
-- 
2.7.4

[RFA 10/20] Use gdbpy_enter in py-xmethods.c

[Tom Tromey]

Change the simple parts of py-xmethods.c to use gdbpy_enter.

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

	* python/py-xmethods.c (gdbpy_free_xmethod_worker_data)
	(gdbpy_clone_xmethod_worker_data): Use gdbpy_enter.
---
 gdb/ChangeLog            |  5 +++++
 gdb/python/py-xmethods.c | 10 ++--------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9e2d08c..8a5a159 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-xmethods.c (gdbpy_free_xmethod_worker_data)
+	(gdbpy_clone_xmethod_worker_data): Use gdbpy_enter.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-type.c (save_objfile_types): Use gdbpy_enter.
 
 2016-11-10  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
index 48205d8..2c86478 100644
--- a/gdb/python/py-xmethods.c
+++ b/gdb/python/py-xmethods.c
@@ -52,18 +52,15 @@ gdbpy_free_xmethod_worker_data (const struct extension_language_defn *extlang,
 				void *data)
 {
   struct gdbpy_worker_data *worker_data = (struct gdbpy_worker_data *) data;
-  struct cleanup *cleanups;
 
   gdb_assert (worker_data->worker != NULL && worker_data->this_type != NULL);
 
   /* We don't do much here, but we still need the GIL.  */
-  cleanups = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   Py_DECREF (worker_data->worker);
   Py_DECREF (worker_data->this_type);
   xfree (worker_data);
-
-  do_cleanups (cleanups);
 }
 
 /* Implementation of clone_xmethod_worker_data for Python.  */
@@ -74,12 +71,11 @@ gdbpy_clone_xmethod_worker_data (const struct extension_language_defn *extlang,
 {
   struct gdbpy_worker_data *worker_data
     = (struct gdbpy_worker_data *) data, *new_data;
-  struct cleanup *cleanups;
 
   gdb_assert (worker_data->worker != NULL && worker_data->this_type != NULL);
 
   /* We don't do much here, but we still need the GIL.  */
-  cleanups = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   new_data = XCNEW (struct gdbpy_worker_data);
   new_data->worker = worker_data->worker;
@@ -87,8 +83,6 @@ gdbpy_clone_xmethod_worker_data (const struct extension_language_defn *extlang,
   Py_INCREF (new_data->worker);
   Py_INCREF (new_data->this_type);
 
-  do_cleanups (cleanups);
-
   return new_data;
 }
 
-- 
2.7.4

[RFA 17/20] Use gdbpy_reference in invoke_match_method

[Tom Tromey]

Change invoke_match_method to use gdbpy_reference.
I neglected to convert this function in my earlier series.

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

	* python/py-xmethods.c (invoke_match_method): Use
	gdbpy_reference.
---
 gdb/ChangeLog            |  5 +++++
 gdb/python/py-xmethods.c | 51 ++++++++++++------------------------------------
 2 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fef6a71..cb0492d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-xmethods.c (invoke_match_method): Use
+	gdbpy_reference.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-xmethods.c (gdbpy_get_matching_xmethod_workers): use
 	gdbpy_enter, gdbpy_reference.
 
diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
index ca83b0b..a5319cc 100644
--- a/gdb/python/py-xmethods.c
+++ b/gdb/python/py-xmethods.c
@@ -94,59 +94,34 @@ static PyObject *
 invoke_match_method (PyObject *matcher, PyObject *py_obj_type,
 		     const char *xmethod_name)
 {
-  PyObject *py_xmethod_name;
-  PyObject *match_method, *enabled_field, *match_result;
-  struct cleanup *cleanups;
   int enabled;
 
-  cleanups = make_cleanup (null_cleanup, NULL);
-
-  enabled_field = PyObject_GetAttrString (matcher, enabled_field_name);
+  gdbpy_reference enabled_field (PyObject_GetAttrString (matcher,
+							 enabled_field_name));
   if (enabled_field == NULL)
-    {
-      do_cleanups (cleanups);
-      return NULL;
-    }
-  make_cleanup_py_decref (enabled_field);
+    return NULL;
 
-  enabled = PyObject_IsTrue (enabled_field);
+  enabled = PyObject_IsTrue (enabled_field.get ());
   if (enabled == -1)
-    {
-      do_cleanups (cleanups);
-      return NULL;
-    }
+    return NULL;
   if (enabled == 0)
     {
       /* Return 'None' if the matcher is not enabled.  */
-      do_cleanups (cleanups);
       Py_RETURN_NONE;
     }
 
-  match_method = PyObject_GetAttrString (matcher, match_method_name);
+  gdbpy_reference match_method (PyObject_GetAttrString (matcher,
+							match_method_name));
   if (match_method == NULL)
-    {
-      do_cleanups (cleanups);
-      return NULL;
-    }
-  make_cleanup_py_decref (match_method);
+    return NULL;
 
-  py_xmethod_name = PyString_FromString (xmethod_name);
+  gdbpy_reference py_xmethod_name (PyString_FromString (xmethod_name));
   if (py_xmethod_name == NULL)
-    {
-      do_cleanups (cleanups);
-      return NULL;
-    }
-  make_cleanup_py_decref (py_xmethod_name);
-
-  match_result = PyObject_CallMethodObjArgs (matcher,
-					     py_match_method_name,
-					     py_obj_type,
-					     py_xmethod_name,
-					     NULL);
-
-  do_cleanups (cleanups);
+    return NULL;
 
-  return match_result;
+  return PyObject_CallMethodObjArgs (matcher, py_match_method_name,
+				     py_obj_type, py_xmethod_name.get (),
+				     NULL);
 }
 
 /* Implementation of get_matching_xmethod_workers for Python.  */
-- 
2.7.4

[RFA 09/20] Use gdbpy_enter in py-type.c

[Tom Tromey]

Change py-type.c to use gdbpy_enter.

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

	* python/py-type.c (save_objfile_types): Use gdbpy_enter.
---
 gdb/ChangeLog        | 4 ++++
 gdb/python/py-type.c | 5 +----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e93e36b..9e2d08c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-type.c (save_objfile_types): Use gdbpy_enter.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/python.c (gdbpy_eval_from_control_command)
 	(gdbpy_source_script, gdbpy_run_events)
 	(gdbpy_source_objfile_script, gdbpy_execute_objfile_script)
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index c832be9..ff1e4c4 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1048,14 +1048,13 @@ save_objfile_types (struct objfile *objfile, void *datum)
 {
   type_object *obj = (type_object *) datum;
   htab_t copied_types;
-  struct cleanup *cleanup;
 
   if (!gdb_python_initialized)
     return;
 
   /* This prevents another thread from freeing the objects we're
      operating on.  */
-  cleanup = ensure_python_env (get_objfile_arch (objfile), current_language);
+  gdbpy_enter enter_py (get_objfile_arch (objfile), current_language);
 
   copied_types = create_copied_types_hash (objfile);
 
@@ -1074,8 +1073,6 @@ save_objfile_types (struct objfile *objfile, void *datum)
     }
 
   htab_delete (copied_types);
-
-  do_cleanups (cleanup);
 }
 
 static void
-- 
2.7.4

[RFA 16/20] Use gdbpy_enter in gdbpy_get_matching_xmethod_workers

[Tom Tromey]

Change gdbpy_get_matching_xmethod_workers to use gdbpy_enter and
gdbpy_reference.

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

	* python/py-xmethods.c (gdbpy_get_matching_xmethod_workers): use
	gdbpy_enter, gdbpy_reference.
---
 gdb/ChangeLog            |   5 ++
 gdb/python/py-xmethods.c | 153 ++++++++++++++++++-----------------------------
 2 files changed, 63 insertions(+), 95 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1a415fb..fef6a71 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-xmethods.c (gdbpy_get_matching_xmethod_workers): use
+	gdbpy_enter, gdbpy_reference.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/python.c (python_interactive_command): Use gdbpy_enter.
 
 2016-11-10  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
index 2c86478..ca83b0b 100644
--- a/gdb/python/py-xmethods.c
+++ b/gdb/python/py-xmethods.c
@@ -26,6 +26,7 @@
 
 #include "python.h"
 #include "python-internal.h"
+#include "py-ref.h"
 
 static const char enabled_field_name[] = "enabled";
 static const char match_method_name[] = "match";
@@ -156,33 +157,26 @@ gdbpy_get_matching_xmethod_workers
    struct type *obj_type, const char *method_name,
    xmethod_worker_vec **dm_vec)
 {
-  struct cleanup *cleanups;
   struct objfile *objfile;
   VEC (xmethod_worker_ptr) *worker_vec = NULL;
-  PyObject *py_type, *py_progspace;
-  PyObject *py_xmethod_matcher_list = NULL, *list_iter, *matcher;
+  PyObject *py_progspace;
 
   gdb_assert (obj_type != NULL && method_name != NULL);
 
-  cleanups = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  py_type = type_to_type_object (obj_type);
+  gdbpy_reference py_type (type_to_type_object (obj_type));
   if (py_type == NULL)
     {
       gdbpy_print_stack ();
-      do_cleanups (cleanups);
-
       return EXT_LANG_RC_ERROR;
     }
-  make_cleanup_py_decref (py_type);
 
   /* Create an empty list of debug methods.  */
-  py_xmethod_matcher_list = PyList_New (0);
+  gdbpy_reference py_xmethod_matcher_list (PyList_New (0));
   if (py_xmethod_matcher_list == NULL)
     {
       gdbpy_print_stack ();
-      do_cleanups (cleanups);
-
       return EXT_LANG_RC_ERROR;
     }
 
@@ -192,28 +186,23 @@ gdbpy_get_matching_xmethod_workers
   ALL_OBJFILES (objfile)
     {
       PyObject *py_objfile = objfile_to_objfile_object (objfile);
-      PyObject *objfile_matchers, *temp = py_xmethod_matcher_list;
 
       if (py_objfile == NULL)
 	{
 	  gdbpy_print_stack ();
-	  Py_DECREF (py_xmethod_matcher_list);
-	  do_cleanups (cleanups);
-
 	  return EXT_LANG_RC_ERROR;
 	}
 
-      objfile_matchers = objfpy_get_xmethods (py_objfile, NULL);
-      py_xmethod_matcher_list = PySequence_Concat (temp, objfile_matchers);
-      Py_DECREF (temp);
-      Py_DECREF (objfile_matchers);
-      if (py_xmethod_matcher_list == NULL)
+      gdbpy_reference objfile_matchers (objfpy_get_xmethods (py_objfile, NULL));
+      gdbpy_reference temp (PySequence_Concat (py_xmethod_matcher_list.get (),
+					       objfile_matchers.get ()));
+      if (temp == NULL)
 	{
 	  gdbpy_print_stack ();
-	  do_cleanups (cleanups);
-
 	  return EXT_LANG_RC_ERROR;
 	}
+
+      py_xmethod_matcher_list = temp;
     }
 
   /* Gather debug methods matchers registered with the current program
@@ -221,26 +210,21 @@ gdbpy_get_matching_xmethod_workers
   py_progspace = pspace_to_pspace_object (current_program_space);
   if (py_progspace != NULL)
     {
-      PyObject *temp = py_xmethod_matcher_list;
-      PyObject *pspace_matchers = pspy_get_xmethods (py_progspace, NULL);
+      gdbpy_reference pspace_matchers (pspy_get_xmethods (py_progspace, NULL));
 
-      py_xmethod_matcher_list = PySequence_Concat (temp, pspace_matchers);
-      Py_DECREF (temp);
-      Py_DECREF (pspace_matchers);
-      if (py_xmethod_matcher_list == NULL)
+      gdbpy_reference temp (PySequence_Concat (py_xmethod_matcher_list.get (),
+					       pspace_matchers.get ()));
+      if (temp == NULL)
 	{
 	  gdbpy_print_stack ();
-	  do_cleanups (cleanups);
-
 	  return EXT_LANG_RC_ERROR;
 	}
+
+      py_xmethod_matcher_list = temp;
     }
   else
     {
       gdbpy_print_stack ();
-      Py_DECREF (py_xmethod_matcher_list);
-      do_cleanups (cleanups);
-
       return EXT_LANG_RC_ERROR;
     }
 
@@ -248,117 +232,96 @@ gdbpy_get_matching_xmethod_workers
   if (gdb_python_module != NULL
       && PyObject_HasAttrString (gdb_python_module, matchers_attr_str))
     {
-      PyObject *gdb_matchers;
-      PyObject *temp = py_xmethod_matcher_list;
-
-      gdb_matchers = PyObject_GetAttrString (gdb_python_module,
-					     matchers_attr_str);
+      gdbpy_reference gdb_matchers (PyObject_GetAttrString (gdb_python_module,
+							    matchers_attr_str));
       if (gdb_matchers != NULL)
 	{
-	  py_xmethod_matcher_list = PySequence_Concat (temp, gdb_matchers);
-	  Py_DECREF (temp);
-	  Py_DECREF (gdb_matchers);
-	  if (py_xmethod_matcher_list == NULL)
+	  gdbpy_reference temp
+	    (PySequence_Concat (py_xmethod_matcher_list.get (),
+				gdb_matchers.get ()));
+	  if (temp == NULL)
 	    {
 	      gdbpy_print_stack ();
-	      do_cleanups (cleanups);
-
 	      return EXT_LANG_RC_ERROR;
 	    }
+
+	  py_xmethod_matcher_list = temp;
 	}
       else
 	{
 	  gdbpy_print_stack ();
-	  Py_DECREF (py_xmethod_matcher_list);
-	  do_cleanups (cleanups);
-
 	  return EXT_LANG_RC_ERROR;
 	}
     }
 
-  /* Safe to make a cleanup for py_xmethod_matcher_list now as it
-     will not change any more.  */
-  make_cleanup_py_decref (py_xmethod_matcher_list);
-
-  list_iter = PyObject_GetIter (py_xmethod_matcher_list);
+  gdbpy_reference list_iter (PyObject_GetIter (py_xmethod_matcher_list.get ()));
   if (list_iter == NULL)
     {
       gdbpy_print_stack ();
-      do_cleanups (cleanups);
-
       return EXT_LANG_RC_ERROR;
     }
-  while ((matcher = PyIter_Next (list_iter)) != NULL)
+  while (true)
     {
-      PyObject *match_result = invoke_match_method (matcher, py_type,
-						    method_name);
+      gdbpy_reference matcher (PyIter_Next (list_iter.get ()));
+      if (matcher == NULL)
+	{
+	  if (PyErr_Occurred ())
+	    {
+	      gdbpy_print_stack ();
+	      return EXT_LANG_RC_ERROR;
+	    }
+	  break;
+	}
+
+      gdbpy_reference match_result
+	(invoke_match_method (matcher.get (), py_type.get (), method_name));
 
       if (match_result == NULL)
 	{
 	  gdbpy_print_stack ();
-	  Py_DECREF (matcher);
-	  do_cleanups (cleanups);
-
 	  return EXT_LANG_RC_ERROR;
 	}
       if (match_result == Py_None)
 	; /* This means there was no match.  */
-      else if (PySequence_Check (match_result))
+      else if (PySequence_Check (match_result.get ()))
 	{
-	  PyObject *iter = PyObject_GetIter (match_result);
-	  PyObject *py_worker;
+	  gdbpy_reference iter (PyObject_GetIter (match_result.get ()));
 
 	  if (iter == NULL)
 	    {
 	      gdbpy_print_stack ();
-	      Py_DECREF (matcher);
-	      Py_DECREF (match_result);
-	      do_cleanups (cleanups);
-
 	      return EXT_LANG_RC_ERROR;
 	    }
-	  while ((py_worker = PyIter_Next (iter)) != NULL)
+	  while (true)
 	    {
 	      struct xmethod_worker *worker;
 
-	      worker = new_python_xmethod_worker (py_worker, py_type);
+	      gdbpy_reference py_worker (PyIter_Next (iter.get ()));
+	      if (py_worker == NULL)
+		{
+		  if (PyErr_Occurred ())
+		    {
+		      gdbpy_print_stack ();
+		      return EXT_LANG_RC_ERROR;
+		    }
+		  break;
+		}
+
+	      worker = new_python_xmethod_worker (py_worker.get (),
+						  py_type.get ());
 	      VEC_safe_push (xmethod_worker_ptr, worker_vec, worker);
-	      Py_DECREF (py_worker);
-	    }
-	  Py_DECREF (iter);
-	  /* Report any error that could have occurred while iterating.  */
-	  if (PyErr_Occurred ())
-	    {
-	      gdbpy_print_stack ();
-	      Py_DECREF (matcher);
-	      Py_DECREF (match_result);
-	      do_cleanups (cleanups);
-
-	      return EXT_LANG_RC_ERROR;
 	    }
 	}
       else
 	{
 	  struct xmethod_worker *worker;
 
-	  worker = new_python_xmethod_worker (match_result, py_type);
+	  worker = new_python_xmethod_worker (match_result.get (),
+					      py_type.get ());
 	  VEC_safe_push (xmethod_worker_ptr, worker_vec, worker);
 	}
-
-      Py_DECREF (match_result);
-      Py_DECREF (matcher);
-    }
-  Py_DECREF (list_iter);
-  /* Report any error that could have occurred while iterating.  */
-  if (PyErr_Occurred ())
-    {
-      gdbpy_print_stack ();
-      do_cleanups (cleanups);
-
-      return EXT_LANG_RC_ERROR;
     }
 
-  do_cleanups (cleanups);
   *dm_vec = worker_vec;
 
   return EXT_LANG_RC_OK;
-- 
2.7.4

Re: [RFA 16/20] Use gdbpy_enter in gdbpy_get_matching_xmethod_workers

[Pedro Alves]

On 11/10/2016 10:19 PM, Tom Tromey wrote:
> @@ -221,26 +210,21 @@ gdbpy_get_matching_xmethod_workers
>    py_progspace = pspace_to_pspace_object (current_program_space);
>    if (py_progspace != NULL)
>      {
> -      PyObject *temp = py_xmethod_matcher_list;
> -      PyObject *pspace_matchers = pspy_get_xmethods (py_progspace, NULL);
> +      gdbpy_reference pspace_matchers (pspy_get_xmethods (py_progspace, NULL));
>  
> -      py_xmethod_matcher_list = PySequence_Concat (temp, pspace_matchers);
> -      Py_DECREF (temp);
> -      Py_DECREF (pspace_matchers);

I'm a little confused here.  Don't we still need to account for
these two Py_DECREFs?

> -      if (py_xmethod_matcher_list == NULL)
> +      gdbpy_reference temp (PySequence_Concat (py_xmethod_matcher_list.get (),
> +					       pspace_matchers.get ()));
> +      if (temp == NULL)
>  	{
>  	  gdbpy_print_stack ();
> -	  do_cleanups (cleanups);
> -
>  	  return EXT_LANG_RC_ERROR;


-- 
Thanks,
Pedro Alves

Re: [RFA 16/20] Use gdbpy_enter in gdbpy_get_matching_xmethod_workers

[Tom Tromey]

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

>> -      Py_DECREF (temp);
>> -      Py_DECREF (pspace_matchers);

Pedro> I'm a little confused here.  Don't we still need to account for
Pedro> these two Py_DECREFs?

The patch reads a little messily.  The new code is:

      gdbpy_reference pspace_matchers (pspy_get_xmethods (py_progspace, NULL));

      gdbpy_reference temp (PySequence_Concat (py_xmethod_matcher_list.get (),
					       pspace_matchers.get ()));
      if (temp == NULL)
	{
	  gdbpy_print_stack ();
	  return EXT_LANG_RC_ERROR;
	}

      py_xmethod_matcher_list = temp;

So the decrefs are accounted for by the destructors; and that assignment
at the end encodes an incref via the copy constructor.  Moving here
would have been a valid choice as well, like:

     py_xmethod_matcher_list = std::move (temp);

... though I deleted the move constructor since it wasn't used in the
earlier series and Jan pointed out that it had a bug.

Tom

Re: [RFA 16/20] Use gdbpy_enter in gdbpy_get_matching_xmethod_workers

[Pedro Alves]

On 11/11/2016 03:17 AM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
>>> -      Py_DECREF (temp);
>>> -      Py_DECREF (pspace_matchers);
> 
> Pedro> I'm a little confused here.  Don't we still need to account for
> Pedro> these two Py_DECREFs?
> 
> The patch reads a little messily.  The new code is:
> 
>       gdbpy_reference pspace_matchers (pspy_get_xmethods (py_progspace, NULL));
> 
>       gdbpy_reference temp (PySequence_Concat (py_xmethod_matcher_list.get (),
> 					       pspace_matchers.get ()));
>       if (temp == NULL)
> 	{
> 	  gdbpy_print_stack ();
> 	  return EXT_LANG_RC_ERROR;
> 	}
> 
>       py_xmethod_matcher_list = temp;
> 
> So the decrefs are accounted for by the destructors; and that assignment
> at the end encodes an incref via the copy constructor.  Moving here
> would have been a valid choice as well, like:

Ah, I see.

> 
>      py_xmethod_matcher_list = std::move (temp);
> 
> ... though I deleted the move constructor since it wasn't used in the
> earlier series and Jan pointed out that it had a bug.

As per my comment on the other patch, I think that's a mistake though.

Thanks,
Pedro Alves

Re: [RFA 16/20] Use gdbpy_enter in gdbpy_get_matching_xmethod_workers

[Tom Tromey]

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

>> py_xmethod_matcher_list = std::move (temp);
>> 
>> ... though I deleted the move constructor since it wasn't used in the
>> earlier series and Jan pointed out that it had a bug.

Pedro> As per my comment on the other patch, I think that's a mistake though.

I've restored the rvalue constructor and assignment operator, and I
changed this patch to use std::move, saving an incref/decref pair.

Tom

[RFA 11/20] Use gdbpy_enter in py-unwind.c

[Tom Tromey]

Change py-unwind.c to use gdbpy_enter.

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

	* python/py-unwind.c (pending_frame_invalidate): Remove.
	(pyuw_sniffer): Use gdbpy_enter and gdbpy_reference.
---
 gdb/ChangeLog          |  5 ++++
 gdb/python/py-unwind.c | 72 +++++++++++++++++++++++---------------------------
 2 files changed, 38 insertions(+), 39 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8a5a159..2101045 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-unwind.c (pending_frame_invalidate): Remove.
+	(pyuw_sniffer): Use gdbpy_enter and gdbpy_reference.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-xmethods.c (gdbpy_free_xmethod_worker_data)
 	(gdbpy_clone_xmethod_worker_data): Use gdbpy_enter.
 
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 52184bd..c9638d8 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -28,6 +28,7 @@
 #include "regcache.h"
 #include "valprint.h"
 #include "user-regs.h"
+#include "py-ref.h"
 
 #define TRACE_PY_UNWIND(level, args...) if (pyuw_debug >= level)  \
   { fprintf_unfiltered (gdb_stdlog, args); }
@@ -460,15 +461,6 @@ pending_framepy_create_unwind_info (PyObject *self, PyObject *args)
                                     frame_id_build_special (sp, pc, special));
 }
 
-/* Invalidate PendingFrame instance.  */
-
-static void
-pending_frame_invalidate (void *pyo_pending_frame)
-{
-  if (pyo_pending_frame != NULL)
-    ((pending_frame_object *) pyo_pending_frame)->frame_info = NULL;
-}
-
 /* frame_unwind.this_id method.  */
 
 static void
@@ -512,25 +504,26 @@ pyuw_sniffer (const struct frame_unwind *self, struct frame_info *this_frame,
               void **cache_ptr)
 {
   struct gdbarch *gdbarch = (struct gdbarch *) (self->unwind_data);
-  struct cleanup *cleanups = ensure_python_env (gdbarch, current_language);
-  PyObject *pyo_execute;
-  PyObject *pyo_pending_frame;
-  PyObject *pyo_unwind_info;
   cached_frame_info *cached_frame;
 
+  gdbpy_enter enter_py (gdbarch, current_language);
+
   TRACE_PY_UNWIND (3, "%s (SP=%s, PC=%s)\n", __FUNCTION__,
                    paddress (gdbarch, get_frame_sp (this_frame)),
                    paddress (gdbarch, get_frame_pc (this_frame)));
 
   /* Create PendingFrame instance to pass to sniffers.  */
-  pyo_pending_frame  = (PyObject *) PyObject_New (pending_frame_object,
-                                                  &pending_frame_object_type);
+  pending_frame_object *pfo = PyObject_New (pending_frame_object,
+					    &pending_frame_object_type);
+  gdbpy_reference pyo_pending_frame ((PyObject *) pfo);
   if (pyo_pending_frame == NULL)
-    goto error;
-  ((pending_frame_object *) pyo_pending_frame)->gdbarch = gdbarch;
-  ((pending_frame_object *) pyo_pending_frame)->frame_info = this_frame;
-  make_cleanup_py_decref (pyo_pending_frame);
-  make_cleanup (pending_frame_invalidate, (void *) pyo_pending_frame);
+    {
+      gdbpy_print_stack ();
+      return 0;
+    }
+  pfo->gdbarch = gdbarch;
+  scoped_restore invalidate_frame = make_scoped_restore (&pfo->frame_info,
+							 this_frame);
 
   /* Run unwinders.  */
   if (gdb_python_module == NULL
@@ -539,27 +532,36 @@ pyuw_sniffer (const struct frame_unwind *self, struct frame_info *this_frame,
       PyErr_SetString (PyExc_NameError,
                        "Installation error: gdb.execute_unwinders function "
                        "is missing");
-      goto error;
+      gdbpy_print_stack ();
+      return 0;
     }
-  pyo_execute = PyObject_GetAttrString (gdb_python_module, "execute_unwinders");
+  gdbpy_reference pyo_execute (PyObject_GetAttrString (gdb_python_module,
+						       "execute_unwinders"));
   if (pyo_execute == NULL)
-    goto error;
-  make_cleanup_py_decref (pyo_execute);
-  pyo_unwind_info
-      = PyObject_CallFunctionObjArgs (pyo_execute, pyo_pending_frame, NULL);
+    {
+      gdbpy_print_stack ();
+      return 0;
+    }
+
+  gdbpy_reference pyo_unwind_info
+    (PyObject_CallFunctionObjArgs (pyo_execute.get (),
+				   pyo_pending_frame.get (), NULL));
   if (pyo_unwind_info == NULL)
-    goto error;
-  make_cleanup_py_decref (pyo_unwind_info);
+    {
+      gdbpy_print_stack ();
+      return 0;
+    }
   if (pyo_unwind_info == Py_None)
-    goto cannot_unwind;
+    return 0;
 
   /* Received UnwindInfo, cache data.  */
-  if (PyObject_IsInstance (pyo_unwind_info,
+  if (PyObject_IsInstance (pyo_unwind_info.get (),
                            (PyObject *) &unwind_info_object_type) <= 0)
     error (_("A Unwinder should return gdb.UnwindInfo instance."));
 
   {
-    unwind_info_object *unwind_info = (unwind_info_object *) pyo_unwind_info;
+    unwind_info_object *unwind_info =
+      (unwind_info_object *) pyo_unwind_info.get ();
     int reg_count = VEC_length (saved_reg, unwind_info->saved_regs);
     saved_reg *reg;
     int i;
@@ -590,15 +592,7 @@ pyuw_sniffer (const struct frame_unwind *self, struct frame_info *this_frame,
   }
 
   *cache_ptr = cached_frame;
-  do_cleanups (cleanups);
   return 1;
-
- error:
-  gdbpy_print_stack ();
-  /* Fallthrough.  */
- cannot_unwind:
-  do_cleanups (cleanups);
-  return 0;
 }
 
 /* Frame cache release shim.  */
-- 
2.7.4

[RFA 06/20] Use gdbpy_enter in py-objfile.c

[Tom Tromey]

Change py-objfile.c to use gdbpy_enter.

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

	* python/py-objfile.c (py_free_objfile): Use gdbpy_enter.
---
 gdb/ChangeLog           | 4 ++++
 gdb/python/py-objfile.c | 4 +---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c1d016f..6acb87a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-objfile.c (py_free_objfile): Use gdbpy_enter.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-inferior.c (python_on_normal_stop, python_on_resume)
 	(python_on_inferior_call_pre, python_on_inferior_call_post)
 	(python_on_memory_change, python_on_register_change)
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index 1972fb5..497d0ba 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -612,13 +612,11 @@ gdbpy_lookup_objfile (PyObject *self, PyObject *args, PyObject *kw)
 static void
 py_free_objfile (struct objfile *objfile, void *datum)
 {
-  struct cleanup *cleanup;
   objfile_object *object = (objfile_object *) datum;
 
-  cleanup = ensure_python_env (get_objfile_arch (objfile), current_language);
+  gdbpy_enter enter_py (get_objfile_arch (objfile), current_language);
   object->objfile = NULL;
   Py_DECREF ((PyObject *) object);
-  do_cleanups (cleanup);
 }
 
 /* Return a borrowed reference to the Python object of type Objfile
-- 
2.7.4

[RFA 13/20] Use gdbpy_enter in py-prettyprint.c

[Tom Tromey]

Change py-prettyprint.c to use gdbpy_enter.

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

	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer): Use
	gdbpy_enter, gdbpy_reference, unique_xmalloc_ptr.
---
 gdb/ChangeLog               |  5 +++++
 gdb/python/py-prettyprint.c | 45 +++++++++++++++------------------------------
 2 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 80d4329..a8237a8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer): Use
+	gdbpy_enter, gdbpy_reference, unique_xmalloc_ptr.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* utils.h (htab_deleter): New struct.
 	(htab_up): New typedef.
 	* python/py-framefilter.c (gdbpy_apply_frame_filter): Use
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index 58f43e7..d2ef920 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -684,12 +684,7 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
 				const struct language_defn *language)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
-  PyObject *printer = NULL;
-  PyObject *val_obj = NULL;
   struct value *value;
-  gdb::unique_xmalloc_ptr<char> hint;
-  struct cleanup *cleanups;
-  enum ext_lang_rc result = EXT_LANG_RC_NOP;
   enum string_repr_result print_result;
 
   /* No pretty-printer support for unavailable values.  */
@@ -699,7 +694,7 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   if (!gdb_python_initialized)
     return EXT_LANG_RC_NOP;
 
-  cleanups = ensure_python_env (gdbarch, language);
+  gdbpy_enter enter_py (gdbarch, language);
 
   /* Instantiate the printer.  */
   if (valaddr)
@@ -715,47 +710,37 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
       && VALUE_LVAL (value) != lval_computed)
     set_value_address (value, address + embedded_offset);
 
-  val_obj = value_to_value_object (value);
-  if (! val_obj)
+  gdbpy_reference val_obj (value_to_value_object (value));
+  if (val_obj == NULL)
     {
-      result = EXT_LANG_RC_ERROR;
-      goto done;
+      print_stack_unless_memory_error (stream);
+      return EXT_LANG_RC_ERROR;
     }
 
   /* Find the constructor.  */
-  printer = find_pretty_printer (val_obj);
-  Py_DECREF (val_obj);
-
+  gdbpy_reference printer (find_pretty_printer (val_obj.get ()));
   if (printer == NULL)
     {
-      result = EXT_LANG_RC_ERROR;
-      goto done;
+      print_stack_unless_memory_error (stream);
+      return EXT_LANG_RC_ERROR;
     }
 
-  make_cleanup_py_decref (printer);
   if (printer == Py_None)
-    {
-      result = EXT_LANG_RC_NOP;
-      goto done;
-    }
+    return EXT_LANG_RC_NOP;
 
   /* If we are printing a map, we want some special formatting.  */
-  hint = gdbpy_get_display_hint (printer);
+  gdb::unique_xmalloc_ptr<char> hint (gdbpy_get_display_hint (printer.get ()));
 
   /* Print the section */
-  print_result = print_string_repr (printer, hint.get (), stream, recurse,
-				    options, language, gdbarch);
+  print_result = print_string_repr (printer.get (), hint.get (), stream,
+				    recurse, options, language, gdbarch);
   if (print_result != string_repr_error)
-    print_children (printer, hint.get (), stream, recurse, options, language,
-		    print_result == string_repr_none);
+    print_children (printer.get (), hint.get (), stream, recurse, options,
+		    language, print_result == string_repr_none);
 
-  result = EXT_LANG_RC_OK;
-
- done:
   if (PyErr_Occurred ())
     print_stack_unless_memory_error (stream);
-  do_cleanups (cleanups);
-  return result;
+  return EXT_LANG_RC_OK;
 }
 
 
-- 
2.7.4

[RFA 02/20] Use gdbpy_enter in py-breakpoint.c

[Tom Tromey]

Change py-breakpoint.c to use gdbpy_enter.

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

	* python/py-breakpoint.c (gdbpy_breakpoint_cond_says_stop): Use
	gdbpy_enter.
	(gdbpy_breakpoint_has_cond): Likewise.
---
 gdb/ChangeLog              |  6 ++++++
 gdb/python/py-breakpoint.c | 14 ++++----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 460acf2..ac27ee7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-breakpoint.c (gdbpy_breakpoint_cond_says_stop): Use
+	gdbpy_enter.
+	(gdbpy_breakpoint_has_cond): Likewise.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/python.c (gdbpy_enter): New constructor.
 	(~gdbpy_enter): New destructor.
 	(restore_python_env, ensure_python_env): Rewrite.
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index ee306c1..c23dba6 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -791,14 +791,14 @@ gdbpy_breakpoint_cond_says_stop (const struct extension_language_defn *extlang,
   struct gdbpy_breakpoint_object *bp_obj = b->py_bp_object;
   PyObject *py_bp = (PyObject *) bp_obj;
   struct gdbarch *garch;
-  struct cleanup *cleanup;
 
   if (bp_obj == NULL)
     return EXT_LANG_BP_STOP_UNSET;
 
   stop = -1;
   garch = b->gdbarch ? b->gdbarch : get_current_arch ();
-  cleanup = ensure_python_env (garch, current_language);
+
+  gdbpy_enter enter_py (garch, current_language);
 
   if (bp_obj->is_finish_bp)
     bpfinishpy_pre_stop_hook (bp_obj);
@@ -829,8 +829,6 @@ gdbpy_breakpoint_cond_says_stop (const struct extension_language_defn *extlang,
   if (bp_obj->is_finish_bp)
     bpfinishpy_post_stop_hook (bp_obj);
 
-  do_cleanups (cleanup);
-
   if (stop < 0)
     return EXT_LANG_BP_STOP_UNSET;
   return stop ? EXT_LANG_BP_STOP_YES : EXT_LANG_BP_STOP_NO;
@@ -844,21 +842,17 @@ int
 gdbpy_breakpoint_has_cond (const struct extension_language_defn *extlang,
 			   struct breakpoint *b)
 {
-  int has_func;
   PyObject *py_bp;
   struct gdbarch *garch;
-  struct cleanup *cleanup;
 
   if (b->py_bp_object == NULL)
     return 0;
 
   py_bp = (PyObject *) b->py_bp_object;
   garch = b->gdbarch ? b->gdbarch : get_current_arch ();
-  cleanup = ensure_python_env (garch, current_language);
-  has_func = PyObject_HasAttrString (py_bp, stop_func);
-  do_cleanups (cleanup);
 
-  return has_func;
+  gdbpy_enter enter_py (garch, current_language);
+  return PyObject_HasAttrString (py_bp, stop_func);
 }
 
 \f
-- 
2.7.4

[RFA 05/20] Use gdbpy_enter in py-inferior.c

[Tom Tromey]

Change py-inferior.c to use gdbpy_enter.

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

	* python/py-inferior.c (python_on_normal_stop, python_on_resume)
	(python_on_inferior_call_pre, python_on_inferior_call_post)
	(python_on_memory_change, python_on_register_change)
	(python_inferior_exit, python_new_objfile, add_thread_object)
	(delete_thread_object, py_free_inferior): Use gdbpy_enter.
---
 gdb/ChangeLog            |  8 ++++++
 gdb/python/py-inferior.c | 74 ++++++++++--------------------------------------
 2 files changed, 23 insertions(+), 59 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2deede2..c1d016f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-inferior.c (python_on_normal_stop, python_on_resume)
+	(python_on_inferior_call_pre, python_on_inferior_call_post)
+	(python_on_memory_change, python_on_register_change)
+	(python_inferior_exit, python_new_objfile, add_thread_object)
+	(delete_thread_object, py_free_inferior): Use gdbpy_enter.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-finishbreakpoint.c (bpfinishpy_handle_stop)
 	(bpfinishpy_handle_exit): Use gdbpy_enter.
 
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index b343c0f..8f80d04 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -81,7 +81,6 @@ extern PyTypeObject membuf_object_type
 static void
 python_on_normal_stop (struct bpstats *bs, int print_frame)
 {
-  struct cleanup *cleanup;
   enum gdb_signal stop_signal;
 
   if (!gdb_python_initialized)
@@ -92,28 +91,22 @@ python_on_normal_stop (struct bpstats *bs, int print_frame)
 
   stop_signal = inferior_thread ()->suspend.stop_signal;
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   if (emit_stop_event (bs, stop_signal) < 0)
     gdbpy_print_stack ();
-
-  do_cleanups (cleanup);
 }
 
 static void
 python_on_resume (ptid_t ptid)
 {
-  struct cleanup *cleanup;
-
   if (!gdb_python_initialized)
     return;
 
-  cleanup = ensure_python_env (target_gdbarch (), current_language);
+  gdbpy_enter enter_py (target_gdbarch (), current_language);
 
   if (emit_continue_event (ptid) < 0)
     gdbpy_print_stack ();
-
-  do_cleanups (cleanup);
 }
 
 /* Callback, registered as an observer, that notifies Python listeners
@@ -122,14 +115,10 @@ python_on_resume (ptid_t ptid)
 static void
 python_on_inferior_call_pre (ptid_t thread, CORE_ADDR address)
 {
-  struct cleanup *cleanup;
-
-  cleanup = ensure_python_env (target_gdbarch (), current_language);
+  gdbpy_enter enter_py (target_gdbarch (), current_language);
 
   if (emit_inferior_call_event (INFERIOR_CALL_PRE, thread, address) < 0)
     gdbpy_print_stack ();
-
-  do_cleanups (cleanup);
 }
 
 /* Callback, registered as an observer, that notifies Python listeners
@@ -138,14 +127,10 @@ python_on_inferior_call_pre (ptid_t thread, CORE_ADDR address)
 static void
 python_on_inferior_call_post (ptid_t thread, CORE_ADDR address)
 {
-  struct cleanup *cleanup;
-
-  cleanup = ensure_python_env (target_gdbarch (), current_language);
+  gdbpy_enter enter_py (target_gdbarch (), current_language);
 
   if (emit_inferior_call_event (INFERIOR_CALL_POST, thread, address) < 0)
     gdbpy_print_stack ();
-
-  do_cleanups (cleanup);
 }
 
 /* Callback, registered as an observer, that notifies Python listeners
@@ -155,14 +140,10 @@ python_on_inferior_call_post (ptid_t thread, CORE_ADDR address)
 static void
 python_on_memory_change (struct inferior *inferior, CORE_ADDR addr, ssize_t len, const bfd_byte *data)
 {
-  struct cleanup *cleanup;
-
-  cleanup = ensure_python_env (target_gdbarch (), current_language);
+  gdbpy_enter enter_py (target_gdbarch (), current_language);
 
   if (emit_memory_changed_event (addr, len) < 0)
     gdbpy_print_stack ();
-
-  do_cleanups (cleanup);
 }
 
 /* Callback, registered as an observer, that notifies Python listeners
@@ -172,34 +153,27 @@ python_on_memory_change (struct inferior *inferior, CORE_ADDR addr, ssize_t len,
 static void
 python_on_register_change (struct frame_info *frame, int regnum)
 {
-  struct cleanup *cleanup;
-
-  cleanup = ensure_python_env (target_gdbarch (), current_language);
+  gdbpy_enter enter_py (target_gdbarch (), current_language);
 
   if (emit_register_changed_event (frame, regnum) < 0)
     gdbpy_print_stack ();
-
-  do_cleanups (cleanup);
 }
 
 static void
 python_inferior_exit (struct inferior *inf)
 {
-  struct cleanup *cleanup;
   const LONGEST *exit_code = NULL;
 
   if (!gdb_python_initialized)
     return;
 
-  cleanup = ensure_python_env (target_gdbarch (), current_language);
+  gdbpy_enter enter_py (target_gdbarch (), current_language);
 
   if (inf->has_exit_code)
     exit_code = &inf->exit_code;
 
   if (emit_exited_event (exit_code, inf) < 0)
     gdbpy_print_stack ();
-
-  do_cleanups (cleanup);
 }
 
 /* Callback used to notify Python listeners about new objfiles loaded in the
@@ -209,15 +183,13 @@ python_inferior_exit (struct inferior *inf)
 static void
 python_new_objfile (struct objfile *objfile)
 {
-  struct cleanup *cleanup;
-
   if (!gdb_python_initialized)
     return;
 
-  cleanup = ensure_python_env (objfile != NULL
-			       ? get_objfile_arch (objfile)
-			       : target_gdbarch (),
-			       current_language);
+  gdbpy_enter enter_py (objfile != NULL
+			? get_objfile_arch (objfile)
+			: target_gdbarch (),
+			current_language);
 
   if (objfile == NULL)
     {
@@ -229,8 +201,6 @@ python_new_objfile (struct objfile *objfile)
       if (emit_new_objfile_event (objfile) < 0)
 	gdbpy_print_stack ();
     }
-
-  do_cleanups (cleanup);
 }
 
 /* Return a reference to the Python object of type Inferior
@@ -312,7 +282,6 @@ find_thread_object (ptid_t ptid)
 static void
 add_thread_object (struct thread_info *tp)
 {
-  struct cleanup *cleanup;
   thread_object *thread_obj;
   inferior_object *inf_obj;
   struct threadlist_entry *entry;
@@ -320,13 +289,12 @@ add_thread_object (struct thread_info *tp)
   if (!gdb_python_initialized)
     return;
 
-  cleanup = ensure_python_env (python_gdbarch, python_language);
+  gdbpy_enter enter_py (python_gdbarch, python_language);
 
   thread_obj = create_thread_object (tp);
   if (!thread_obj)
     {
       gdbpy_print_stack ();
-      do_cleanups (cleanup);
       return;
     }
 
@@ -338,29 +306,23 @@ add_thread_object (struct thread_info *tp)
 
   inf_obj->threads = entry;
   inf_obj->nthreads++;
-
-  do_cleanups (cleanup);
 }
 
 static void
 delete_thread_object (struct thread_info *tp, int ignore)
 {
-  struct cleanup *cleanup;
   inferior_object *inf_obj;
   struct threadlist_entry **entry, *tmp;
 
   if (!gdb_python_initialized)
     return;
 
-  cleanup = ensure_python_env (python_gdbarch, python_language);
+  gdbpy_enter enter_py (python_gdbarch, python_language);
 
   inf_obj
     = (inferior_object *) find_inferior_object (ptid_get_pid (tp->ptid));
   if (!inf_obj)
-    {
-      do_cleanups (cleanup);
-      return;
-    }
+    return;
 
   /* Find thread entry in its inferior's thread_list.  */
   for (entry = &inf_obj->threads; *entry != NULL; entry =
@@ -371,7 +333,6 @@ delete_thread_object (struct thread_info *tp, int ignore)
   if (!*entry)
     {
       Py_DECREF (inf_obj);
-      do_cleanups (cleanup);
       return;
     }
 
@@ -384,8 +345,6 @@ delete_thread_object (struct thread_info *tp, int ignore)
   Py_DECREF (tmp->thread_obj);
   Py_DECREF (inf_obj);
   xfree (tmp);
-
-  do_cleanups (cleanup);
 }
 
 static PyObject *
@@ -828,15 +787,13 @@ infpy_dealloc (PyObject *obj)
 static void
 py_free_inferior (struct inferior *inf, void *datum)
 {
-
-  struct cleanup *cleanup;
   inferior_object *inf_obj = (inferior_object *) datum;
   struct threadlist_entry *th_entry, *th_tmp;
 
   if (!gdb_python_initialized)
     return;
 
-  cleanup = ensure_python_env (python_gdbarch, python_language);
+  gdbpy_enter enter_py (python_gdbarch, python_language);
 
   inf_obj->inferior = NULL;
 
@@ -853,7 +810,6 @@ py_free_inferior (struct inferior *inf, void *datum)
   inf_obj->nthreads = 0;
 
   Py_DECREF ((PyObject *) inf_obj);
-  do_cleanups (cleanup);
 }
 
 /* Implementation of gdb.selected_inferior() -> gdb.Inferior.
-- 
2.7.4

[RFA 14/20] Use gdbpy_enter in gdbpy_before_prompt_hook

[Tom Tromey]

Change gdbpy_before_prompt_hook to use gdbpy_enter and
gdbpy_reference.

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

	* python/python.c (gdbpy_before_prompt_hook): Use gdbpy_enter,
	gdbpy_reference.
---
 gdb/ChangeLog       |  5 +++++
 gdb/python/python.c | 60 ++++++++++++++++++++++++++---------------------------
 2 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a8237a8..14ecd00 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/python.c (gdbpy_before_prompt_hook): Use gdbpy_enter,
+	gdbpy_reference.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer): Use
 	gdbpy_enter, gdbpy_reference, unique_xmalloc_ptr.
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index de75b0e..393937d 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1005,60 +1005,64 @@ static enum ext_lang_rc
 gdbpy_before_prompt_hook (const struct extension_language_defn *extlang,
 			  const char *current_gdb_prompt)
 {
-  struct cleanup *cleanup;
   gdb::unique_xmalloc_ptr<char> prompt;
 
   if (!gdb_python_initialized)
     return EXT_LANG_RC_NOP;
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   if (gdb_python_module
       && PyObject_HasAttrString (gdb_python_module, "prompt_hook"))
     {
-      PyObject *hook;
-
-      hook = PyObject_GetAttrString (gdb_python_module, "prompt_hook");
+      gdbpy_reference hook (PyObject_GetAttrString (gdb_python_module,
+						    "prompt_hook"));
       if (hook == NULL)
-	goto fail;
-
-      make_cleanup_py_decref (hook);
-
-      if (PyCallable_Check (hook))
 	{
-	  PyObject *result;
-	  PyObject *current_prompt;
+	  gdbpy_print_stack ();
+	  return EXT_LANG_RC_ERROR;
+	}
 
-	  current_prompt = PyString_FromString (current_gdb_prompt);
+      if (PyCallable_Check (hook.get ()))
+	{
+	  gdbpy_reference current_prompt
+	    (PyString_FromString (current_gdb_prompt));
 	  if (current_prompt == NULL)
-	    goto fail;
-
-	  result = PyObject_CallFunctionObjArgs (hook, current_prompt, NULL);
-
-	  Py_DECREF (current_prompt);
+	    {
+	      gdbpy_print_stack ();
+	      return EXT_LANG_RC_ERROR;
+	    }
 
+	  gdbpy_reference result
+	    (PyObject_CallFunctionObjArgs (hook.get (), current_prompt.get (),
+					   NULL));
 	  if (result == NULL)
-	    goto fail;
-
-	  make_cleanup_py_decref (result);
+	    {
+	      gdbpy_print_stack ();
+	      return EXT_LANG_RC_ERROR;
+	    }
 
 	  /* Return type should be None, or a String.  If it is None,
 	     fall through, we will not set a prompt.  If it is a
 	     string, set  PROMPT.  Anything else, set an exception.  */
-	  if (result != Py_None && ! PyString_Check (result))
+	  if (result != Py_None && ! PyString_Check (result.get ()))
 	    {
 	      PyErr_Format (PyExc_RuntimeError,
 			    _("Return from prompt_hook must " \
 			      "be either a Python string, or None"));
-	      goto fail;
+	      gdbpy_print_stack ();
+	      return EXT_LANG_RC_ERROR;
 	    }
 
 	  if (result != Py_None)
 	    {
-	      prompt = python_string_to_host_string (result);
+	      prompt = python_string_to_host_string (result.get ());
 
 	      if (prompt == NULL)
-		goto fail;
+		{
+		  gdbpy_print_stack ();
+		  return EXT_LANG_RC_ERROR;
+		}
 	    }
 	}
     }
@@ -1068,13 +1072,7 @@ gdbpy_before_prompt_hook (const struct extension_language_defn *extlang,
   if (prompt != NULL)
     set_prompt (prompt.get ());
 
-  do_cleanups (cleanup);
   return prompt != NULL ? EXT_LANG_RC_OK : EXT_LANG_RC_NOP;
-
- fail:
-  gdbpy_print_stack ();
-  do_cleanups (cleanup);
-  return EXT_LANG_RC_ERROR;
 }
 
 \f
-- 
2.7.4

Re: [RFA 14/20] Use gdbpy_enter in gdbpy_before_prompt_hook

[Pedro Alves]

LGTM.  A suggestion below.

On 11/10/2016 10:19 PM, Tom Tromey wrote:
>  	      if (prompt == NULL)
> -		goto fail;
> +		{
> +		  gdbpy_print_stack ();
> +		  return EXT_LANG_RC_ERROR;
> +		}
>  	    }
>  	}
>      }
> @@ -1068,13 +1072,7 @@ gdbpy_before_prompt_hook (const struct extension_language_defn *extlang,
>    if (prompt != NULL)
>      set_prompt (prompt.get ());

I haven't applied the patch locally, but I suspect this set_prompt call
could move to the tail of the nested ifs, where you could
return EXT_LANG_RC_OK unconditionally, and then the bottom
could just unconditionally return EXT_LANG_RC_NOP.

Thanks,
Pedro Alves

Re: [RFA 14/20] Use gdbpy_enter in gdbpy_before_prompt_hook

[Tom Tromey]

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

>> if (prompt != NULL)
>> set_prompt (prompt.get ());

Pedro> I haven't applied the patch locally, but I suspect this set_prompt call
Pedro> could move to the tail of the nested ifs, where you could
Pedro> return EXT_LANG_RC_OK unconditionally, and then the bottom
Pedro> could just unconditionally return EXT_LANG_RC_NOP.

Yes, that's correct.  I'm making this change.

Tom

[RFA 08/20] Use gdbpy_enter in python.c

[Tom Tromey]

Change the simple parts of python.c to use gdbpy_enter.

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

	* python/python.c (gdbpy_eval_from_control_command)
	(gdbpy_source_script, gdbpy_run_events)
	(gdbpy_source_objfile_script, gdbpy_execute_objfile_script)
	(gdbpy_free_type_printers, gdbpy_finish_initialization): Use
	gdbpy_enter.
---
 gdb/ChangeLog       |  8 ++++++++
 gdb/python/python.c | 38 ++++++++------------------------------
 2 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3a25270..e93e36b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/python.c (gdbpy_eval_from_control_command)
+	(gdbpy_source_script, gdbpy_run_events)
+	(gdbpy_source_objfile_script, gdbpy_execute_objfile_script)
+	(gdbpy_free_type_printers, gdbpy_finish_initialization): Use
+	gdbpy_enter.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-progspace.c (py_free_pspace): Use gdbpy_enter.
 
 2016-11-10  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 78b4507..de75b0e 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -434,20 +434,17 @@ gdbpy_eval_from_control_command (const struct extension_language_defn *extlang,
 {
   int ret;
   char *script;
-  struct cleanup *cleanup;
 
   if (cmd->body_count != 1)
     error (_("Invalid \"python\" block structure."));
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   script = compute_python_string (cmd->body_list[0]);
   ret = PyRun_SimpleString (script);
   xfree (script);
   if (ret)
     error (_("Error while executing Python code."));
-
-  do_cleanups (cleanup);
 }
 
 /* Implementation of the gdb "python" command.  */
@@ -888,11 +885,8 @@ static void
 gdbpy_source_script (const struct extension_language_defn *extlang,
 		     FILE *file, const char *filename)
 {
-  struct cleanup *cleanup;
-
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
   python_run_simple_file (file, filename);
-  do_cleanups (cleanup);
 }
 
 \f
@@ -924,9 +918,7 @@ static struct serial_event *gdbpy_serial_event;
 static void
 gdbpy_run_events (int error, gdb_client_data client_data)
 {
-  struct cleanup *cleanup;
-
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   /* Clear the event fd.  Do this before flushing the events list, so
      that any new event post afterwards is sure to re-awake the event
@@ -953,8 +945,6 @@ gdbpy_run_events (int error, gdb_client_data client_data)
       Py_DECREF (item->event);
       xfree (item);
     }
-
-  do_cleanups (cleanup);
 }
 
 /* Submit an event to the gdb thread.  */
@@ -1297,17 +1287,14 @@ gdbpy_source_objfile_script (const struct extension_language_defn *extlang,
 			     struct objfile *objfile, FILE *file,
 			     const char *filename)
 {
-  struct cleanup *cleanups;
-
   if (!gdb_python_initialized)
     return;
 
-  cleanups = ensure_python_env (get_objfile_arch (objfile), current_language);
+  gdbpy_enter enter_py (get_objfile_arch (objfile), current_language);
   gdbpy_current_objfile = objfile;
 
   python_run_simple_file (file, filename);
 
-  do_cleanups (cleanups);
   gdbpy_current_objfile = NULL;
 }
 
@@ -1322,17 +1309,14 @@ gdbpy_execute_objfile_script (const struct extension_language_defn *extlang,
 			      struct objfile *objfile, const char *name,
 			      const char *script)
 {
-  struct cleanup *cleanups;
-
   if (!gdb_python_initialized)
     return;
 
-  cleanups = ensure_python_env (get_objfile_arch (objfile), current_language);
+  gdbpy_enter enter_py (get_objfile_arch (objfile), current_language);
   gdbpy_current_objfile = objfile;
 
   PyRun_SimpleString (script);
 
-  do_cleanups (cleanups);
   gdbpy_current_objfile = NULL;
 }
 
@@ -1383,13 +1367,12 @@ static void
 gdbpy_start_type_printers (const struct extension_language_defn *extlang,
 			   struct ext_lang_type_printers *ext_printers)
 {
-  struct cleanup *cleanups;
   PyObject *type_module, *func = NULL, *printers_obj = NULL;
 
   if (!gdb_python_initialized)
     return;
 
-  cleanups = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   type_module = PyImport_ImportModule ("gdb.types");
   if (type_module == NULL)
@@ -1414,7 +1397,6 @@ gdbpy_start_type_printers (const struct extension_language_defn *extlang,
  done:
   Py_XDECREF (type_module);
   Py_XDECREF (func);
-  do_cleanups (cleanups);
 }
 
 /* If TYPE is recognized by some type printer, store in *PRETTIED_TYPE
@@ -1429,7 +1411,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)
 {
-  struct cleanup *cleanups;
   PyObject *type_obj, *type_module = NULL, *func = NULL;
   PyObject *result_obj = NULL;
   PyObject *printers_obj = (PyObject *) ext_printers->py_type_printers;
@@ -1441,7 +1422,7 @@ gdbpy_apply_type_printers (const struct extension_language_defn *extlang,
   if (!gdb_python_initialized)
     return EXT_LANG_RC_NOP;
 
-  cleanups = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   type_obj = type_to_type_object (type);
   if (type_obj == NULL)
@@ -1484,7 +1465,6 @@ gdbpy_apply_type_printers (const struct extension_language_defn *extlang,
   Py_XDECREF (type_module);
   Py_XDECREF (func);
   Py_XDECREF (result_obj);
-  do_cleanups (cleanups);
   if (result != NULL)
     {
       *prettied_type = result.release ();
@@ -1500,7 +1480,6 @@ static void
 gdbpy_free_type_printers (const struct extension_language_defn *extlang,
 			  struct ext_lang_type_printers *ext_printers)
 {
-  struct cleanup *cleanups;
   PyObject *printers = (PyObject *) ext_printers->py_type_printers;
 
   if (printers == NULL)
@@ -1509,9 +1488,8 @@ gdbpy_free_type_printers (const struct extension_language_defn *extlang,
   if (!gdb_python_initialized)
     return;
 
-  cleanups = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
   Py_DECREF (printers);
-  do_cleanups (cleanups);
 }
 
 #else /* HAVE_PYTHON */
-- 
2.7.4

[RFA 01/20] Introduce gdbpy_enter

[Tom Tromey]

This introduces gdbpy_enter, a class that can be used to acquire and
release the Python GIL, and also set other Python-related globals used
by gdb.  ensure_python_env is rewritten in terms of this new class.

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

	* python/python.c (gdbpy_enter): New constructor.
	(~gdbpy_enter): New destructor.
	(restore_python_env, ensure_python_env): Rewrite.
	* python/python-internal.h (gdbpy_enter): New class.
---
 gdb/ChangeLog                |  7 +++++
 gdb/python/python-internal.h | 23 +++++++++++++++
 gdb/python/python.c          | 69 ++++++++++++++++++++------------------------
 3 files changed, 62 insertions(+), 37 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b670e69..460acf2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/python.c (gdbpy_enter): New constructor.
+	(~gdbpy_enter): New destructor.
+	(restore_python_env, ensure_python_env): Rewrite.
+	* python/python-internal.h (gdbpy_enter): New class.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-symbol.c (gdbpy_lookup_symbol): Use gdbpy_reference.
 
 2016-11-10  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index f3213bb..2aca6ff 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -501,6 +501,29 @@ int gdbpy_initialize_unwind (void)
 struct cleanup *make_cleanup_py_decref (PyObject *py);
 struct cleanup *make_cleanup_py_xdecref (PyObject *py);
 
+/* Restore global language and architecture and Python GIL state
+   when leaving the Python interpreter.  */
+class gdbpy_enter
+{
+ public:
+
+  gdbpy_enter (struct gdbarch *gdbarch, const struct language_defn *language);
+
+  ~gdbpy_enter ();
+
+ private:
+
+  // Intentionally not defined.
+  gdbpy_enter (const gdbpy_enter &);
+  gdbpy_enter &operator= (const gdbpy_enter &);
+
+  struct active_ext_lang_state *m_previous_active;
+  PyGILState_STATE m_state;
+  struct gdbarch *m_gdbarch;
+  const struct language_defn *m_language;
+  PyObject *m_error_type, *m_error_value, *m_error_traceback;
+};
+
 struct cleanup *ensure_python_env (struct gdbarch *gdbarch,
 				   const struct language_defn *language);
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 3d7b88c..78b4507 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -202,23 +202,28 @@ const struct extension_language_ops python_extension_ops =
 struct gdbarch *python_gdbarch;
 const struct language_defn *python_language;
 
-/* Restore global language and architecture and Python GIL state
-   when leaving the Python interpreter.  */
-
-struct python_env
+gdbpy_enter::gdbpy_enter  (struct gdbarch *gdbarch,
+			   const struct language_defn *language)
+: m_gdbarch (python_gdbarch),
+  m_language (python_language)
 {
-  struct active_ext_lang_state *previous_active;
-  PyGILState_STATE state;
-  struct gdbarch *gdbarch;
-  const struct language_defn *language;
-  PyObject *error_type, *error_value, *error_traceback;
-};
+  /* We should not ever enter Python unless initialized.  */
+  if (!gdb_python_initialized)
+    error (_("Python not initialized"));
 
-static void
-restore_python_env (void *p)
-{
-  struct python_env *env = (struct python_env *)p;
+  m_previous_active = set_active_ext_lang (&extension_language_python);
 
+  m_state = PyGILState_Ensure ();
+
+  python_gdbarch = gdbarch;
+  python_language = language;
+
+  /* Save it and ensure ! PyErr_Occurred () afterwards.  */
+  PyErr_Fetch (&m_error_type, &m_error_value, &m_error_traceback);
+}
+
+gdbpy_enter::~gdbpy_enter ()
+{
   /* Leftover Python error is forbidden by Python Exception Handling.  */
   if (PyErr_Occurred ())
     {
@@ -227,15 +232,21 @@ restore_python_env (void *p)
       warning (_("internal error: Unhandled Python exception"));
     }
 
-  PyErr_Restore (env->error_type, env->error_value, env->error_traceback);
+  PyErr_Restore (m_error_type, m_error_value, m_error_traceback);
 
-  PyGILState_Release (env->state);
-  python_gdbarch = env->gdbarch;
-  python_language = env->language;
+  PyGILState_Release (m_state);
+  python_gdbarch = m_gdbarch;
+  python_language = m_language;
 
-  restore_active_ext_lang (env->previous_active);
+  restore_active_ext_lang (m_previous_active);
+}
+
+static void
+restore_python_env (void *p)
+{
+  gdbpy_enter *env = (gdbpy_enter *) p;
 
-  xfree (env);
+  delete env;
 }
 
 /* Called before entering the Python interpreter to install the
@@ -248,23 +259,7 @@ struct cleanup *
 ensure_python_env (struct gdbarch *gdbarch,
                    const struct language_defn *language)
 {
-  struct python_env *env = XNEW (struct python_env);
-
-  /* We should not ever enter Python unless initialized.  */
-  if (!gdb_python_initialized)
-    error (_("Python not initialized"));
-
-  env->previous_active = set_active_ext_lang (&extension_language_python);
-
-  env->state = PyGILState_Ensure ();
-  env->gdbarch = python_gdbarch;
-  env->language = python_language;
-
-  python_gdbarch = gdbarch;
-  python_language = language;
-
-  /* Save it and ensure ! PyErr_Occurred () afterwards.  */
-  PyErr_Fetch (&env->error_type, &env->error_value, &env->error_traceback);
+  gdbpy_enter *env = new gdbpy_enter (gdbarch, language);
 
   return make_cleanup (restore_python_env, env);
 }
-- 
2.7.4

Re: [RFA 01/20] Introduce gdbpy_enter

[Pedro Alves]

LGTM with a nit below.

On 11/10/2016 10:19 PM, Tom Tromey wrote:
> +
> + private:
> +
> +  // Intentionally not defined.

Use /**/ comments.

But actually, since we require C++11 now, use =delete instead.

> +  gdbpy_enter (const gdbpy_enter &);
> +  gdbpy_enter &operator= (const gdbpy_enter &);

Thanks,
Pedro Alves

[RFA 07/20] Use gdbpy_enter in py-progspace.c

[Tom Tromey]

Change py-progspace.c to use gdbpy_enter.

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

	* python/py-progspace.c (py_free_pspace): Use gdbpy_enter.
---
 gdb/ChangeLog             | 4 ++++
 gdb/python/py-progspace.c | 4 +---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6acb87a..3a25270 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-progspace.c (py_free_pspace): Use gdbpy_enter.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-objfile.c (py_free_objfile): Use gdbpy_enter.
 
 2016-11-10  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 6fc53cb..26119c2 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -323,7 +323,6 @@ pspy_set_type_printers (PyObject *o, PyObject *value, void *ignore)
 static void
 py_free_pspace (struct program_space *pspace, void *datum)
 {
-  struct cleanup *cleanup;
   pspace_object *object = (pspace_object *) datum;
   /* This is a fiction, but we're in a nasty spot: The pspace is in the
      process of being deleted, we can't rely on anything in it.  Plus
@@ -336,10 +335,9 @@ py_free_pspace (struct program_space *pspace, void *datum)
      being deleted.  */
   struct gdbarch *arch = target_gdbarch ();
 
-  cleanup = ensure_python_env (arch, current_language);
+  gdbpy_enter enter_py (arch, current_language);
   object->pspace = NULL;
   Py_DECREF ((PyObject *) object);
-  do_cleanups (cleanup);
 }
 
 /* Return a borrowed reference to the Python object of type Pspace
-- 
2.7.4

[RFA 15/20] Use gdbpy_enter in python_interactive_command

[Tom Tromey]

This changes python_interactive_command to use gdbpy_enter.
Previously this function was leaving a dangling cleanup -- this is
sort of ok in a command function, but IMO it's still better to clean
up.

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

	* python/python.c (python_interactive_command): Use gdbpy_enter.
---
 gdb/ChangeLog       | 4 ++++
 gdb/python/python.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 14ecd00..1a415fb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/python.c (python_interactive_command): Use gdbpy_enter.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/python.c (gdbpy_before_prompt_hook): Use gdbpy_enter,
 	gdbpy_reference.
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 393937d..471f9e2 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -321,7 +321,7 @@ python_interactive_command (char *arg, int from_tty)
 
   arg = skip_spaces (arg);
 
-  ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   if (arg && *arg)
     {
-- 
2.7.4

[RFA 04/20] Use gdbpy_enter in py-finishbreakpoint.c

[Tom Tromey]

Change py-finishbreakpoint.c to use gdbpy_enter.

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

	* python/py-finishbreakpoint.c (bpfinishpy_handle_stop)
	(bpfinishpy_handle_exit): Use gdbpy_enter.
---
 gdb/ChangeLog                    |  5 +++++
 gdb/python/py-finishbreakpoint.c | 10 ++--------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2478e7e..2deede2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-finishbreakpoint.c (bpfinishpy_handle_stop)
+	(bpfinishpy_handle_exit): Use gdbpy_enter.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-cmd.c (cmdpy_destroyer)
 	(cmdpy_completer_handle_brkchars, cmdpy_completer): Use
 	gdbpy_enter.
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 74d225b..fdac696 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -392,13 +392,10 @@ bpfinishpy_detect_out_scope_cb (struct breakpoint *b, void *args)
 static void
 bpfinishpy_handle_stop (struct bpstats *bs, int print_frame)
 {
-  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
-                                               current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   iterate_over_breakpoints (bpfinishpy_detect_out_scope_cb,
                             bs == NULL ? NULL : bs->breakpoint_at);
-
-  do_cleanups (cleanup);
 }
 
 /* Attached to `exit' notifications, triggers all the necessary out of
@@ -407,12 +404,9 @@ bpfinishpy_handle_stop (struct bpstats *bs, int print_frame)
 static void
 bpfinishpy_handle_exit (struct inferior *inf)
 {
-  struct cleanup *cleanup = ensure_python_env (target_gdbarch (),
-                                               current_language);
+  gdbpy_enter enter_py (target_gdbarch (), current_language);
 
   iterate_over_breakpoints (bpfinishpy_detect_out_scope_cb, NULL);
-
-  do_cleanups (cleanup);
 }
 
 /* Initialize the Python finish breakpoint code.  */
-- 
2.7.4

[RFA 18/20] Use gdbpy_enter in py-xmethod.c

[Tom Tromey]

This changes the remaining functions in py-xmethod.c to use
gdbpy_enter; using gdbpy_reference and unique_xmalloc_ptr as
appropriate.

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

	* python/py-xmethods.c (gdbpy_get_xmethod_result_type): Use
	gdbpy_enter, gdbpy_reference, unique_xmalloc_ptr.
	(gdbpy_invoke_xmethod): Use gdbpy_reference, gdbpy_enter.
	(gdbpy_get_xmethod_arg_types): Use gdbpy_reference,
	unique_xmalloc_ptr.
	(gdbpy_get_xmethod_arg_types): Use gdbpy_reference, gdbpy_enter.
---
 gdb/ChangeLog            |   9 +++
 gdb/python/py-xmethods.c | 165 +++++++++++++++++++++--------------------------
 2 files changed, 83 insertions(+), 91 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cb0492d..41e4081 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-xmethods.c (gdbpy_get_xmethod_result_type): Use
+	gdbpy_enter, gdbpy_reference, unique_xmalloc_ptr.
+	(gdbpy_invoke_xmethod): Use gdbpy_reference, gdbpy_enter.
+	(gdbpy_get_xmethod_arg_types): Use gdbpy_reference,
+	unique_xmalloc_ptr.
+	(gdbpy_get_xmethod_arg_types): Use gdbpy_reference, gdbpy_enter.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-xmethods.c (invoke_match_method): Use
 	gdbpy_reference.
 
diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
index a5319cc..e2458b7 100644
--- a/gdb/python/py-xmethods.c
+++ b/gdb/python/py-xmethods.c
@@ -312,86 +312,84 @@ gdbpy_get_xmethod_arg_types (const struct extension_language_defn *extlang,
   struct gdbpy_worker_data *worker_data
     = (struct gdbpy_worker_data *) worker->data;
   PyObject *py_worker = worker_data->worker;
-  PyObject *get_arg_types_method;
-  PyObject *py_argtype_list, *list_iter = NULL, *item;
-  struct cleanup *cleanups;
-  struct type **type_array, *obj_type;
+  struct type *obj_type;
   int i = 1, arg_count;
+  gdbpy_reference list_iter;
 
   /* Set nargs to -1 so that any premature return from this function returns
      an invalid/unusable number of arg types.  */
   *nargs = -1;
 
-  cleanups = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  get_arg_types_method =  PyObject_GetAttrString (py_worker,
-						  get_arg_types_method_name);
+  gdbpy_reference get_arg_types_method
+    (PyObject_GetAttrString (py_worker, get_arg_types_method_name));
   if (get_arg_types_method == NULL)
     {
       gdbpy_print_stack ();
-      do_cleanups (cleanups);
-
       return EXT_LANG_RC_ERROR;
     }
-  make_cleanup_py_decref (get_arg_types_method);
 
-  py_argtype_list = PyObject_CallMethodObjArgs (py_worker,
-						py_get_arg_types_method_name,
-						NULL);
+  gdbpy_reference py_argtype_list
+    (PyObject_CallMethodObjArgs (py_worker, py_get_arg_types_method_name,
+				 NULL));
   if (py_argtype_list == NULL)
     {
       gdbpy_print_stack ();
-      do_cleanups (cleanups);
-
       return EXT_LANG_RC_ERROR;
     }
-  make_cleanup_py_decref (py_argtype_list);
+
   if (py_argtype_list == Py_None)
     arg_count = 0;
-  else if (PySequence_Check (py_argtype_list))
+  else if (PySequence_Check (py_argtype_list.get ()))
     {
-      arg_count = PySequence_Size (py_argtype_list);
+      arg_count = PySequence_Size (py_argtype_list.get ());
       if (arg_count == -1)
 	{
 	  gdbpy_print_stack ();
-	  do_cleanups (cleanups);
-
 	  return EXT_LANG_RC_ERROR;
 	}
 
-      list_iter = PyObject_GetIter (py_argtype_list);
+      list_iter.reset (PyObject_GetIter (py_argtype_list.get ()));
       if (list_iter == NULL)
 	{
 	  gdbpy_print_stack ();
-	  do_cleanups (cleanups);
-
 	  return EXT_LANG_RC_ERROR;
 	}
-      make_cleanup_py_decref (list_iter);
     }
   else
     arg_count = 1;
 
   /* Include the 'this' argument in the size.  */
-  type_array = XCNEWVEC (struct type *, arg_count + 1);
+  gdb::unique_xmalloc_ptr<struct type *> type_array
+    (XCNEWVEC (struct type *, arg_count + 1));
   i = 1;
   if (list_iter != NULL)
     {
-      while ((item = PyIter_Next (list_iter)) != NULL)
+      while (true)
 	{
-	  struct type *arg_type = type_object_to_type (item);
+	  gdbpy_reference item (PyIter_Next (list_iter.get ()));
+	  if (item == NULL)
+	    {
+	      if (PyErr_Occurred ())
+		{
+		  gdbpy_print_stack ();
+		  return EXT_LANG_RC_ERROR;
+		}
+	      break;
+	    }
 
-	  Py_DECREF (item);
+	  struct type *arg_type = type_object_to_type (item.get ());
 	  if (arg_type == NULL)
 	    {
 	      PyErr_SetString (PyExc_TypeError,
 			       _("Arg type returned by the get_arg_types "
 				 "method of a debug method worker object is "
 				 "not a gdb.Type object."));
-	      break;
+	      return EXT_LANG_RC_ERROR;
 	    }
 
-	  type_array[i] = arg_type;
+	  (type_array.get ())[i] = arg_type;
 	  i++;
 	}
     }
@@ -399,7 +397,7 @@ gdbpy_get_xmethod_arg_types (const struct extension_language_defn *extlang,
     {
       /* py_argtype_list is not actually a list but a single gdb.Type
 	 object.  */
-      struct type *arg_type = type_object_to_type (py_argtype_list);
+      struct type *arg_type = type_object_to_type (py_argtype_list.get ());
 
       if (arg_type == NULL)
 	{
@@ -407,30 +405,23 @@ gdbpy_get_xmethod_arg_types (const struct extension_language_defn *extlang,
 			   _("Arg type returned by the get_arg_types method "
 			     "of an xmethod worker object is not a gdb.Type "
 			     "object."));
+	  return EXT_LANG_RC_ERROR;
 	}
       else
 	{
-	  type_array[i] = arg_type;
+	  (type_array.get ())[i] = arg_type;
 	  i++;
 	}
     }
-  if (PyErr_Occurred ())
-    {
-      gdbpy_print_stack ();
-      do_cleanups (cleanups);
-      xfree (type_array);
-
-      return EXT_LANG_RC_ERROR;
-    }
 
   /* Add the type of 'this' as the first argument.  The 'this' pointer should
      be a 'const' value.  Hence, create a 'const' variant of the 'this' pointer
      type.  */
   obj_type = type_object_to_type (worker_data->this_type);
-  type_array[0] = make_cv_type (1, 0, lookup_pointer_type (obj_type), NULL);
+  (type_array.get ())[0] = make_cv_type (1, 0, lookup_pointer_type (obj_type),
+					 NULL);
   *nargs = i;
-  *arg_types = type_array;
-  do_cleanups (cleanups);
+  *arg_types = type_array.release ();
 
   return EXT_LANG_RC_OK;
 }
@@ -447,26 +438,21 @@ gdbpy_get_xmethod_result_type (const struct extension_language_defn *extlang,
   struct gdbpy_worker_data *worker_data
     = (struct gdbpy_worker_data *) worker->data;
   PyObject *py_worker = worker_data->worker;
-  PyObject *py_value_obj, *py_arg_tuple, *py_result_type;
-  PyObject *get_result_type_method;
   struct type *obj_type, *this_type;
-  struct cleanup *cleanups;
   int i;
 
-  cleanups = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   /* First see if there is a get_result_type method.
      If not this could be an old xmethod (pre 7.9.1).  */
-  get_result_type_method
-    = PyObject_GetAttrString (py_worker, get_result_type_method_name);
+  gdbpy_reference get_result_type_method
+    (PyObject_GetAttrString (py_worker, get_result_type_method_name));
   if (get_result_type_method == NULL)
     {
       PyErr_Clear ();
-      do_cleanups (cleanups);
       *result_type_ptr = NULL;
       return EXT_LANG_RC_OK;
     }
-  make_cleanup_py_decref (get_result_type_method);
 
   obj_type = check_typedef (value_type (obj));
   this_type = check_typedef (type_object_to_type (worker_data->this_type));
@@ -489,51 +475,55 @@ gdbpy_get_xmethod_result_type (const struct extension_language_defn *extlang,
       if (!types_equal (obj_type, this_type))
 	obj = value_cast (this_type, obj);
     }
-  py_value_obj = value_to_value_object (obj);
+  gdbpy_reference py_value_obj (value_to_value_object (obj));
   if (py_value_obj == NULL)
-    goto Fail;
-  make_cleanup_py_decref (py_value_obj);
+    {
+      gdbpy_print_stack ();
+      return EXT_LANG_RC_ERROR;
+    }
 
-  py_arg_tuple = PyTuple_New (nargs + 1);
+  gdbpy_reference py_arg_tuple (PyTuple_New (nargs + 1));
   if (py_arg_tuple == NULL)
-    goto Fail;
-  make_cleanup_py_decref (py_arg_tuple);
+    {
+      gdbpy_print_stack ();
+      return EXT_LANG_RC_ERROR;
+    }
 
-  /* PyTuple_SET_ITEM steals the reference of the element.  Hence INCREF the
-     reference to the 'this' object as we have a cleanup to DECREF it.  */
-  Py_INCREF (py_value_obj);
-  PyTuple_SET_ITEM (py_arg_tuple, 0, py_value_obj);
+  /* PyTuple_SET_ITEM steals the reference of the element, hence the
+     release.  */
+  PyTuple_SET_ITEM (py_arg_tuple.get (), 0, py_value_obj.release ());
 
   for (i = 0; i < nargs; i++)
     {
       PyObject *py_value_arg = value_to_value_object (args[i]);
 
       if (py_value_arg == NULL)
-	goto Fail;
-      PyTuple_SET_ITEM (py_arg_tuple, i + 1, py_value_arg);
+	{
+	  gdbpy_print_stack ();
+	  return EXT_LANG_RC_ERROR;
+	}
+      PyTuple_SET_ITEM (py_arg_tuple.get (), i + 1, py_value_arg);
     }
 
-  py_result_type = PyObject_CallObject (get_result_type_method, py_arg_tuple);
+  gdbpy_reference py_result_type
+    (PyObject_CallObject (get_result_type_method.get (), py_arg_tuple.get ()));
   if (py_result_type == NULL)
-    goto Fail;
-  make_cleanup_py_decref (py_result_type);
+    {
+      gdbpy_print_stack ();
+      return EXT_LANG_RC_ERROR;
+    }
 
-  *result_type_ptr = type_object_to_type (py_result_type);
+  *result_type_ptr = type_object_to_type (py_result_type.get ());
   if (*result_type_ptr == NULL)
     {
       PyErr_SetString (PyExc_TypeError,
 		       _("Type returned by the get_result_type method of an"
 			 " xmethod worker object is not a gdb.Type object."));
-      goto Fail;
+      gdbpy_print_stack ();
+      return EXT_LANG_RC_ERROR;
     }
 
-  do_cleanups (cleanups);
   return EXT_LANG_RC_OK;
-
- Fail:
-  gdbpy_print_stack ();
-  do_cleanups (cleanups);
-  return EXT_LANG_RC_ERROR;
 }
 
 /* Implementation of invoke_xmethod for Python.  */
@@ -544,15 +534,13 @@ gdbpy_invoke_xmethod (const struct extension_language_defn *extlang,
 		      struct value *obj, struct value **args, int nargs)
 {
   int i;
-  struct cleanup *cleanups;
-  PyObject *py_value_obj, *py_arg_tuple, *py_result;
   struct type *obj_type, *this_type;
   struct value *res = NULL;
   struct gdbpy_worker_data *worker_data
     = (struct gdbpy_worker_data *) worker->data;
   PyObject *xmethod_worker = worker_data->worker;
 
-  cleanups = ensure_python_env (get_current_arch (), current_language);
+  gdbpy_enter enter_py (get_current_arch (), current_language);
 
   obj_type = check_typedef (value_type (obj));
   this_type = check_typedef (type_object_to_type (worker_data->this_type));
@@ -575,26 +563,23 @@ gdbpy_invoke_xmethod (const struct extension_language_defn *extlang,
       if (!types_equal (obj_type, this_type))
 	obj = value_cast (this_type, obj);
     }
-  py_value_obj = value_to_value_object (obj);
+  gdbpy_reference py_value_obj (value_to_value_object (obj));
   if (py_value_obj == NULL)
     {
       gdbpy_print_stack ();
       error (_("Error while executing Python code."));
     }
-  make_cleanup_py_decref (py_value_obj);
 
-  py_arg_tuple = PyTuple_New (nargs + 1);
+  gdbpy_reference py_arg_tuple (PyTuple_New (nargs + 1));
   if (py_arg_tuple == NULL)
     {
       gdbpy_print_stack ();
       error (_("Error while executing Python code."));
     }
-  make_cleanup_py_decref (py_arg_tuple);
 
-  /* PyTuple_SET_ITEM steals the reference of the element.  Hence INCREF the
-     reference to the 'this' object as we have a cleanup to DECREF it.  */
-  Py_INCREF (py_value_obj);
-  PyTuple_SET_ITEM (py_arg_tuple, 0, py_value_obj);
+  /* PyTuple_SET_ITEM steals the reference of the element, hence the
+     release.  */
+  PyTuple_SET_ITEM (py_arg_tuple.get (), 0, py_value_obj.release ());
 
   for (i = 0; i < nargs; i++)
     {
@@ -606,20 +591,20 @@ gdbpy_invoke_xmethod (const struct extension_language_defn *extlang,
 	  error (_("Error while executing Python code."));
 	}
 
-      PyTuple_SET_ITEM (py_arg_tuple, i + 1, py_value_arg);
+      PyTuple_SET_ITEM (py_arg_tuple.get (), i + 1, py_value_arg);
     }
 
-  py_result = PyObject_CallObject (xmethod_worker, py_arg_tuple);
+  gdbpy_reference py_result (PyObject_CallObject (xmethod_worker,
+						  py_arg_tuple.get ()));
   if (py_result == NULL)
     {
       gdbpy_print_stack ();
       error (_("Error while executing Python code."));
     }
-  make_cleanup_py_decref (py_result);
 
   if (py_result != Py_None)
     {
-      res = convert_value_from_python (py_result);
+      res = convert_value_from_python (py_result.get ());
       if (res == NULL)
 	{
 	  gdbpy_print_stack ();
@@ -632,8 +617,6 @@ gdbpy_invoke_xmethod (const struct extension_language_defn *extlang,
 					     "void", NULL, 0));
     }
 
-  do_cleanups (cleanups);
-
   return res;
 }
 
-- 
2.7.4

[RFA 20/20] Use gdbpy_enter_varobj in py-varobj.c

[Tom Tromey]

This converts the remaining functions in py-varobj.c to use
gdbpy_enter_varobj.

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

	* python/py-varobj.c (py_varobj_iter_dtor, py_varobj_iter_next):
	Use gdbpy_enter_varobj.
---
 gdb/ChangeLog          |  5 +++++
 gdb/python/py-varobj.c | 16 +++++-----------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 839f24d..aafb07a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* python/py-varobj.c (py_varobj_iter_dtor, py_varobj_iter_next):
+	Use gdbpy_enter_varobj.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* varobj.c (gdbpy_enter_varobj): New constructor.
 	* python/python-internal.h (gdbpy_enter_varobj): New class.
 	* python/py-varobj.c (py_varobj_get_iterator): Use
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index 5d02549..f28bf43 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -39,11 +39,8 @@ static void
 py_varobj_iter_dtor (struct varobj_iter *self)
 {
   struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
-  struct cleanup *back_to = varobj_ensure_python_env (self->var);
-
+  gdbpy_enter_varobj enter_py (self->var);
   Py_XDECREF (dis->iter);
-
-  do_cleanups (back_to);
 }
 
 /* Implementation of the 'next' method of pretty-printed varobj
@@ -53,8 +50,6 @@ static varobj_item *
 py_varobj_iter_next (struct varobj_iter *self)
 {
   struct py_varobj_iter *t = (struct py_varobj_iter *) self;
-  struct cleanup *back_to;
-  PyObject *item;
   PyObject *py_v;
   varobj_item *vitem;
   const char *name = NULL;
@@ -62,9 +57,9 @@ py_varobj_iter_next (struct varobj_iter *self)
   if (!gdb_python_initialized)
     return NULL;
 
-  back_to = varobj_ensure_python_env (self->var);
+  gdbpy_enter_varobj enter_py (self->var);
 
-  item = PyIter_Next (t->iter);
+  gdbpy_reference item (PyIter_Next (t->iter));
 
   if (item == NULL)
     {
@@ -92,7 +87,7 @@ py_varobj_iter_next (struct varobj_iter *self)
 
 	  name_str = xstrprintf ("<error at %d>",
 				 self->next_raw_index++);
-	  item = Py_BuildValue ("(ss)", name_str, value_str.get ());
+	  item.reset (Py_BuildValue ("(ss)", name_str, value_str.get ()));
 	  xfree (name_str);
 	  if (item == NULL)
 	    {
@@ -108,7 +103,7 @@ py_varobj_iter_next (struct varobj_iter *self)
 	}
     }
 
-  if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
+  if (!PyArg_ParseTuple (item.get (), "sO", &name, &py_v))
     {
       gdbpy_print_stack ();
       error (_("Invalid item from the child list"));
@@ -121,7 +116,6 @@ py_varobj_iter_next (struct varobj_iter *self)
   vitem->name = name;
 
   self->next_raw_index++;
-  do_cleanups (back_to);
   return vitem;
 }
 
-- 
2.7.4

[RFA 19/20] Introduce gdbpy_enter_varobj and use it

[Tom Tromey]

This introduces gdbpy_enter_varobj, a subclass of gdbpy_enter; then
changes one function in py-varobj.c to use it.  gdbpy_enter_varobj
takes a varobj as an argument, similar to varobj_ensure_python_env.

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

	* varobj.c (gdbpy_enter_varobj): New constructor.
	* python/python-internal.h (gdbpy_enter_varobj): New class.
	* python/py-varobj.c (py_varobj_get_iterator): Use
	gdbpy_enter_varobj.
---
 gdb/ChangeLog                |  7 +++++++
 gdb/python/py-varobj.c       | 21 ++++++++-------------
 gdb/python/python-internal.h | 12 ++++++++++++
 gdb/varobj.c                 |  7 +++++++
 4 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 41e4081..839f24d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* varobj.c (gdbpy_enter_varobj): New constructor.
+	* python/python-internal.h (gdbpy_enter_varobj): New class.
+	* python/py-varobj.c (py_varobj_get_iterator): Use
+	gdbpy_enter_varobj.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-xmethods.c (gdbpy_get_xmethod_result_type): Use
 	gdbpy_enter, gdbpy_reference, unique_xmalloc_ptr.
 	(gdbpy_invoke_xmethod): Use gdbpy_reference, gdbpy_enter.
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index 72c876c..5d02549 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -17,6 +17,7 @@
 #include "python-internal.h"
 #include "varobj.h"
 #include "varobj-iter.h"
+#include "py-ref.h"
 
 /* A dynamic varobj iterator "class" for python pretty-printed
    varobjs.  This inherits struct varobj_iter.  */
@@ -167,28 +168,24 @@ py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
 struct varobj_iter *
 py_varobj_get_iterator (struct varobj *var, PyObject *printer)
 {
-  PyObject *children;
   PyObject *iter;
   struct py_varobj_iter *py_iter;
-  struct cleanup *back_to = varobj_ensure_python_env (var);
+
+  gdbpy_enter_varobj enter_py (var);
 
   if (!PyObject_HasAttr (printer, gdbpy_children_cst))
-    {
-      do_cleanups (back_to);
-      return NULL;
-    }
+    return NULL;
 
-  children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
-					 NULL);
+  gdbpy_reference children (PyObject_CallMethodObjArgs (printer,
+							gdbpy_children_cst,
+							NULL));
   if (children == NULL)
     {
       gdbpy_print_stack ();
       error (_("Null value returned for children"));
     }
 
-  make_cleanup_py_decref (children);
-
-  iter = PyObject_GetIter (children);
+  iter = PyObject_GetIter (children.get ());
   if (iter == NULL)
     {
       gdbpy_print_stack ();
@@ -197,7 +194,5 @@ py_varobj_get_iterator (struct varobj *var, PyObject *printer)
 
   py_iter = py_varobj_iter_new (var, iter);
 
-  do_cleanups (back_to);
-
   return &py_iter->base;
 }
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 2aca6ff..66856df 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -524,6 +524,18 @@ class gdbpy_enter
   PyObject *m_error_type, *m_error_value, *m_error_traceback;
 };
 
+/* Like gdbpy_enter, but takes a varobj.  This is a subclass just to
+   make constructor delegation a little nicer.  */
+class gdbpy_enter_varobj : public gdbpy_enter
+{
+ public:
+
+  /* This is defined in varobj.c, where it can access varobj
+     internals.  */
+  gdbpy_enter_varobj (const struct varobj *var);
+
+};
+
 struct cleanup *ensure_python_env (struct gdbarch *gdbarch,
 				   const struct language_defn *language);
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 03edf0e..8abda59 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -233,6 +233,13 @@ varobj_ensure_python_env (const struct varobj *var)
   return ensure_python_env (var->root->exp->gdbarch,
 			    var->root->exp->language_defn);
 }
+
+/* See python-internal.h.  */
+gdbpy_enter_varobj::gdbpy_enter_varobj (const struct varobj *var)
+: gdbpy_enter (var->root->exp->gdbarch, var->root->exp->language_defn)
+{
+}
+
 #endif
 
 /* Return the full FRAME which corresponds to the given CORE_ADDR
-- 
2.7.4

[RFA 12/20] Introduce htab_up and use gdbpy_enter in py-framefilter.c

[Tom Tromey]

This introduces a new "htab_up" typedef, which is a std::unique_ptr
that can call htab_delete.  Then it changes some code in
py-framefilter.c to use both gdbpy_enter and the new htab_up.

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

	* utils.h (htab_deleter): New struct.
	(htab_up): New typedef.
	* python/py-framefilter.c (gdbpy_apply_frame_filter): Use
	gdbpy_enter, gdbpy_reference, htab_up.
---
 gdb/ChangeLog               |  7 ++++++
 gdb/python/py-framefilter.c | 55 +++++++++++++++++----------------------------
 gdb/utils.h                 | 12 ++++++++++
 3 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2101045..80d4329 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2016-11-10  Tom Tromey  <tom@tromey.com>
 
+	* utils.h (htab_deleter): New struct.
+	(htab_up): New typedef.
+	* python/py-framefilter.c (gdbpy_apply_frame_filter): Use
+	gdbpy_enter, gdbpy_reference, htab_up.
+
+2016-11-10  Tom Tromey  <tom@tromey.com>
+
 	* python/py-unwind.c (pending_frame_invalidate): Remove.
 	(pyuw_sniffer): Use gdbpy_enter and gdbpy_reference.
 
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 021b3d4..b14440a 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1491,11 +1491,7 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
 			  struct ui_out *out, int frame_low, int frame_high)
 {
   struct gdbarch *gdbarch = NULL;
-  struct cleanup *cleanups;
   enum ext_lang_bt_status success = EXT_LANG_BT_ERROR;
-  PyObject *iterable;
-  PyObject *item;
-  htab_t levels_printed;
 
   if (!gdb_python_initialized)
     return EXT_LANG_BT_NO_FILTERS;
@@ -1511,9 +1507,10 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
     }
   END_CATCH
 
-  cleanups = ensure_python_env (gdbarch, current_language);
+  gdbpy_enter enter_py (gdbarch, current_language);
 
-  iterable = bootstrap_python_frame_filters (frame, frame_low, frame_high);
+  gdbpy_reference iterable (bootstrap_python_frame_filters (frame, frame_low,
+							    frame_high));
 
   if (iterable == NULL)
     {
@@ -1531,34 +1528,36 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
 	 default backtrace.  */
 
       gdbpy_print_stack ();
-      do_cleanups (cleanups);
       return EXT_LANG_BT_NO_FILTERS;
     }
 
   /* If iterable is None, then there are no frame filters registered.
      If this is the case, defer to default GDB printing routines in MI
      and CLI.  */
-  make_cleanup_py_decref (iterable);
   if (iterable == Py_None)
-    {
-      success = EXT_LANG_BT_NO_FILTERS;
-      goto done;
-    }
+    return EXT_LANG_BT_NO_FILTERS;
 
-  levels_printed = htab_create (20,
-				htab_hash_pointer,
-				htab_eq_pointer,
-				NULL);
-  make_cleanup_htab_delete (levels_printed);
+  htab_up levels_printed (htab_create (20,
+				       htab_hash_pointer,
+				       htab_eq_pointer,
+				       NULL));
 
-  while ((item = PyIter_Next (iterable)))
+  while (true)
     {
-      struct cleanup *item_cleanup = make_cleanup_py_decref (item);
+      gdbpy_reference item (PyIter_Next (iterable.get ()));
 
-      success = py_print_frame (item, flags, args_type, out, 0,
-				levels_printed);
+      if (item == NULL)
+	{
+	  if (PyErr_Occurred ())
+	    {
+	      gdbpy_print_stack ();
+	      return EXT_LANG_BT_ERROR;
+	    }
+	  break;
+	}
 
-      do_cleanups (item_cleanup);
+      success = py_print_frame (item.get (), flags, args_type, out, 0,
+				levels_printed.get ());
 
       /* Do not exit on error printing a single frame.  Print the
 	 error and continue with other frames.  */
@@ -1566,17 +1565,5 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
 	gdbpy_print_stack ();
     }
 
-  if (item == NULL && PyErr_Occurred ())
-    goto error;
-
- done:
-  do_cleanups (cleanups);
   return success;
-
-  /* Exit and abandon backtrace on error, printing the exception that
-     is set.  */
- error:
-  gdbpy_print_stack ();
-  do_cleanups (cleanups);
-  return EXT_LANG_BT_ERROR;
 }
diff --git a/gdb/utils.h b/gdb/utils.h
index 36f5294..a1692c6 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -99,6 +99,18 @@ extern struct cleanup *make_cleanup_free_so (struct so_list *so);
 
 extern struct cleanup *make_cleanup_restore_current_language (void);
 
+/* A deleter for a hash table.  */
+struct htab_deleter
+{
+  void operator() (htab *ptr) const
+  {
+    htab_delete (ptr);
+  }
+};
+
+/* A unique_ptr wrapper for htab_t.  */
+typedef std::unique_ptr<htab, htab_deleter> htab_up;
+
 extern struct cleanup *make_cleanup_htab_delete (htab_t htab);
 
 struct parser_state;
-- 
2.7.4

Re: [RFA 00/20] more use of C++ in the Python layer

[Pedro Alves]

On 11/10/2016 10:19 PM, Tom Tromey wrote:
> This patch series removes many more cleanups from the Python layer.
> It mostly replaces ensure_python_env, but also fixes a few spots that
> were missing in the earlier series.  You may also want to take notice
> of the htab_up patch, which adds a unique_ptr typedef for managing
> libiberty hash tables.
> 
> These patches require my earlier gdbpy_reference series.

Looking at it now. 

> I've built and tested this locally.  I will run it through the
> buildbot as well -- long story, but I have to wait until one upstream
> patch lands to reset the baseline before I can do this.

I've no idea what that means, but, is that a posted patch waiting
for review?

I sent minor comments to a few patches, but otherwise this series
looks great to me.  Thanks a lot for doing this.

Thanks,
Pedro Alves

Re: [RFA 00/20] more use of C++ in the Python layer

[Tom Tromey]

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

>> I've built and tested this locally.  I will run it through the
>> buildbot as well -- long story, but I have to wait until one upstream
>> patch lands to reset the baseline before I can do this.

Pedro> I've no idea what that means, but, is that a posted patch waiting
Pedro> for review?

Sorry for the confusion; the story is that I tried to push this patch to
the buildbot earlier, but the buildbot had a bug.  Sergio fixed the
problems, but this resulted in there being no baseline test results
against which to compare a try run.  So, I'm waiting for something to be
committed and run through the buildbot so that a baseline will exist.

Pedro> I sent minor comments to a few patches, but otherwise this series
Pedro> looks great to me.  Thanks a lot for doing this.

My pleasure.  I find it greatly helps the code.  I think it'll make
future Python extensions much simpler to write.

Tom

Re: [RFA 00/20] more use of C++ in the Python layer

[Tom Tromey]

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

Pedro> I sent minor comments to a few patches, but otherwise this series
Pedro> looks great to me.  Thanks a lot for doing this.

I'm not sure if I can take this as an approval or not.

Tom

Re: [RFA 00/20] more use of C++ in the Python layer

[Pedro Alves]

On 11/12/2016 05:24 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I sent minor comments to a few patches, but otherwise this series
> Pedro> looks great to me.  Thanks a lot for doing this.
> 
> I'm not sure if I can take this as an approval or not.

Yes, what I meant was that all patches that I didn't explicitly
reply to are approved.

Thanks,
Pedro Alves