[PATCH v1 2/4] gdb/python: eval_python_command returns both exit code and result

[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 both the exit code and the result
of the evaluation.

This patch updates eval_python_command() to return both values and
adjusts existing callers accordingly. It also 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 |  3 +-
 gdb/python/python-internal.h |  4 +--
 gdb/python/python.c          | 54 +++++++++++++++++++-----------------
 gdb/varobj.c                 | 12 ++------
 4 files changed, 35 insertions(+), 38 deletions(-)

diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
index e8e2c23547c..eedeb659c18 100644
--- a/gdb/python/py-gdb-readline.c
+++ b/gdb/python/py-gdb-readline.c
@@ -114,7 +114,8 @@ class GdbRemoveReadlineFinder(MetaPathFinder):\n\
 \n\
 sys.meta_path.insert(2, GdbRemoveReadlineFinder())\n\
 ";
-  if (eval_python_command (code, Py_file_input) == 0)
+  auto [ret, _] = eval_python_command (code, Py_file_input);
+  if (ret == 0)
     PyOS_ReadlineFunctionPointer = gdbpy_readline_wrapper;
 
   return 0;
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 37bc37691fe..ca70ca57fd2 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 std::pair<int, 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 4182c699cb5..85bd88cd243 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -299,42 +299,40 @@ gdbpy_check_quit_flag (const struct extension_language_defn *extlang)
    NULL means that this is evaluating a string, not the contents of a
    file.  */
 
-int
+std::pair<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 std::make_pair(-1, nullptr);
 
-  d = PyModule_GetDict (m);
-  if (d == NULL)
-    return -1;
+  gdbpy_borrowed_ref<> globals = PyModule_GetDict (mainmod);
+  if (globals == nullptr)
+    return std::make_pair(-1, nullptr);
 
   bool file_set = false;
   if (filename != nullptr)
     {
       gdbpy_ref<> file = host_string_to_python_string ("__file__");
       if (file == nullptr)
-	return -1;
+	return std::make_pair(-1, 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 std::make_pair(-1, nullptr);
 
 	  gdbpy_ref<> filename_obj = host_string_to_python_string (filename);
 	  if (filename_obj == nullptr)
-	    return -1;
+	    return std::make_pair(-1, 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 std::make_pair(-1, nullptr);
+	  if (PyDict_SetItemString (globals, "__cached__", Py_None) < 0)
+	    return std::make_pair(-1, nullptr);
 
 	  file_set = true;
 	}
@@ -348,9 +346,10 @@ eval_python_command (const char *command, int start_symbol,
 				      start_symbol));
 
   int result = -1;
+  gdbpy_ref<> eval_result;
   if (code != nullptr)
     {
-      gdbpy_ref<> eval_result (PyEval_EvalCode (code.get (), d, d));
+      eval_result.reset (PyEval_EvalCode (code.get (), globals, globals));
       if (eval_result != nullptr)
 	result = 0;
     }
@@ -365,16 +364,16 @@ eval_python_command (const char *command, int start_symbol,
 
       /* 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 std::make_pair(result, eval_result);
 }
 
 /* Implementation of the gdb "python-interactive" command.  */
@@ -395,7 +394,8 @@ 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);
+      std::tie (err, std::ignore)
+	= eval_python_command (script.c_str (), Py_single_input);
     }
   else
     {
@@ -424,7 +424,9 @@ static int
 python_run_simple_file (FILE *file, const char *filename)
 {
   std::string contents = read_remainder_of_file (file);
-  return eval_python_command (contents.c_str (), Py_file_input, filename);
+  auto [ret, _]
+    = eval_python_command (contents.c_str (), Py_file_input, filename);
+  return ret;
 }
 
 /* Given a command_line, return a command string suitable for passing
@@ -457,7 +459,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);
+  auto [ret, _] = eval_python_command (script.c_str (), Py_file_input);
   if (ret != 0)
     gdbpy_handle_exception ();
 }
@@ -474,7 +476,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);
+      auto [ret, _] = eval_python_command (arg, Py_file_input);
       if (ret != 0)
 	gdbpy_handle_exception ();
     }
@@ -1848,7 +1850,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);
+  auto [ret, _] = eval_python_command (script, Py_file_input);
   if (ret != 0)
     gdbpy_print_stack ();
 }
diff --git a/gdb/varobj.c b/gdb/varobj.c
index edd94ea4963..b20fc78eb28 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1365,20 +1365,14 @@ 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 [ret_code, constructor]
+    = eval_python_command (visualizer, Py_eval_input);
+  if (ret_code != 0 || constructor == nullptr)
     {
       gdbpy_print_stack ();
       error (_("Could not evaluate visualizer expression: %s"), visualizer);
-- 
2.53.0