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

["Jan Vraný" <Jan.Vrany@labware.com>]

On Mon, 2023-09-11 at 15:21 +0100, Andrew Burgess wrote:
> 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.

Yes, that's exactly what I have added to (not yet submitted) V2. 

Thanks! Jan
> 
> 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