[PATCH v2 2/4] gdb/python: eval_python_command returns result of the evaluation

[Matthieu Longo <matthieu.longo@arm.com>]

A previous commit [1] centralized the Python code evaluation in the
helper function eval_python_command(). However, a missed case in
varobj_set_visualizer() requires not only the exit status but the
result of the evaluation too.

This patch updates eval_python_command() to return the result of
the evaluation, or NULL if an error occurred. It also adjusts the
existing callers accordingly. Finally, it replaces the use of
PyRun_String() in varobj_set_visualizer() with a call to
eval_python_command().

[1]: 264a8a2236e8aa64b333a69e42a55ff8c0844f6e

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830
---
 gdb/python/py-gdb-readline.c |  2 +-
 gdb/python/python-internal.h |  4 +-
 gdb/python/python.c          | 75 +++++++++++++++++-------------------
 gdb/varobj.c                 | 11 +-----
 gdbsupport/gdb_ref_ptr.h     |  5 +++
 5 files changed, 45 insertions(+), 52 deletions(-)

diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
index e8e2c23547c..b7f8caeccd4 100644
--- a/gdb/python/py-gdb-readline.c
+++ b/gdb/python/py-gdb-readline.c
@@ -114,7 +114,7 @@ class GdbRemoveReadlineFinder(MetaPathFinder):\n\
 \n\
 sys.meta_path.insert(2, GdbRemoveReadlineFinder())\n\
 ";
-  if (eval_python_command (code, Py_file_input) == 0)
+  if (eval_python_command (code, Py_file_input))
     PyOS_ReadlineFunctionPointer = gdbpy_readline_wrapper;
 
   return 0;
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 37bc37691fe..577260c4066 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -1337,7 +1337,7 @@ class gdbpy_memoizing_registry_storage
   gdb::unordered_map<val_type *, obj_type *> m_objects;
 };
 
-extern int eval_python_command (const char *command, int start_symbol,
-				const char *filename = nullptr);
+extern gdbpy_ref<> eval_python_command
+  (const char *command, int start_symbol, const char *filename = nullptr);
 
 #endif /* GDB_PYTHON_PYTHON_INTERNAL_H */
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 502fe682b2c..546d1272007 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -297,44 +297,43 @@ gdbpy_check_quit_flag (const struct extension_language_defn *extlang)
    Python start symbol, and does not automatically print the stack on
    errors.  FILENAME is used to set the file name in error messages;
    NULL means that this is evaluating a string, not the contents of a
-   file.  */
+   file.
+   Return the result of the evaluation on success, NULL on failure.  */
 
-int
+gdbpy_ref<>
 eval_python_command (const char *command, int start_symbol,
 		     const char *filename)
 {
-  PyObject *m, *d;
-
-  m = PyImport_AddModule ("__main__");
-  if (m == NULL)
-    return -1;
+  gdbpy_borrowed_ref<> mainmod = PyImport_AddModule ("__main__");
+  if (mainmod == nullptr)
+    return nullptr;
 
-  d = PyModule_GetDict (m);
-  if (d == NULL)
-    return -1;
+  gdbpy_borrowed_ref<> globals = PyModule_GetDict (mainmod);
+  if (globals == nullptr)
+    return nullptr;
 
   bool file_set = false;
   if (filename != nullptr)
     {
       gdbpy_ref<> file = host_string_to_python_string ("__file__");
       if (file == nullptr)
-	return -1;
+	return nullptr;
 
       /* PyDict_GetItemWithError returns a borrowed reference.  */
-      PyObject *found = PyDict_GetItemWithError (d, file.get ());
+      PyObject *found = PyDict_GetItemWithError (globals, file.get ());
       if (found == nullptr)
 	{
 	  if (PyErr_Occurred ())
-	    return -1;
+	    return nullptr;
 
 	  gdbpy_ref<> filename_obj = host_string_to_python_string (filename);
 	  if (filename_obj == nullptr)
-	    return -1;
+	    return nullptr;
 
-	  if (PyDict_SetItem (d, file.get (), filename_obj.get ()) < 0)
-	    return -1;
-	  if (PyDict_SetItemString (d, "__cached__", Py_None) < 0)
-	    return -1;
+	  if (PyDict_SetItem (globals, file.get (), filename_obj.get ()) < 0)
+	    return nullptr;
+	  if (PyDict_SetItemString (globals, "__cached__", Py_None) < 0)
+	    return nullptr;
 
 	  file_set = true;
 	}
@@ -347,12 +346,12 @@ eval_python_command (const char *command, int start_symbol,
 				      : filename,
 				      start_symbol));
 
-  int result = -1;
+  gdbpy_ref<> eval_result;
   if (code != nullptr)
     {
-      gdbpy_ref<> eval_result (PyEval_EvalCode (code.get (), d, d));
-      if (eval_result != nullptr)
-	result = 0;
+      eval_result.reset (PyEval_EvalCode (code.get (), globals, globals));
+      if (eval_result == nullptr)
+	gdb_assert (PyErr_Occurred ());
     }
 
   if (file_set)
@@ -360,21 +359,21 @@ eval_python_command (const char *command, int start_symbol,
       /* If there's already an exception occurring, preserve it and
 	 restore it before returning from this function.  */
       std::optional<gdbpy_err_fetch> save_error;
-      if (result < 0)
+      if (eval_result == nullptr)
 	save_error.emplace ();
 
       /* CPython also just ignores errors here.  These should be
 	 expected to be exceedingly rare anyway.  */
-      if (PyDict_DelItemString (d, "__file__") < 0)
+      if (PyDict_DelItemString (globals, "__file__") < 0)
 	PyErr_Clear ();
-      if (PyDict_DelItemString (d, "__cached__") < 0)
+      if (PyDict_DelItemString (globals, "__cached__") < 0)
 	PyErr_Clear ();
 
       if (save_error.has_value ())
 	save_error->restore ();
     }
 
-  return result;
+  return eval_result;
 }
 
 /* Implementation of the gdb "python-interactive" command.  */
@@ -383,7 +382,7 @@ static void
 python_interactive_command (const char *arg, int from_tty)
 {
   struct ui *ui = current_ui;
-  int err;
+  bool err;
 
   scoped_restore save_async = make_scoped_restore (&current_ui->async, 0);
 
@@ -395,11 +394,11 @@ python_interactive_command (const char *arg, int from_tty)
     {
       std::string script = std::string (arg) + "\n";
       /* Py_single_input causes the result to be displayed.  */
-      err = eval_python_command (script.c_str (), Py_single_input);
+      err = ! eval_python_command (script.c_str (), Py_single_input);
     }
   else
     {
-      err = PyRun_InteractiveLoop (ui->instream, "<stdin>");
+      err = PyRun_InteractiveLoop (ui->instream, "<stdin>") != 0;
       dont_repeat ();
     }
 
@@ -410,6 +409,7 @@ python_interactive_command (const char *arg, int from_tty)
 /* Like PyRun_SimpleFile, but if there is an exception, it is not
    automatically displayed.  FILE is the Python script to run named
    FILENAME.
+   Return true on success, false on failure.
 
    On Windows hosts few users would build Python themselves (this is no
    trivial task on this platform), and thus use binaries built by
@@ -420,7 +420,7 @@ python_interactive_command (const char *arg, int from_tty)
    A FILE * from one runtime does not necessarily operate correctly in
    the other runtime.  */
 
-static int
+static bool
 python_run_simple_file (FILE *file, const char *filename)
 {
   std::string contents = read_remainder_of_file (file);
@@ -458,8 +458,7 @@ gdbpy_eval_from_control_command (const struct extension_language_defn *extlang,
   gdbpy_enter enter_py;
 
   std::string script = compute_python_string (cmd->body_list_0.get ());
-  int ret = eval_python_command (script.c_str (), Py_file_input);
-  if (ret != 0)
+  if (! eval_python_command (script.c_str (), Py_file_input))
     gdbpy_handle_exception ();
 }
 
@@ -475,8 +474,7 @@ python_command (const char *arg, int from_tty)
   arg = skip_spaces (arg);
   if (arg && *arg)
     {
-      int ret = eval_python_command (arg, Py_file_input);
-      if (ret != 0)
+      if (! eval_python_command (arg, Py_file_input))
 	gdbpy_handle_exception ();
     }
   else
@@ -1119,8 +1117,7 @@ gdbpy_source_script (const struct extension_language_defn *extlang,
 		     FILE *file, const char *filename)
 {
   gdbpy_enter enter_py;
-  int result = python_run_simple_file (file, filename);
-  if (result != 0)
+  if (! python_run_simple_file (file, filename))
     gdbpy_handle_exception ();
 }
 
@@ -1826,8 +1823,7 @@ gdbpy_source_objfile_script (const struct extension_language_defn *extlang,
   scoped_restore restire_current_objfile
     = make_scoped_restore (&gdbpy_current_objfile, objfile);
 
-  int result = python_run_simple_file (file, filename);
-  if (result != 0)
+  if (! python_run_simple_file (file, filename))
     gdbpy_print_stack ();
 }
 
@@ -1849,8 +1845,7 @@ gdbpy_execute_objfile_script (const struct extension_language_defn *extlang,
   scoped_restore restire_current_objfile
     = make_scoped_restore (&gdbpy_current_objfile, objfile);
 
-  int ret = eval_python_command (script, Py_file_input);
-  if (ret != 0)
+  if (! eval_python_command (script, Py_file_input))
     gdbpy_print_stack ();
 }
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index edd94ea4963..a8c991e50eb 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1365,20 +1365,13 @@ void
 varobj_set_visualizer (struct varobj *var, const char *visualizer)
 {
 #if HAVE_PYTHON
-  PyObject *mainmod;
-
   if (!gdb_python_initialized)
     return;
 
   gdbpy_enter_varobj enter_py (var);
 
-  mainmod = PyImport_AddModule ("__main__");
-  gdbpy_ref<> globals
-    = gdbpy_ref<>::new_reference (PyModule_GetDict (mainmod));
-  gdbpy_ref<> constructor (PyRun_String (visualizer, Py_eval_input,
-					 globals.get (), globals.get ()));
-
-  if (constructor == NULL)
+  auto constructor = eval_python_command (visualizer, Py_eval_input);
+  if (constructor == nullptr)
     {
       gdbpy_print_stack ();
       error (_("Could not evaluate visualizer expression: %s"), visualizer);
diff --git a/gdbsupport/gdb_ref_ptr.h b/gdbsupport/gdb_ref_ptr.h
index 0eb654324c6..f7c7c0afac6 100644
--- a/gdbsupport/gdb_ref_ptr.h
+++ b/gdbsupport/gdb_ref_ptr.h
@@ -179,6 +179,11 @@ class ref_ptr
     return m_obj;
   }
 
+  operator bool () const noexcept
+  {
+    return m_obj != nullptr;
+  }
+
   /* Return this instance's referent, and stop managing this
      reference.  The caller is now responsible for the ownership of
      the reference.  */
-- 
2.54.0