RFA: add to_string argument to gdb.execute

RFA: add to_string argument to gdb.execute

[Tom Tromey]

I plan to check this in.  It needs a doc review.

This adds a to_string argument to gdb.execute, so you can capture
command output in a python string.

I've gone back and forth on how I wanted to implement this idea, but in
the end I settled on this approach as very simple and useful.

This patch also adds keyword arguments to gdb.execute.  I think our rule
should be that any function with 2 or more arguments should take keyword
arguments; we violate this in a couple of places.

There is still room for a bigger change, involving structured output
from gdb.  My plan there is to expose MI commands to Python, and make a
new kind of ui_out that creates Python objects.  I think this approach
has several nice points: we know it will be fairly complete (because MI
is), and we already have documentation.

Built and regtested on x86-64 (compile farm).

Tom

2010-06-10  Tom Tromey  <tromey@redhat.com>

	PR python/10808:
	* python/python.c (execute_gdb_command): Add keywords.  Accept
	"to_string" argument.
	(struct restore_ui_file_closure): New.
	(restore_ui_file): New function.
	(make_cleanup_restore_ui_file): Likewise.
	(GdbMethods) <execute>: Update.

2010-06-10  Tom Tromey  <tromey@redhat.com>

	PR python/10808:
	* gdb.texinfo (Basic Python): Document new gdb.execute argument.

2010-06-10  Tom Tromey  <tromey@redhat.com>

	PR python/10808:
	* gdb.python/python.exp: Add new tests.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index fa7a0ec..e7af24a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20045,15 +20045,20 @@ methods and classes added by @value{GDBN} are placed in this module.
 use in all scripts evaluated by the @code{python} command.
 
 @findex gdb.execute
-@defun execute command [from_tty]
+@defun execute command [from_tty] [to_string]
 Evaluate @var{command}, a string, as a @value{GDBN} CLI command.
 If a GDB exception happens while @var{command} runs, it is
 translated as described in @ref{Exception Handling,,Exception Handling}.
-If no exceptions occur, this function returns @code{None}.
 
 @var{from_tty} specifies whether @value{GDBN} ought to consider this
 command as having originated from the user invoking it interactively.
 It must be a boolean value.  If omitted, it defaults to @code{False}.
+
+By default, any output produced by @var{command} is sent to
+@value{GDBN}'s standard output.  If the @var{to_string} parameter is
+@code{True}, then output will be collected by @code{gdb.execute} and
+returned as a string.  The default is @code{False}, in which case the
+return value is @code{None}.
 @end defun
 
 @findex gdb.breakpoints
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 31880c1..c4d4a55 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -309,36 +309,94 @@ gdbpy_target_wide_charset (PyObject *self, PyObject *args)
   return PyUnicode_Decode (cset, strlen (cset), host_charset (), NULL);
 }
 
+struct restore_ui_file_closure
+{
+  struct ui_file **variable;
+  struct ui_file *value;
+};
+
+static void
+restore_ui_file (void *p)
+{
+  struct restore_ui_file_closure *closure = p;
+
+  *(closure->variable) = closure->value;
+}
+
+/* Remember the current value of *VARIABLE and make it restored when
+   the cleanup is run.  */
+struct cleanup *
+make_cleanup_restore_ui_file (struct ui_file **variable)
+{
+  struct restore_ui_file_closure *c = XNEW (struct restore_ui_file_closure);
+
+  c->variable = variable;
+  c->value = *variable;
+
+  return make_cleanup_dtor (restore_ui_file, (void *) c, xfree);
+}
+
 /* A Python function which evaluates a string using the gdb CLI.  */
 
 static PyObject *
-execute_gdb_command (PyObject *self, PyObject *args)
+execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
 {
   char *arg;
-  PyObject *from_tty_obj = NULL;
-  int from_tty;
-  int cmp;
+  PyObject *from_tty_obj = NULL, *to_string_obj = NULL;
+  int from_tty, to_string;
   volatile struct gdb_exception except;
+  static char *keywords[] = {"command", "from_tty", "to_string", NULL };
+  char *result = NULL;
 
-  if (! PyArg_ParseTuple (args, "s|O!", &arg, &PyBool_Type, &from_tty_obj))
+  if (! PyArg_ParseTupleAndKeywords (args, kw, "s|O!O!", keywords, &arg,
+				     &PyBool_Type, &from_tty_obj,
+				     &PyBool_Type, &to_string_obj))
     return NULL;
 
   from_tty = 0;
   if (from_tty_obj)
     {
-      cmp = PyObject_IsTrue (from_tty_obj);
+      int cmp = PyObject_IsTrue (from_tty_obj);
       if (cmp < 0)
-	  return NULL;
+	return NULL;
       from_tty = cmp;
     }
 
+  to_string = 0;
+  if (to_string_obj)
+    {
+      int cmp = PyObject_IsTrue (to_string_obj);
+      if (cmp < 0)
+	return NULL;
+      to_string = cmp;
+    }
+
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
       /* Copy the argument text in case the command modifies it.  */
       char *copy = xstrdup (arg);
       struct cleanup *cleanup = make_cleanup (xfree, copy);
+      struct ui_file *str_file = NULL;
+
+      if (to_string)
+	{
+	  str_file = mem_fileopen ();
+
+	  make_cleanup_restore_ui_file (&gdb_stdout);
+	  make_cleanup_restore_ui_file (&gdb_stderr);
+	  make_cleanup_ui_file_delete (str_file);
+
+	  gdb_stdout = str_file;
+	  gdb_stderr = str_file;
+	}
 
       execute_command (copy, from_tty);
+
+      if (str_file)
+	result = ui_file_xstrdup (str_file, NULL);
+      else
+	result = NULL;
+
       do_cleanups (cleanup);
     }
   GDB_PY_HANDLE_EXCEPTION (except);
@@ -346,6 +404,12 @@ execute_gdb_command (PyObject *self, PyObject *args)
   /* Do any commands attached to breakpoint we stopped at.  */
   bpstat_do_actions ();
 
+  if (result)
+    {
+      PyObject *r = PyString_FromString (result);
+      xfree (result);
+      return r;
+    }
   Py_RETURN_NONE;
 }
 
@@ -737,7 +801,7 @@ static PyMethodDef GdbMethods[] =
 {
   { "history", gdbpy_history, METH_VARARGS,
     "Get a value from history" },
-  { "execute", execute_gdb_command, METH_VARARGS,
+  { "execute", (PyCFunction) execute_gdb_command, METH_VARARGS | METH_KEYWORDS,
     "Execute a gdb command" },
   { "parameter", gdbpy_parameter, METH_VARARGS,
     "Return a gdb parameter's value" },
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index b345ad2..f7f11cc 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -80,3 +80,7 @@ gdb_test "source $srcdir/$subdir/source2.py" "yes"
 
 gdb_test "python print gdb.current_objfile()" "None"
 gdb_test "python print gdb.objfiles()" "\\\[\\\]"
+
+gdb_test_no_output \
+    "python x = gdb.execute('printf \"%d\", 23', to_string = True)"
+gdb_test "python print x" "23"

Re: RFA: add to_string argument to gdb.execute

[Doug Evans]

On Thu, Jun 10, 2010 at 1:26 PM, Tom Tromey <tromey@redhat.com> wrote:
> I plan to check this in.  It needs a doc review.
>
> This adds a to_string argument to gdb.execute, so you can capture
> command output in a python string.
>
> I've gone back and forth on how I wanted to implement this idea, but in
> the end I settled on this approach as very simple and useful.
>
> This patch also adds keyword arguments to gdb.execute.  I think our rule
> should be that any function with 2 or more arguments should take keyword
> arguments; we violate this in a couple of places.
>
> There is still room for a bigger change, involving structured output
> from gdb.  My plan there is to expose MI commands to Python, and make a
> new kind of ui_out that creates Python objects.  I think this approach
> has several nice points: we know it will be fairly complete (because MI
> is), and we already have documentation.

This feels like a with-output-to-string kind-of-problem.
i.e. the wrapping of collecting the output to a string belongs at a
higher level.

Scheme example:

(define x (with-output-to-string (lambda () (display "foo"))))

That way one can wrap anything and have all the gdb output collected
without having to modify all the individual gdb.execute calls inside.
Dunno if it's as easy to implement though.

Re: RFA: add to_string argument to gdb.execute

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> This feels like a with-output-to-string kind-of-problem.
Doug> i.e. the wrapping of collecting the output to a string belongs at a
Doug> higher level.

It seems reasonable to me to have both.
I've never wanted more than a single command's worth of output fwiw.

Doug> That way one can wrap anything and have all the gdb output collected
Doug> without having to modify all the individual gdb.execute calls inside.
Doug> Dunno if it's as easy to implement though.

It is not extremely hard with the Python 'with' statement.
I think it will be a lot more code though, on the order of a new
class.

I don't plan to work on that.  If you are against this patch, please say
so, and I will drop it.

Tom

Re: RFA: add to_string argument to gdb.execute

[Doug Evans]

On Thu, Jun 10, 2010 at 2:06 PM, Tom Tromey <tromey@redhat.com> wrote:
> If you are against this patch, please say
> so, and I will drop it.

No objections, just checking.
I don't mind having both.

Re: RFA: add to_string argument to gdb.execute

[Daniel Jacobowitz]

On Thu, Jun 10, 2010 at 02:26:44PM -0600, Tom Tromey wrote:
> This adds a to_string argument to gdb.execute, so you can capture
> command output in a python string.

Thank you, Tom!  I like this solution.

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFA: add to_string argument to gdb.execute

[Tom Tromey]

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

Tom> I plan to check this in.  It needs a doc review.

Ping.

Tom

Re: RFA: add to_string argument to gdb.execute

[Tom Tromey]

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

Tom> I plan to check this in.  It needs a doc review.

Tom> Ping.

Ping.

Tom

Re: RFA: add to_string argument to gdb.execute

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> CC: Eli Zaretskii <eliz@gnu.org>
> Date: Fri, 25 Jun 2010 10:55:05 -0600
> 
> >>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
> 
> Tom> I plan to check this in.  It needs a doc review.
> 
> Tom> Ping.
> 
> Ping.

Oops, sorry.  The docs part is okay to go in.