[RFA,14/20] Use gdbpy_enter in gdbpy_before_prompt_hook

Message ID 1478816387-27064-15-git-send-email-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Nov. 10, 2016, 10:19 p.m. UTC
  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(-)
  

Comments

Pedro Alves Nov. 10, 2016, 11:19 p.m. UTC | #1
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
  
Tom Tromey Nov. 12, 2016, 5:04 p.m. UTC | #2
>>>>> "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
  

Patch

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;
 }