Re: [PATCH 2/2] gdb/python: implement support for sending custom MI async notifications

[Andrew Burgess <aburgess@redhat.com>]

Jan Vrany via Gdb-patches <gdb-patches@sourceware.org> writes:

> This commit adds a new Python function, gdb.notify_mi, that can be used
> to emit custom async notification to MI channel.  This can be used, among
> other things, to implement notifications about events MI does not support,
> such as remote connection closed or register change.
> ---
>  gdb/NEWS                                  |  3 ++
>  gdb/doc/python.texi                       | 38 ++++++++++++++++++
>  gdb/python/py-mi.c                        | 49 +++++++++++++++++++++++
>  gdb/python/python-internal.h              |  3 ++
>  gdb/python/python.c                       |  4 ++
>  gdb/testsuite/gdb.python/py-mi-notify.exp | 47 ++++++++++++++++++++++
>  6 files changed, 144 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.python/py-mi-notify.exp
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 98ff00d5efc..4a1f383a666 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -286,6 +286,9 @@ info main
>       might be array- or string-like, even if they do not have the
>       corresponding type code.
>  
> +  ** New function gdb.notify_mi(NAME, DATA), that emits custom
> +     GDB/MI async notification.
> +
>  *** Changes in GDB 13
>  
>  * MI version 1 is deprecated, and will be removed in GDB 14.
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index e9936991c49..cb2073976ba 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -4704,6 +4704,44 @@ Here is how this works using the commands from the example above:
>  @{'string': 'abc, def, ghi'@}
>  @end smallexample
>  
> +@node GDB/MI Notifications In Python
> +@subsubsection @sc{gdb/mi} Notifications In Python
> +
> +@cindex MI notifications in python
> +@cindex notifications in python, GDB/MI
> +@cindex python notifications, GDB/MI
> +
> +It is possible to emit @sc{gdb/mi} notifications from
> +Python.  This is done with the @code{gdb.notify_mi} function.
> +
> +@defun gdb.notify_mi (name , data)
> +Emit a @sc{gdb/mi} asynchronous notification.  @var{name} is the name of the
> +notification, a string.  @var{data} are additional values emitted with the notification, passed
> +as Python dictionary. The dictionary is converted to converted
> +to a @sc{gdb/mi} @var{result-record} (@pxref{GDB/MI Output Syntax}) the same way
> +as result of Python MI command (@pxref{GDB/MI Commands In Python}).
> +
> +If @var{data} is @code{None} then no additional values are emitted.
> +@end defun
> +
> +Here is how to emit @code{=connection-removed} whenever a connection to remote
> +GDB server is closed (see @pxref{Connections In Python}):
> +
> +@smallexample
> +def notify_connection_removed (event):
> +    data = @{ 'id'   : event.connection.num,
> +              'type' : event.connection.type @}
> +    gdb.notify_mi("connection-removed", data)
> +
> +gdb.events.connection_removed.connect (notify_connection_removed)
> +@end smallexample
> +
> +Then, each time a connection is closed, there will be a notification on MI channel:
> +
> +@smallexample
> +=connection-removed,id="1",type="remote"
> +@end smallexample
> +
>  @node Parameters In Python
>  @subsubsection Parameters In Python
>  
> diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
> index 66dc6fb8a32..dc2ec5ddd0f 100644
> --- a/gdb/python/py-mi.c
> +++ b/gdb/python/py-mi.c
> @@ -19,8 +19,14 @@
>  
>  #include "defs.h"
>  #include "python-internal.h"
> +#include "utils.h"
> +#include "ui.h"
>  #include "ui-out.h"
> +#include "interps.h"
> +#include "target.h"
>  #include "mi/mi-parse.h"
> +#include "mi/mi-console.h"
> +#include "mi/mi-interp.h"
>  
>  /* A ui_out subclass that creates a Python object based on the data
>     that is passed in.  */
> @@ -296,3 +302,46 @@ gdbpy_execute_mi_command (PyObject *self, PyObject *args, PyObject *kw)
>  
>    return uiout.result ();
>  }
> +
> +/* Implementation of the gdb.notify_mi function.  */
> +
> +PyObject *
> +gdbpy_notify_mi (PyObject *self, PyObject *args, PyObject *kwargs)
> +{
> +  static const char *keywords[] = { "name", "data", nullptr };
> +  const char *name;
> +  PyObject *data;
> +
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "sO", keywords,
> +					&name, &data))
> +    return nullptr; // FIXME: is this the correct way to signal error?

I knew there was something else I wanted to say :)

I think you should add some validation of 'name' here too.  If you check
patch 1/2 in this series and look for 'is_valid_key_name', you should do
something similar for 'name', and document the restrictions. e.g. I
don't think a name like "  *** bad idea ***  " would be a great choice
for a notification name.

Thanks,
Andrew

> +
> +  SWITCH_THRU_ALL_UIS ()
> +    {
> +      struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
> +
> +      if (mi == NULL)
> +        continue;
> +      
> +      gdb_printf (mi->event_channel, "%s", name);
> +      if (data != Py_None)      
> +        {
> +          /* At the top-level, the data must be a dictionary.  */
> +          if (!PyDict_Check (data))
> +            gdbpy_error (_("Data passed to notify_mi must be either None or a dictionary"));
> +
> +          target_terminal::scoped_restore_terminal_state term_state;
> +          target_terminal::ours_for_output ();
> +
> +          ui_out *mi_uiout = mi->interp_ui_out ();
> +          ui_out_redirect_pop redir (mi_uiout, mi->event_channel);
> +          scoped_restore restore_uiout
> +            = make_scoped_restore (&current_uiout, mi_uiout);
> +
> +          serialize_mi_data (data);	  
> +        }
> +      gdb_flush (mi->event_channel);
> +    }
> +    
> +  Py_RETURN_NONE;
> +}
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index 3f53b0ab6f0..2a7e8d68179 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -501,6 +501,9 @@ extern PyObject *gdbpy_execute_mi_command (PyObject *self, PyObject *args,
>  
>  extern void serialize_mi_data (PyObject *result);
>  
> +extern PyObject *gdbpy_notify_mi (PyObject *self, PyObject *args,
> +				  PyObject *kw);
> +
>  /* Convert Python object OBJ to a program_space pointer.  OBJ must be a
>     gdb.Progspace reference.  Return nullptr if the gdb.Progspace is not
>     valid (see gdb.Progspace.is_valid), otherwise return the program_space
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 6a978d632e9..faa7e0c217d 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -2669,6 +2669,10 @@ Return the name of the currently selected language." },
>      "print_options () -> dict\n\
>  Return the current print options." },
>  
> +  { "notify_mi", (PyCFunction) gdbpy_notify_mi,
> +    METH_VARARGS | METH_KEYWORDS,
> +    "notify_mi (name, data) -> None\n\
> +Output async record to MI channels if any." },
>    {NULL, NULL, 0, NULL}
>  };
>  
> diff --git a/gdb/testsuite/gdb.python/py-mi-notify.exp b/gdb/testsuite/gdb.python/py-mi-notify.exp
> new file mode 100644
> index 00000000000..8221794c4f3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-mi-notify.exp
> @@ -0,0 +1,47 @@
> +# Copyright (C) 2019-2023 Free Software Foundation, Inc.
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test custom MI notifications implemented in Python.
> +
> +load_lib gdb-python.exp
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +gdb_exit
> +if {[mi_gdb_start]} {
> +    return
> +}
> +
> +if {[lsearch -exact [mi_get_features] python] < 0} {
> +    unsupported "python support is disabled"
> +    return -1
> +}
> +
> +standard_testfile
> +
> +mi_gdb_test "set python print-stack full" \
> +    ".*\\^done" \
> +    "set python print-stack full"
> +
> +mi_gdb_test "python gdb.notify_mi('test-notification', None)" \
> +    ".*=test-notification\r\n\\^done" \
> +    "python notification, no additional data"
> +
> +mi_gdb_test "python gdb.notify_mi('test-notification', \{ 'data1' : 1 , 'data2' : 2 })" \
> +    ".*=test-notification,data1=\"1\",data2=\"2\"\r\n\\^done" \
> +    "python notification, with additional data"
> +
> +mi_gdb_test "python gdb.notify_mi('test-notification', 1)" \
> +    ".*\\^error,msg=\".*\"" \
> +    "python notification, invalid additional data"
> -- 
> 2.40.1