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

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

[Jan Vrany]

I hope I addressed all comments raised by Andrew and Eli
in this version. The main change is added restriction on
NAME parameter and hopefully more clear wording in
documentation.

Thanks, Jan

Changes since V1:

* Moved serialize_mi_result and helpers to py-mi.c as suggested.
  Also, renamed serialize_mi_result to serialize_mi_results
  instead of serialize_mi_data and do not rename serialize_mi_result_1.

  The reason for this is that GDB documentation,
  section GDB/MI Output Syntax, describes "result" being single 'variable = value'
  pair and "result-record" and "async-output" contains zero or more of these "result"s,
  so calling top-level serialization function serialize_mi_results seems
  a better name than serialize_mi_data (used in previous version).

* Made gdb.notify_mi DATA parameter optional as suggested.

* Validate gdb.notify_mi NAME parameter as suggested.

* Updated documentation:
  * example formatted as black tool would do it,
  * rephrase some sentences as suggested,
  * document that DATA parameter is optional,
  * document restrictions on NAME parameter and
  * document that users should prefix user-defined notification
    with hyphen to avoid possible conflicts.

* Add more tests.


Jan Vrany (2):
  gdb/python: generalize serialize_mi_result()
  gdb/python: implement support for sending custom MI async
    notifications

 gdb/NEWS                                  |   3 +
 gdb/doc/python.texi                       |  45 +++++
 gdb/python/py-mi.c                        | 230 ++++++++++++++++++++++
 gdb/python/py-micmd.c                     | 185 +----------------
 gdb/python/python-internal.h              |  18 ++
 gdb/python/python.c                       |   4 +
 gdb/testsuite/gdb.python/py-mi-notify.exp |  71 +++++++
 7 files changed, 380 insertions(+), 176 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-mi-notify.exp

-- 
2.40.1

[PATCH v2 1/2] gdb/python: generalize serialize_mi_result()

[Jan Vrany]

This commit generalizes serialize_mi_result() to make usable in
different contexts than printing result of custom MI command.

To do so, the check whether passed Python object is a dictionary has been
moved to the caller - at the very least, different uses require different
error messages.  Also it has been renamed to serialize_mi_results() to better
match GDB/MI output syntax (see corresponding section in documentation,
in particular rules 'result-record' and 'async-output'.

Since it is now more generic function, it has been moved to py-mi.c.

This is a preparation for implementing Python support for sending custom
MI async events.
---
 gdb/python/py-mi.c           | 159 ++++++++++++++++++++++++++++++
 gdb/python/py-micmd.c        | 185 ++---------------------------------
 gdb/python/python-internal.h |  13 +++
 3 files changed, 181 insertions(+), 176 deletions(-)

diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
index 66dc6fb8a32..36bcb6ceece 100644
--- a/gdb/python/py-mi.c
+++ b/gdb/python/py-mi.c
@@ -296,3 +296,162 @@ gdbpy_execute_mi_command (PyObject *self, PyObject *args, PyObject *kw)
 
   return uiout.result ();
 }
+
+/* Convert KEY_OBJ into a string that can be used as a field name in MI
+   output.  KEY_OBJ must be a Python string object, and must only contain
+   characters suitable for use as an MI field name.
+
+   If KEY_OBJ is not a string, or if KEY_OBJ contains invalid characters,
+   then an error is thrown.  Otherwise, KEY_OBJ is converted to a string
+   and returned.  */
+
+static gdb::unique_xmalloc_ptr<char>
+py_object_to_mi_key (PyObject *key_obj)
+{
+  /* The key must be a string.  */
+  if (!PyUnicode_Check (key_obj))
+    {
+      gdbpy_ref<> key_repr (PyObject_Repr (key_obj));
+      gdb::unique_xmalloc_ptr<char> key_repr_string;
+      if (key_repr != nullptr)
+	key_repr_string = python_string_to_target_string (key_repr.get ());
+      if (key_repr_string == nullptr)
+	gdbpy_handle_exception ();
+
+      gdbpy_error (_("non-string object used as key: %s"),
+		   key_repr_string.get ());
+    }
+
+  gdb::unique_xmalloc_ptr<char> key_string
+    = python_string_to_target_string (key_obj);
+  if (key_string == nullptr)
+    gdbpy_handle_exception ();
+
+  /* Predicate function, returns true if NAME is a valid field name for use
+     in MI result output, otherwise, returns false.  */
+  auto is_valid_key_name = [] (const char *name) -> bool
+  {
+    gdb_assert (name != nullptr);
+
+    if (*name == '\0' || !isalpha (*name))
+      return false;
+
+    for (; *name != '\0'; ++name)
+      if (!isalnum (*name) && *name != '_' && *name != '-')
+	return false;
+
+    return true;
+  };
+
+  if (!is_valid_key_name (key_string.get ()))
+    {
+      if (*key_string.get () == '\0')
+	gdbpy_error (_("Invalid empty key in MI result"));
+      else
+	gdbpy_error (_("Invalid key in MI result: %s"), key_string.get ());
+    }
+
+  return key_string;
+}
+
+/* Serialize RESULT and print it in MI format to the current_uiout.
+   FIELD_NAME is used as the name of this result field.
+
+   RESULT can be a dictionary, a sequence, an iterator, or an object that
+   can be converted to a string, these are converted to the matching MI
+   output format (dictionaries as tuples, sequences and iterators as lists,
+   and strings as named fields).
+
+   If anything goes wrong while formatting the output then an error is
+   thrown.
+
+   This function is the recursive inner core of serialize_mi_result, and
+   should only be called from that function.  */
+
+static void
+serialize_mi_result_1 (PyObject *result, const char *field_name)
+{
+  struct ui_out *uiout = current_uiout;
+
+  if (PyDict_Check (result))
+    {
+      PyObject *key, *value;
+      Py_ssize_t pos = 0;
+      ui_out_emit_tuple tuple_emitter (uiout, field_name);
+      while (PyDict_Next (result, &pos, &key, &value))
+	{
+	  gdb::unique_xmalloc_ptr<char> key_string
+	    (py_object_to_mi_key (key));
+	  serialize_mi_result_1 (value, key_string.get ());
+	}
+    }
+  else if (PySequence_Check (result) && !PyUnicode_Check (result))
+    {
+      ui_out_emit_list list_emitter (uiout, field_name);
+      Py_ssize_t len = PySequence_Size (result);
+      if (len == -1)
+	gdbpy_handle_exception ();
+      for (Py_ssize_t i = 0; i < len; ++i)
+	{
+	  gdbpy_ref<> item (PySequence_ITEM (result, i));
+	  if (item == nullptr)
+	    gdbpy_handle_exception ();
+	  serialize_mi_result_1 (item.get (), nullptr);
+	}
+    }
+  else if (PyIter_Check (result))
+    {
+      gdbpy_ref<> item;
+      ui_out_emit_list list_emitter (uiout, field_name);
+      while (true)
+	{
+	  item.reset (PyIter_Next (result));
+	  if (item == nullptr)
+	    {
+	      if (PyErr_Occurred () != nullptr)
+		gdbpy_handle_exception ();
+	      break;
+	    }
+	  serialize_mi_result_1 (item.get (), nullptr);
+	}
+    }
+  else
+    {
+      if (PyLong_Check (result))
+	{
+	  int overflow = 0;
+	  gdb_py_longest val = gdb_py_long_as_long_and_overflow (result,
+								 &overflow);
+	  if (PyErr_Occurred () != nullptr)
+	    gdbpy_handle_exception ();
+	  if (overflow == 0)
+	    {
+	      uiout->field_signed (field_name, val);
+	      return;
+	    }
+	  /* Fall through to the string case on overflow.  */
+	}
+
+      gdb::unique_xmalloc_ptr<char> string (gdbpy_obj_to_string (result));
+      if (string == nullptr)
+	gdbpy_handle_exception ();
+      uiout->field_string (field_name, string.get ());
+    }
+}
+
+/* See python-internal.h.  */
+
+void
+serialize_mi_results (PyObject *results)
+{
+  gdb_assert (PyDict_Check (results));
+
+  PyObject *key, *value;
+  Py_ssize_t pos = 0;
+  while (PyDict_Next (results, &pos, &key, &value))
+    {
+      gdb::unique_xmalloc_ptr<char> key_string
+	(py_object_to_mi_key (key));
+      serialize_mi_result_1 (value, key_string.get ());
+    }
+}
diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
index 01fc6060ece..dbfd44c892b 100644
--- a/gdb/python/py-micmd.c
+++ b/gdb/python/py-micmd.c
@@ -173,178 +173,6 @@ extern PyTypeObject micmdpy_object_type
 
 static PyObject *invoke_cst;
 
-/* Convert KEY_OBJ into a string that can be used as a field name in MI
-   output.  KEY_OBJ must be a Python string object, and must only contain
-   characters suitable for use as an MI field name.
-
-   If KEY_OBJ is not a string, or if KEY_OBJ contains invalid characters,
-   then an error is thrown.  Otherwise, KEY_OBJ is converted to a string
-   and returned.  */
-
-static gdb::unique_xmalloc_ptr<char>
-py_object_to_mi_key (PyObject *key_obj)
-{
-  /* The key must be a string.  */
-  if (!PyUnicode_Check (key_obj))
-    {
-      gdbpy_ref<> key_repr (PyObject_Repr (key_obj));
-      gdb::unique_xmalloc_ptr<char> key_repr_string;
-      if (key_repr != nullptr)
-	key_repr_string = python_string_to_target_string (key_repr.get ());
-      if (key_repr_string == nullptr)
-	gdbpy_handle_exception ();
-
-      gdbpy_error (_("non-string object used as key: %s"),
-		   key_repr_string.get ());
-    }
-
-  gdb::unique_xmalloc_ptr<char> key_string
-    = python_string_to_target_string (key_obj);
-  if (key_string == nullptr)
-    gdbpy_handle_exception ();
-
-  /* Predicate function, returns true if NAME is a valid field name for use
-     in MI result output, otherwise, returns false.  */
-  auto is_valid_key_name = [] (const char *name) -> bool
-  {
-    gdb_assert (name != nullptr);
-
-    if (*name == '\0' || !isalpha (*name))
-      return false;
-
-    for (; *name != '\0'; ++name)
-      if (!isalnum (*name) && *name != '_' && *name != '-')
-	return false;
-
-    return true;
-  };
-
-  if (!is_valid_key_name (key_string.get ()))
-    {
-      if (*key_string.get () == '\0')
-	gdbpy_error (_("Invalid empty key in MI result"));
-      else
-	gdbpy_error (_("Invalid key in MI result: %s"), key_string.get ());
-    }
-
-  return key_string;
-}
-
-/* Serialize RESULT and print it in MI format to the current_uiout.
-   FIELD_NAME is used as the name of this result field.
-
-   RESULT can be a dictionary, a sequence, an iterator, or an object that
-   can be converted to a string, these are converted to the matching MI
-   output format (dictionaries as tuples, sequences and iterators as lists,
-   and strings as named fields).
-
-   If anything goes wrong while formatting the output then an error is
-   thrown.
-
-   This function is the recursive inner core of serialize_mi_result, and
-   should only be called from that function.  */
-
-static void
-serialize_mi_result_1 (PyObject *result, const char *field_name)
-{
-  struct ui_out *uiout = current_uiout;
-
-  if (PyDict_Check (result))
-    {
-      PyObject *key, *value;
-      Py_ssize_t pos = 0;
-      ui_out_emit_tuple tuple_emitter (uiout, field_name);
-      while (PyDict_Next (result, &pos, &key, &value))
-	{
-	  gdb::unique_xmalloc_ptr<char> key_string
-	    (py_object_to_mi_key (key));
-	  serialize_mi_result_1 (value, key_string.get ());
-	}
-    }
-  else if (PySequence_Check (result) && !PyUnicode_Check (result))
-    {
-      ui_out_emit_list list_emitter (uiout, field_name);
-      Py_ssize_t len = PySequence_Size (result);
-      if (len == -1)
-	gdbpy_handle_exception ();
-      for (Py_ssize_t i = 0; i < len; ++i)
-	{
-	  gdbpy_ref<> item (PySequence_ITEM (result, i));
-	  if (item == nullptr)
-	    gdbpy_handle_exception ();
-	  serialize_mi_result_1 (item.get (), nullptr);
-	}
-    }
-  else if (PyIter_Check (result))
-    {
-      gdbpy_ref<> item;
-      ui_out_emit_list list_emitter (uiout, field_name);
-      while (true)
-	{
-	  item.reset (PyIter_Next (result));
-	  if (item == nullptr)
-	    {
-	      if (PyErr_Occurred () != nullptr)
-		gdbpy_handle_exception ();
-	      break;
-	    }
-	  serialize_mi_result_1 (item.get (), nullptr);
-	}
-    }
-  else
-    {
-      if (PyLong_Check (result))
-	{
-	  int overflow = 0;
-	  gdb_py_longest val = gdb_py_long_as_long_and_overflow (result,
-								 &overflow);
-	  if (PyErr_Occurred () != nullptr)
-	    gdbpy_handle_exception ();
-	  if (overflow == 0)
-	    {
-	      uiout->field_signed (field_name, val);
-	      return;
-	    }
-	  /* Fall through to the string case on overflow.  */
-	}
-
-      gdb::unique_xmalloc_ptr<char> string (gdbpy_obj_to_string (result));
-      if (string == nullptr)
-	gdbpy_handle_exception ();
-      uiout->field_string (field_name, string.get ());
-    }
-}
-
-/* Serialize RESULT and print it in MI format to the current_uiout.
-
-   This function handles the top-level result initially returned from the
-   invoke method of the Python command implementation.  At the top-level
-   the result must be a dictionary.  The values within this dictionary can
-   be a wider range of types.  Handling the values of the top-level
-   dictionary is done by serialize_mi_result_1, see that function for more
-   details.
-
-   If anything goes wrong while parsing and printing the MI output then an
-   error is thrown.  */
-
-static void
-serialize_mi_result (PyObject *result)
-{
-  /* At the top-level, the result must be a dictionary.  */
-
-  if (!PyDict_Check (result))
-    gdbpy_error (_("Result from invoke must be a dictionary"));
-
-  PyObject *key, *value;
-  Py_ssize_t pos = 0;
-  while (PyDict_Next (result, &pos, &key, &value))
-    {
-      gdb::unique_xmalloc_ptr<char> key_string
-	(py_object_to_mi_key (key));
-      serialize_mi_result_1 (value, key_string.get ());
-    }
-}
-
 /* Called when the MI command is invoked.  PARSE contains the parsed
    command line arguments from the user.  */
 
@@ -381,14 +209,19 @@ mi_command_py::invoke (struct mi_parse *parse) const
 
   gdb_assert (this->m_pyobj != nullptr);
   gdb_assert (PyErr_Occurred () == nullptr);
-  gdbpy_ref<> result
+  gdbpy_ref<> results
     (PyObject_CallMethodObjArgs ((PyObject *) this->m_pyobj.get (), invoke_cst,
 				 argobj.get (), nullptr));
-  if (result == nullptr)
+  if (results == nullptr)
     gdbpy_handle_exception ();
 
-  if (result != Py_None)
-    serialize_mi_result (result.get ());
+  if (results != Py_None)
+    {
+      /* At the top-level, the results must be a dictionary.  */
+      if (!PyDict_Check (results.get ()))
+        gdbpy_error (_("Result from invoke must be a dictionary"));
+      serialize_mi_results (results.get ());
+    }
 }
 
 /* See declaration above.  */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 93217375cc5..785dc4a5743 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -486,6 +486,19 @@ struct gdbarch *arch_object_to_gdbarch (PyObject *obj);
 extern PyObject *gdbpy_execute_mi_command (PyObject *self, PyObject *args,
 					   PyObject *kw);
 
+/* Serialize RESULTS and print it in MI format to the current_uiout.
+
+   This function handles the top-level results passed as dictionary.  
+   The caller is responsible for ensuring that.  The values within this 
+   dictionary can be a wider range of types.  Handling the values of the top-level
+   dictionary is done by serialize_mi_result_1, see that function for more
+   details.
+
+   If anything goes wrong while parsing and printing the MI output then an
+   error is thrown.  */
+
+extern void serialize_mi_results (PyObject *results);
+
 /* 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
-- 
2.40.1

Re: [PATCH v2 1/2] gdb/python: generalize serialize_mi_result()

[Andrew Burgess]

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

> This commit generalizes serialize_mi_result() to make usable in
> different contexts than printing result of custom MI command.
>
> To do so, the check whether passed Python object is a dictionary has been
> moved to the caller - at the very least, different uses require different
> error messages.  Also it has been renamed to serialize_mi_results() to better
> match GDB/MI output syntax (see corresponding section in documentation,
> in particular rules 'result-record' and 'async-output'.
>
> Since it is now more generic function, it has been moved to py-mi.c.
>
> This is a preparation for implementing Python support for sending custom
> MI async events.
> ---
>  gdb/python/py-mi.c           | 159 ++++++++++++++++++++++++++++++
>  gdb/python/py-micmd.c        | 185 ++---------------------------------
>  gdb/python/python-internal.h |  13 +++
>  3 files changed, 181 insertions(+), 176 deletions(-)
>
> diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
> index 66dc6fb8a32..36bcb6ceece 100644
> --- a/gdb/python/py-mi.c
> +++ b/gdb/python/py-mi.c
> @@ -296,3 +296,162 @@ gdbpy_execute_mi_command (PyObject *self, PyObject *args, PyObject *kw)
>  
>    return uiout.result ();
>  }
> +
> +/* Convert KEY_OBJ into a string that can be used as a field name in MI
> +   output.  KEY_OBJ must be a Python string object, and must only contain
> +   characters suitable for use as an MI field name.
> +
> +   If KEY_OBJ is not a string, or if KEY_OBJ contains invalid characters,
> +   then an error is thrown.  Otherwise, KEY_OBJ is converted to a string
> +   and returned.  */
> +
> +static gdb::unique_xmalloc_ptr<char>
> +py_object_to_mi_key (PyObject *key_obj)
> +{
> +  /* The key must be a string.  */
> +  if (!PyUnicode_Check (key_obj))
> +    {
> +      gdbpy_ref<> key_repr (PyObject_Repr (key_obj));
> +      gdb::unique_xmalloc_ptr<char> key_repr_string;
> +      if (key_repr != nullptr)
> +	key_repr_string = python_string_to_target_string (key_repr.get ());
> +      if (key_repr_string == nullptr)
> +	gdbpy_handle_exception ();
> +
> +      gdbpy_error (_("non-string object used as key: %s"),
> +		   key_repr_string.get ());
> +    }
> +
> +  gdb::unique_xmalloc_ptr<char> key_string
> +    = python_string_to_target_string (key_obj);
> +  if (key_string == nullptr)
> +    gdbpy_handle_exception ();
> +
> +  /* Predicate function, returns true if NAME is a valid field name for use
> +     in MI result output, otherwise, returns false.  */
> +  auto is_valid_key_name = [] (const char *name) -> bool
> +  {
> +    gdb_assert (name != nullptr);
> +
> +    if (*name == '\0' || !isalpha (*name))
> +      return false;
> +
> +    for (; *name != '\0'; ++name)
> +      if (!isalnum (*name) && *name != '_' && *name != '-')
> +	return false;
> +
> +    return true;
> +  };
> +
> +  if (!is_valid_key_name (key_string.get ()))
> +    {
> +      if (*key_string.get () == '\0')
> +	gdbpy_error (_("Invalid empty key in MI result"));
> +      else
> +	gdbpy_error (_("Invalid key in MI result: %s"), key_string.get ());
> +    }
> +
> +  return key_string;
> +}
> +
> +/* Serialize RESULT and print it in MI format to the current_uiout.
> +   FIELD_NAME is used as the name of this result field.
> +
> +   RESULT can be a dictionary, a sequence, an iterator, or an object that
> +   can be converted to a string, these are converted to the matching MI
> +   output format (dictionaries as tuples, sequences and iterators as lists,
> +   and strings as named fields).
> +
> +   If anything goes wrong while formatting the output then an error is
> +   thrown.
> +
> +   This function is the recursive inner core of serialize_mi_result, and
> +   should only be called from that function.  */
> +
> +static void
> +serialize_mi_result_1 (PyObject *result, const char *field_name)
> +{
> +  struct ui_out *uiout = current_uiout;
> +
> +  if (PyDict_Check (result))
> +    {
> +      PyObject *key, *value;
> +      Py_ssize_t pos = 0;
> +      ui_out_emit_tuple tuple_emitter (uiout, field_name);
> +      while (PyDict_Next (result, &pos, &key, &value))
> +	{
> +	  gdb::unique_xmalloc_ptr<char> key_string
> +	    (py_object_to_mi_key (key));
> +	  serialize_mi_result_1 (value, key_string.get ());
> +	}
> +    }
> +  else if (PySequence_Check (result) && !PyUnicode_Check (result))
> +    {
> +      ui_out_emit_list list_emitter (uiout, field_name);
> +      Py_ssize_t len = PySequence_Size (result);
> +      if (len == -1)
> +	gdbpy_handle_exception ();
> +      for (Py_ssize_t i = 0; i < len; ++i)
> +	{
> +	  gdbpy_ref<> item (PySequence_ITEM (result, i));
> +	  if (item == nullptr)
> +	    gdbpy_handle_exception ();
> +	  serialize_mi_result_1 (item.get (), nullptr);
> +	}
> +    }
> +  else if (PyIter_Check (result))
> +    {
> +      gdbpy_ref<> item;
> +      ui_out_emit_list list_emitter (uiout, field_name);
> +      while (true)
> +	{
> +	  item.reset (PyIter_Next (result));
> +	  if (item == nullptr)
> +	    {
> +	      if (PyErr_Occurred () != nullptr)
> +		gdbpy_handle_exception ();
> +	      break;
> +	    }
> +	  serialize_mi_result_1 (item.get (), nullptr);
> +	}
> +    }
> +  else
> +    {
> +      if (PyLong_Check (result))
> +	{
> +	  int overflow = 0;
> +	  gdb_py_longest val = gdb_py_long_as_long_and_overflow (result,
> +								 &overflow);
> +	  if (PyErr_Occurred () != nullptr)
> +	    gdbpy_handle_exception ();
> +	  if (overflow == 0)
> +	    {
> +	      uiout->field_signed (field_name, val);
> +	      return;
> +	    }
> +	  /* Fall through to the string case on overflow.  */
> +	}
> +
> +      gdb::unique_xmalloc_ptr<char> string (gdbpy_obj_to_string (result));
> +      if (string == nullptr)
> +	gdbpy_handle_exception ();
> +      uiout->field_string (field_name, string.get ());
> +    }
> +}
> +
> +/* See python-internal.h.  */
> +
> +void
> +serialize_mi_results (PyObject *results)
> +{
> +  gdb_assert (PyDict_Check (results));
> +
> +  PyObject *key, *value;
> +  Py_ssize_t pos = 0;
> +  while (PyDict_Next (results, &pos, &key, &value))
> +    {
> +      gdb::unique_xmalloc_ptr<char> key_string
> +	(py_object_to_mi_key (key));
> +      serialize_mi_result_1 (value, key_string.get ());
> +    }
> +}
> diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
> index 01fc6060ece..dbfd44c892b 100644
> --- a/gdb/python/py-micmd.c
> +++ b/gdb/python/py-micmd.c
> @@ -173,178 +173,6 @@ extern PyTypeObject micmdpy_object_type
>  
>  static PyObject *invoke_cst;
>  
> -/* Convert KEY_OBJ into a string that can be used as a field name in MI
> -   output.  KEY_OBJ must be a Python string object, and must only contain
> -   characters suitable for use as an MI field name.
> -
> -   If KEY_OBJ is not a string, or if KEY_OBJ contains invalid characters,
> -   then an error is thrown.  Otherwise, KEY_OBJ is converted to a string
> -   and returned.  */
> -
> -static gdb::unique_xmalloc_ptr<char>
> -py_object_to_mi_key (PyObject *key_obj)
> -{
> -  /* The key must be a string.  */
> -  if (!PyUnicode_Check (key_obj))
> -    {
> -      gdbpy_ref<> key_repr (PyObject_Repr (key_obj));
> -      gdb::unique_xmalloc_ptr<char> key_repr_string;
> -      if (key_repr != nullptr)
> -	key_repr_string = python_string_to_target_string (key_repr.get ());
> -      if (key_repr_string == nullptr)
> -	gdbpy_handle_exception ();
> -
> -      gdbpy_error (_("non-string object used as key: %s"),
> -		   key_repr_string.get ());
> -    }
> -
> -  gdb::unique_xmalloc_ptr<char> key_string
> -    = python_string_to_target_string (key_obj);
> -  if (key_string == nullptr)
> -    gdbpy_handle_exception ();
> -
> -  /* Predicate function, returns true if NAME is a valid field name for use
> -     in MI result output, otherwise, returns false.  */
> -  auto is_valid_key_name = [] (const char *name) -> bool
> -  {
> -    gdb_assert (name != nullptr);
> -
> -    if (*name == '\0' || !isalpha (*name))
> -      return false;
> -
> -    for (; *name != '\0'; ++name)
> -      if (!isalnum (*name) && *name != '_' && *name != '-')
> -	return false;
> -
> -    return true;
> -  };
> -
> -  if (!is_valid_key_name (key_string.get ()))
> -    {
> -      if (*key_string.get () == '\0')
> -	gdbpy_error (_("Invalid empty key in MI result"));
> -      else
> -	gdbpy_error (_("Invalid key in MI result: %s"), key_string.get ());
> -    }
> -
> -  return key_string;
> -}
> -
> -/* Serialize RESULT and print it in MI format to the current_uiout.
> -   FIELD_NAME is used as the name of this result field.
> -
> -   RESULT can be a dictionary, a sequence, an iterator, or an object that
> -   can be converted to a string, these are converted to the matching MI
> -   output format (dictionaries as tuples, sequences and iterators as lists,
> -   and strings as named fields).
> -
> -   If anything goes wrong while formatting the output then an error is
> -   thrown.
> -
> -   This function is the recursive inner core of serialize_mi_result, and
> -   should only be called from that function.  */
> -
> -static void
> -serialize_mi_result_1 (PyObject *result, const char *field_name)
> -{
> -  struct ui_out *uiout = current_uiout;
> -
> -  if (PyDict_Check (result))
> -    {
> -      PyObject *key, *value;
> -      Py_ssize_t pos = 0;
> -      ui_out_emit_tuple tuple_emitter (uiout, field_name);
> -      while (PyDict_Next (result, &pos, &key, &value))
> -	{
> -	  gdb::unique_xmalloc_ptr<char> key_string
> -	    (py_object_to_mi_key (key));
> -	  serialize_mi_result_1 (value, key_string.get ());
> -	}
> -    }
> -  else if (PySequence_Check (result) && !PyUnicode_Check (result))
> -    {
> -      ui_out_emit_list list_emitter (uiout, field_name);
> -      Py_ssize_t len = PySequence_Size (result);
> -      if (len == -1)
> -	gdbpy_handle_exception ();
> -      for (Py_ssize_t i = 0; i < len; ++i)
> -	{
> -	  gdbpy_ref<> item (PySequence_ITEM (result, i));
> -	  if (item == nullptr)
> -	    gdbpy_handle_exception ();
> -	  serialize_mi_result_1 (item.get (), nullptr);
> -	}
> -    }
> -  else if (PyIter_Check (result))
> -    {
> -      gdbpy_ref<> item;
> -      ui_out_emit_list list_emitter (uiout, field_name);
> -      while (true)
> -	{
> -	  item.reset (PyIter_Next (result));
> -	  if (item == nullptr)
> -	    {
> -	      if (PyErr_Occurred () != nullptr)
> -		gdbpy_handle_exception ();
> -	      break;
> -	    }
> -	  serialize_mi_result_1 (item.get (), nullptr);
> -	}
> -    }
> -  else
> -    {
> -      if (PyLong_Check (result))
> -	{
> -	  int overflow = 0;
> -	  gdb_py_longest val = gdb_py_long_as_long_and_overflow (result,
> -								 &overflow);
> -	  if (PyErr_Occurred () != nullptr)
> -	    gdbpy_handle_exception ();
> -	  if (overflow == 0)
> -	    {
> -	      uiout->field_signed (field_name, val);
> -	      return;
> -	    }
> -	  /* Fall through to the string case on overflow.  */
> -	}
> -
> -      gdb::unique_xmalloc_ptr<char> string (gdbpy_obj_to_string (result));
> -      if (string == nullptr)
> -	gdbpy_handle_exception ();
> -      uiout->field_string (field_name, string.get ());
> -    }
> -}
> -
> -/* Serialize RESULT and print it in MI format to the current_uiout.
> -
> -   This function handles the top-level result initially returned from the
> -   invoke method of the Python command implementation.  At the top-level
> -   the result must be a dictionary.  The values within this dictionary can
> -   be a wider range of types.  Handling the values of the top-level
> -   dictionary is done by serialize_mi_result_1, see that function for more
> -   details.
> -
> -   If anything goes wrong while parsing and printing the MI output then an
> -   error is thrown.  */
> -
> -static void
> -serialize_mi_result (PyObject *result)
> -{
> -  /* At the top-level, the result must be a dictionary.  */
> -
> -  if (!PyDict_Check (result))
> -    gdbpy_error (_("Result from invoke must be a dictionary"));
> -
> -  PyObject *key, *value;
> -  Py_ssize_t pos = 0;
> -  while (PyDict_Next (result, &pos, &key, &value))
> -    {
> -      gdb::unique_xmalloc_ptr<char> key_string
> -	(py_object_to_mi_key (key));
> -      serialize_mi_result_1 (value, key_string.get ());
> -    }
> -}
> -
>  /* Called when the MI command is invoked.  PARSE contains the parsed
>     command line arguments from the user.  */
>  
> @@ -381,14 +209,19 @@ mi_command_py::invoke (struct mi_parse *parse) const
>  
>    gdb_assert (this->m_pyobj != nullptr);
>    gdb_assert (PyErr_Occurred () == nullptr);
> -  gdbpy_ref<> result
> +  gdbpy_ref<> results
>      (PyObject_CallMethodObjArgs ((PyObject *) this->m_pyobj.get (), invoke_cst,
>  				 argobj.get (), nullptr));
> -  if (result == nullptr)
> +  if (results == nullptr)
>      gdbpy_handle_exception ();
>  
> -  if (result != Py_None)
> -    serialize_mi_result (result.get ());
> +  if (results != Py_None)
> +    {
> +      /* At the top-level, the results must be a dictionary.  */
> +      if (!PyDict_Check (results.get ()))
> +        gdbpy_error (_("Result from invoke must be a dictionary"));

This line should be indented with a single tab, not spaces.

> +      serialize_mi_results (results.get ());
> +    }
>  }
>  
>  /* See declaration above.  */
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index 93217375cc5..785dc4a5743 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -486,6 +486,19 @@ struct gdbarch *arch_object_to_gdbarch (PyObject *obj);
>  extern PyObject *gdbpy_execute_mi_command (PyObject *self, PyObject *args,
>  					   PyObject *kw);
>  
> +/* Serialize RESULTS and print it in MI format to the current_uiout.
> +
> +   This function handles the top-level results passed as dictionary.  

... passed as a dictionary.

Also, you have some trailing white space after 'dictionary.'.

> +   The caller is responsible for ensuring that.  The values within this 

Again, you have some trailing white space at the end of this line.

With these nits fixed:

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


> +   dictionary can be a wider range of types.  Handling the values of the top-level
> +   dictionary is done by serialize_mi_result_1, see that function for more
> +   details.
> +
> +   If anything goes wrong while parsing and printing the MI output then an
> +   error is thrown.  */
> +
> +extern void serialize_mi_results (PyObject *results);
> +
>  /* 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
> -- 
> 2.40.1

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

[Jan Vrany]

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                       | 45 ++++++++++++++
 gdb/python/py-mi.c                        | 71 +++++++++++++++++++++++
 gdb/python/python-internal.h              |  5 ++
 gdb/python/python.c                       |  4 ++
 gdb/testsuite/gdb.python/py-mi-notify.exp | 71 +++++++++++++++++++++++
 6 files changed, 199 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 5b13958aeaf..7764101e521 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -211,6 +211,7 @@ optional arguments while skipping others.  Example:
 * Recordings In Python::        Accessing recordings from Python.
 * CLI Commands In Python::      Implementing new CLI commands in Python.
 * GDB/MI Commands In Python::   Implementing new @sc{gdb/mi} commands in Python.
+* GDB/MI Notifications In Python:: Implementing new @sc{gdb/mi} notifications in Python.
 * Parameters In Python::        Adding new @value{GDBN} parameters.
 * Functions In Python::         Writing new convenience functions.
 * Progspaces In Python::        Program spaces.
@@ -4706,6 +4707,50 @@ 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.  Use the @code{gdb.notify_mi} function to do that.
+
+@defun gdb.notify_mi (name @r{[}, data@r{]})
+Emit a @sc{gdb/mi} asynchronous notification.  @var{name} is the name of the
+notification, consisting of alphanumeric characters and a hyphen (@code{-}).
+@var{data} is any additional data to be emitted with the notification, passed
+as a Python dictionary. This argument is optional. The dictionary is converted
+to a @sc{gdb/mi} @var{result} records (@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
+
+While using existing notification names (@pxref{GDB/MI Async Records}) with
+@code{gdb.notify_mi} is allowed, users are encouraged to prefix user-defined
+notification with a hyphen (@code{-}) to avoid possible conflict. @value{GDBN}
+will never introduce notification starting with hyphen.
+
+Here is how to emit @code{=-connection-removed} whenever a connection to remote
+GDB server is closed (@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 36bcb6ceece..77479e31f30 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.  */
@@ -455,3 +461,68 @@ serialize_mi_results (PyObject *results)
       serialize_mi_result_1 (value, key_string.get ());
     }
 }
+
+/* See python-internal.h.  */
+
+PyObject *
+gdbpy_notify_mi (PyObject *self, PyObject *args, PyObject *kwargs)
+{
+  static const char *keywords[] = { "name", "data", nullptr };
+  char *name = nullptr;
+  PyObject *data = Py_None;
+
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "s|O", keywords,
+					&name, &data))
+    return nullptr;
+
+  /* Validate notification name */
+  const int name_len = strlen (name);
+  if (name_len == 0)
+    {
+      PyErr_SetString (PyExc_ValueError, _("MI notification name is empty."));
+      return nullptr;
+    }
+  for (int i = 0; i < name_len; i++)
+    {
+      if (!isalnum (name[i]) && name[i] != '-')
+	{
+	  PyErr_Format (PyExc_ValueError,
+			_("MI notification name contains invalid character: %c."),
+			name[i]);
+	  return nullptr;
+	}
+    }
+
+  /* Validate additional data */
+  if (! (data == Py_None || PyDict_Check (data)))
+    {
+      PyErr_SetString (PyExc_ValueError,
+		       _("MI notification data must be either None or a dictionary"));
+      return nullptr;
+    }
+
+  SWITCH_THRU_ALL_UIS ()
+    {
+      struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
+
+      if (mi == nullptr)
+	continue;
+
+      gdb_printf (mi->event_channel, "%s", name);
+      if (data != Py_None)
+	{
+	  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_results (data);
+	}
+      gdb_flush (mi->event_channel);
+    }
+
+  Py_RETURN_NONE;
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 785dc4a5743..c7ac6817bd3 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -499,6 +499,11 @@ extern PyObject *gdbpy_execute_mi_command (PyObject *self, PyObject *args,
 
 extern void serialize_mi_results (PyObject *results);
 
+/* Implementation of the gdb.notify_mi function.  */
+
+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..7c99f7c93b5
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-mi-notify.exp
@@ -0,0 +1,71 @@
+# Copyright (C) 2023-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')" \
+    ".*=-test-notification\r\n\\^done" \
+    "python notification, no additional data parameter"
+
+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"
+
+mi_gdb_test "python gdb.notify_mi('', 1)" \
+    ".*\\^error,msg=\".*\"" \
+    "python notification, empty notification name"
+
+mi_gdb_test "python gdb.notify_mi('**invalid**', 1)" \
+    ".*\\^error,msg=\".*\"" \
+    "python notification, invalid notification name"
+
+mi_gdb_test "python gdb.notify_mi(\[1,2,3\], 1)" \
+    ".*\\^error,msg=\".*\"" \
+    "python notification, non-string notification name"
+
+mi_gdb_test "python gdb.notify_mi()" \
+    ".*\\^error,msg=\".*\"" \
+    "python notification, no arguments passed"
+
+mi_gdb_test "python gdb.notify_mi('thread-group-added', \{'id' : 'i2'\})" \
+    ".*=thread-group-added,id=\"i2\"\r\n\\^done" \
+    "python notification, using existing internal notification name"
-- 
2.40.1

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

[Eli Zaretskii]

> CC: Jan Vrany <jan.vrany@labware.com>
> Date: Wed, 13 Sep 2023 15:38:43 +0100
> From: Jan Vrany via Gdb-patches <gdb-patches@sourceware.org>
> 
> 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                       | 45 ++++++++++++++
>  gdb/python/py-mi.c                        | 71 +++++++++++++++++++++++
>  gdb/python/python-internal.h              |  5 ++
>  gdb/python/python.c                       |  4 ++
>  gdb/testsuite/gdb.python/py-mi-notify.exp | 71 +++++++++++++++++++++++
>  6 files changed, 199 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.python/py-mi-notify.exp

Thanks.

> 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

This part is OK.

> +While using existing notification names (@pxref{GDB/MI Async Records}) with
> +@code{gdb.notify_mi} is allowed, users are encouraged to prefix user-defined
> +notification with a hyphen (@code{-}) to avoid possible conflict. @value{GDBN}
                                                                   ^^
Likewise here.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

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

[Andrew Burgess]

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.

Thanks for working on this.  I have a couple of minor formatting nits,
but also a couple of small changes that I think are needed.  But this
looks super close now.  See the details inline.


> ---
>  gdb/NEWS                                  |  3 +
>  gdb/doc/python.texi                       | 45 ++++++++++++++
>  gdb/python/py-mi.c                        | 71 +++++++++++++++++++++++
>  gdb/python/python-internal.h              |  5 ++
>  gdb/python/python.c                       |  4 ++
>  gdb/testsuite/gdb.python/py-mi-notify.exp | 71 +++++++++++++++++++++++
>  6 files changed, 199 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 5b13958aeaf..7764101e521 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -211,6 +211,7 @@ optional arguments while skipping others.  Example:
>  * Recordings In Python::        Accessing recordings from Python.
>  * CLI Commands In Python::      Implementing new CLI commands in Python.
>  * GDB/MI Commands In Python::   Implementing new @sc{gdb/mi} commands in Python.
> +* GDB/MI Notifications In Python:: Implementing new @sc{gdb/mi} notifications in Python.
>  * Parameters In Python::        Adding new @value{GDBN} parameters.
>  * Functions In Python::         Writing new convenience functions.
>  * Progspaces In Python::        Program spaces.
> @@ -4706,6 +4707,50 @@ 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.  Use the @code{gdb.notify_mi} function to do that.
> +
> +@defun gdb.notify_mi (name @r{[}, data@r{]})
> +Emit a @sc{gdb/mi} asynchronous notification.  @var{name} is the name of the
> +notification, consisting of alphanumeric characters and a hyphen (@code{-}).
> +@var{data} is any additional data to be emitted with the notification, passed
> +as a Python dictionary. This argument is optional. The dictionary is converted
> +to a @sc{gdb/mi} @var{result} records (@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
> +
> +While using existing notification names (@pxref{GDB/MI Async Records}) with
> +@code{gdb.notify_mi} is allowed, users are encouraged to prefix user-defined
> +notification with a hyphen (@code{-}) to avoid possible conflict. @value{GDBN}
> +will never introduce notification starting with hyphen.
> +
> +Here is how to emit @code{=-connection-removed} whenever a connection to remote
> +GDB server is closed (@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 36bcb6ceece..77479e31f30 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.  */
> @@ -455,3 +461,68 @@ serialize_mi_results (PyObject *results)
>        serialize_mi_result_1 (value, key_string.get ());
>      }
>  }
> +
> +/* See python-internal.h.  */
> +
> +PyObject *
> +gdbpy_notify_mi (PyObject *self, PyObject *args, PyObject *kwargs)
> +{
> +  static const char *keywords[] = { "name", "data", nullptr };
> +  char *name = nullptr;
> +  PyObject *data = Py_None;
> +
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "s|O", keywords,
> +					&name, &data))
> +    return nullptr;
> +
> +  /* Validate notification name */

/* Validate notification name.  */

> +  const int name_len = strlen (name);
> +  if (name_len == 0)
> +    {
> +      PyErr_SetString (PyExc_ValueError, _("MI notification name is empty."));
> +      return nullptr;
> +    }
> +  for (int i = 0; i < name_len; i++)
> +    {
> +      if (!isalnum (name[i]) && name[i] != '-')
> +	{
> +	  PyErr_Format (PyExc_ValueError,
> +			_("MI notification name contains invalid character: %c."),
> +			name[i]);

The line containing the string is too long (82 characters), if you
reformat like this:

	  PyErr_Format
	    (PyExc_ValueError,
	     _("MI notification name contains invalid character: %c."),
	     name[i]);

Then you should be good.


> +	  return nullptr;
> +	}
> +    }
> +
> +  /* Validate additional data */

/* Validate additional data.  */

> +  if (! (data == Py_None || PyDict_Check (data)))

No space after '!'.

> +    {
> +      PyErr_SetString (PyExc_ValueError,
> +		       _("MI notification data must be either None or a dictionary"));

Again, the error string line is too long.  Also, I'd be tempted to
include the incorrect type name, like this:

      PyErr_Format
	(PyExc_ValueError,
	 _("MI notification data must be either None or a dictionary, not %s"),
	 Py_TYPE (data)->tp_name);

> +      return nullptr;
> +    }
> +
> +  SWITCH_THRU_ALL_UIS ()
> +    {
> +      struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
> +
> +      if (mi == nullptr)
> +	continue;
> +
> +      gdb_printf (mi->event_channel, "%s", name);
> +      if (data != Py_None)
> +	{
> +	  target_terminal::scoped_restore_terminal_state term_state;
> +	  target_terminal::ours_for_output ();

I was surprised to see these here, when we've already written out some
output.  Should these not appear earlier, after the 'mi == nullptr'
check?
> +
> +	  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_results (data);
> +	}
> +      gdb_flush (mi->event_channel);
> +    }
> +
> +  Py_RETURN_NONE;
> +}
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index 785dc4a5743..c7ac6817bd3 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -499,6 +499,11 @@ extern PyObject *gdbpy_execute_mi_command (PyObject *self, PyObject *args,
>  
>  extern void serialize_mi_results (PyObject *results);
>  
> +/* Implementation of the gdb.notify_mi function.  */
> +
> +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..7c99f7c93b5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-mi-notify.exp
> @@ -0,0 +1,71 @@
> +# Copyright (C) 2023-2023 Free Software Foundation, Inc.

Just '2023' here.

> +# 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.

You should add a test for the error cases:

  - MI notification name is empty.

  - MI notification name contains invalid character: %c.

  - MI notification data must be either None or a dictionary

I know for the last two you can't test _every_ possible wrong input, but
you can send at least one, just to check that the error works.

Thanks,
Andrew

> +
> +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')" \
> +    ".*=-test-notification\r\n\\^done" \
> +    "python notification, no additional data parameter"
> +
> +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"
> +
> +mi_gdb_test "python gdb.notify_mi('', 1)" \
> +    ".*\\^error,msg=\".*\"" \
> +    "python notification, empty notification name"
> +
> +mi_gdb_test "python gdb.notify_mi('**invalid**', 1)" \
> +    ".*\\^error,msg=\".*\"" \
> +    "python notification, invalid notification name"
> +
> +mi_gdb_test "python gdb.notify_mi(\[1,2,3\], 1)" \
> +    ".*\\^error,msg=\".*\"" \
> +    "python notification, non-string notification name"
> +
> +mi_gdb_test "python gdb.notify_mi()" \
> +    ".*\\^error,msg=\".*\"" \
> +    "python notification, no arguments passed"
> +
> +mi_gdb_test "python gdb.notify_mi('thread-group-added', \{'id' : 'i2'\})" \
> +    ".*=thread-group-added,id=\"i2\"\r\n\\^done" \
> +    "python notification, using existing internal notification name"
> -- 
> 2.40.1

[PATCH v3 0/2] gdb/python: implement support for sending custom MI async

[Jan Vrany]

I hope I addressed all comments raised by Andrew for V2 series

Changes since V2:

* Fixed formatting errors in patch 1 of this series
* Fixed formatting errors in patch 2 of this series
* Added report name of incorrect data type into error message
  as suggested.
* Moved target_terminal::ours_for_output () right after the
  check if UI is an MI UI as suggested.
* Fixed tests.

Changes since V1:

* Moved serialize_mi_result and helpers to py-mi.c as suggested.
  Also, renamed serialize_mi_result to serialize_mi_results
  instead of serialize_mi_data and do not rename serialize_mi_result_1.

  The reason for this is that GDB documentation,
  section GDB/MI Output Syntax, describes "result" being single 'variable = value'
  pair and "result-record" and "async-output" contains zero or more of these "result"s,
  so calling top-level serialization function serialize_mi_results seems
  a better name than serialize_mi_data (used in previous version).

* Made gdb.notify_mi DATA parameter optional as suggested.

* Validate gdb.notify_mi NAME parameter as suggested.

* Updated documentation:
  * example formatted as black tool would do it,
  * rephrase some sentences as suggested,
  * document that DATA parameter is optional,
  * document restrictions on NAME parameter and
  * document that users should prefix user-defined notification
    with hyphen to avoid possible conflicts.

Jan Vrany (2):
  gdb/python: generalize serialize_mi_result()
  gdb/python: implement support for sending custom MI async
    notifications

 gdb/NEWS                                  |   3 +
 gdb/doc/python.texi                       |  45 +++++
 gdb/python/py-mi.c                        | 233 ++++++++++++++++++++++
 gdb/python/py-micmd.c                     | 185 +----------------
 gdb/python/python-internal.h              |  18 ++
 gdb/python/python.c                       |   4 +
 gdb/testsuite/gdb.python/py-mi-notify.exp |  71 +++++++
 7 files changed, 383 insertions(+), 176 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-mi-notify.exp

-- 
2.40.1

[PATCH v3 1/2] gdb/python: generalize serialize_mi_result()

[Jan Vrany]

This commit generalizes serialize_mi_result() to make usable in
different contexts than printing result of custom MI command.

To do so, the check whether passed Python object is a dictionary has been
moved to the caller - at the very least, different uses require different
error messages.  Also it has been renamed to serialize_mi_results() to better
match GDB/MI output syntax (see corresponding section in documentation,
in particular rules 'result-record' and 'async-output'.

Since it is now more generic function, it has been moved to py-mi.c.

This is a preparation for implementing Python support for sending custom
MI async events.

Approved-By: Andrew Burgess <aburgess@redhat.com>
---
 gdb/python/py-mi.c           | 159 ++++++++++++++++++++++++++++++
 gdb/python/py-micmd.c        | 185 ++---------------------------------
 gdb/python/python-internal.h |  13 +++
 3 files changed, 181 insertions(+), 176 deletions(-)

diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
index 66dc6fb8a32..36bcb6ceece 100644
--- a/gdb/python/py-mi.c
+++ b/gdb/python/py-mi.c
@@ -296,3 +296,162 @@ gdbpy_execute_mi_command (PyObject *self, PyObject *args, PyObject *kw)
 
   return uiout.result ();
 }
+
+/* Convert KEY_OBJ into a string that can be used as a field name in MI
+   output.  KEY_OBJ must be a Python string object, and must only contain
+   characters suitable for use as an MI field name.
+
+   If KEY_OBJ is not a string, or if KEY_OBJ contains invalid characters,
+   then an error is thrown.  Otherwise, KEY_OBJ is converted to a string
+   and returned.  */
+
+static gdb::unique_xmalloc_ptr<char>
+py_object_to_mi_key (PyObject *key_obj)
+{
+  /* The key must be a string.  */
+  if (!PyUnicode_Check (key_obj))
+    {
+      gdbpy_ref<> key_repr (PyObject_Repr (key_obj));
+      gdb::unique_xmalloc_ptr<char> key_repr_string;
+      if (key_repr != nullptr)
+	key_repr_string = python_string_to_target_string (key_repr.get ());
+      if (key_repr_string == nullptr)
+	gdbpy_handle_exception ();
+
+      gdbpy_error (_("non-string object used as key: %s"),
+		   key_repr_string.get ());
+    }
+
+  gdb::unique_xmalloc_ptr<char> key_string
+    = python_string_to_target_string (key_obj);
+  if (key_string == nullptr)
+    gdbpy_handle_exception ();
+
+  /* Predicate function, returns true if NAME is a valid field name for use
+     in MI result output, otherwise, returns false.  */
+  auto is_valid_key_name = [] (const char *name) -> bool
+  {
+    gdb_assert (name != nullptr);
+
+    if (*name == '\0' || !isalpha (*name))
+      return false;
+
+    for (; *name != '\0'; ++name)
+      if (!isalnum (*name) && *name != '_' && *name != '-')
+	return false;
+
+    return true;
+  };
+
+  if (!is_valid_key_name (key_string.get ()))
+    {
+      if (*key_string.get () == '\0')
+	gdbpy_error (_("Invalid empty key in MI result"));
+      else
+	gdbpy_error (_("Invalid key in MI result: %s"), key_string.get ());
+    }
+
+  return key_string;
+}
+
+/* Serialize RESULT and print it in MI format to the current_uiout.
+   FIELD_NAME is used as the name of this result field.
+
+   RESULT can be a dictionary, a sequence, an iterator, or an object that
+   can be converted to a string, these are converted to the matching MI
+   output format (dictionaries as tuples, sequences and iterators as lists,
+   and strings as named fields).
+
+   If anything goes wrong while formatting the output then an error is
+   thrown.
+
+   This function is the recursive inner core of serialize_mi_result, and
+   should only be called from that function.  */
+
+static void
+serialize_mi_result_1 (PyObject *result, const char *field_name)
+{
+  struct ui_out *uiout = current_uiout;
+
+  if (PyDict_Check (result))
+    {
+      PyObject *key, *value;
+      Py_ssize_t pos = 0;
+      ui_out_emit_tuple tuple_emitter (uiout, field_name);
+      while (PyDict_Next (result, &pos, &key, &value))
+	{
+	  gdb::unique_xmalloc_ptr<char> key_string
+	    (py_object_to_mi_key (key));
+	  serialize_mi_result_1 (value, key_string.get ());
+	}
+    }
+  else if (PySequence_Check (result) && !PyUnicode_Check (result))
+    {
+      ui_out_emit_list list_emitter (uiout, field_name);
+      Py_ssize_t len = PySequence_Size (result);
+      if (len == -1)
+	gdbpy_handle_exception ();
+      for (Py_ssize_t i = 0; i < len; ++i)
+	{
+	  gdbpy_ref<> item (PySequence_ITEM (result, i));
+	  if (item == nullptr)
+	    gdbpy_handle_exception ();
+	  serialize_mi_result_1 (item.get (), nullptr);
+	}
+    }
+  else if (PyIter_Check (result))
+    {
+      gdbpy_ref<> item;
+      ui_out_emit_list list_emitter (uiout, field_name);
+      while (true)
+	{
+	  item.reset (PyIter_Next (result));
+	  if (item == nullptr)
+	    {
+	      if (PyErr_Occurred () != nullptr)
+		gdbpy_handle_exception ();
+	      break;
+	    }
+	  serialize_mi_result_1 (item.get (), nullptr);
+	}
+    }
+  else
+    {
+      if (PyLong_Check (result))
+	{
+	  int overflow = 0;
+	  gdb_py_longest val = gdb_py_long_as_long_and_overflow (result,
+								 &overflow);
+	  if (PyErr_Occurred () != nullptr)
+	    gdbpy_handle_exception ();
+	  if (overflow == 0)
+	    {
+	      uiout->field_signed (field_name, val);
+	      return;
+	    }
+	  /* Fall through to the string case on overflow.  */
+	}
+
+      gdb::unique_xmalloc_ptr<char> string (gdbpy_obj_to_string (result));
+      if (string == nullptr)
+	gdbpy_handle_exception ();
+      uiout->field_string (field_name, string.get ());
+    }
+}
+
+/* See python-internal.h.  */
+
+void
+serialize_mi_results (PyObject *results)
+{
+  gdb_assert (PyDict_Check (results));
+
+  PyObject *key, *value;
+  Py_ssize_t pos = 0;
+  while (PyDict_Next (results, &pos, &key, &value))
+    {
+      gdb::unique_xmalloc_ptr<char> key_string
+	(py_object_to_mi_key (key));
+      serialize_mi_result_1 (value, key_string.get ());
+    }
+}
diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
index 01fc6060ece..0153f84e7a0 100644
--- a/gdb/python/py-micmd.c
+++ b/gdb/python/py-micmd.c
@@ -173,178 +173,6 @@ extern PyTypeObject micmdpy_object_type
 
 static PyObject *invoke_cst;
 
-/* Convert KEY_OBJ into a string that can be used as a field name in MI
-   output.  KEY_OBJ must be a Python string object, and must only contain
-   characters suitable for use as an MI field name.
-
-   If KEY_OBJ is not a string, or if KEY_OBJ contains invalid characters,
-   then an error is thrown.  Otherwise, KEY_OBJ is converted to a string
-   and returned.  */
-
-static gdb::unique_xmalloc_ptr<char>
-py_object_to_mi_key (PyObject *key_obj)
-{
-  /* The key must be a string.  */
-  if (!PyUnicode_Check (key_obj))
-    {
-      gdbpy_ref<> key_repr (PyObject_Repr (key_obj));
-      gdb::unique_xmalloc_ptr<char> key_repr_string;
-      if (key_repr != nullptr)
-	key_repr_string = python_string_to_target_string (key_repr.get ());
-      if (key_repr_string == nullptr)
-	gdbpy_handle_exception ();
-
-      gdbpy_error (_("non-string object used as key: %s"),
-		   key_repr_string.get ());
-    }
-
-  gdb::unique_xmalloc_ptr<char> key_string
-    = python_string_to_target_string (key_obj);
-  if (key_string == nullptr)
-    gdbpy_handle_exception ();
-
-  /* Predicate function, returns true if NAME is a valid field name for use
-     in MI result output, otherwise, returns false.  */
-  auto is_valid_key_name = [] (const char *name) -> bool
-  {
-    gdb_assert (name != nullptr);
-
-    if (*name == '\0' || !isalpha (*name))
-      return false;
-
-    for (; *name != '\0'; ++name)
-      if (!isalnum (*name) && *name != '_' && *name != '-')
-	return false;
-
-    return true;
-  };
-
-  if (!is_valid_key_name (key_string.get ()))
-    {
-      if (*key_string.get () == '\0')
-	gdbpy_error (_("Invalid empty key in MI result"));
-      else
-	gdbpy_error (_("Invalid key in MI result: %s"), key_string.get ());
-    }
-
-  return key_string;
-}
-
-/* Serialize RESULT and print it in MI format to the current_uiout.
-   FIELD_NAME is used as the name of this result field.
-
-   RESULT can be a dictionary, a sequence, an iterator, or an object that
-   can be converted to a string, these are converted to the matching MI
-   output format (dictionaries as tuples, sequences and iterators as lists,
-   and strings as named fields).
-
-   If anything goes wrong while formatting the output then an error is
-   thrown.
-
-   This function is the recursive inner core of serialize_mi_result, and
-   should only be called from that function.  */
-
-static void
-serialize_mi_result_1 (PyObject *result, const char *field_name)
-{
-  struct ui_out *uiout = current_uiout;
-
-  if (PyDict_Check (result))
-    {
-      PyObject *key, *value;
-      Py_ssize_t pos = 0;
-      ui_out_emit_tuple tuple_emitter (uiout, field_name);
-      while (PyDict_Next (result, &pos, &key, &value))
-	{
-	  gdb::unique_xmalloc_ptr<char> key_string
-	    (py_object_to_mi_key (key));
-	  serialize_mi_result_1 (value, key_string.get ());
-	}
-    }
-  else if (PySequence_Check (result) && !PyUnicode_Check (result))
-    {
-      ui_out_emit_list list_emitter (uiout, field_name);
-      Py_ssize_t len = PySequence_Size (result);
-      if (len == -1)
-	gdbpy_handle_exception ();
-      for (Py_ssize_t i = 0; i < len; ++i)
-	{
-	  gdbpy_ref<> item (PySequence_ITEM (result, i));
-	  if (item == nullptr)
-	    gdbpy_handle_exception ();
-	  serialize_mi_result_1 (item.get (), nullptr);
-	}
-    }
-  else if (PyIter_Check (result))
-    {
-      gdbpy_ref<> item;
-      ui_out_emit_list list_emitter (uiout, field_name);
-      while (true)
-	{
-	  item.reset (PyIter_Next (result));
-	  if (item == nullptr)
-	    {
-	      if (PyErr_Occurred () != nullptr)
-		gdbpy_handle_exception ();
-	      break;
-	    }
-	  serialize_mi_result_1 (item.get (), nullptr);
-	}
-    }
-  else
-    {
-      if (PyLong_Check (result))
-	{
-	  int overflow = 0;
-	  gdb_py_longest val = gdb_py_long_as_long_and_overflow (result,
-								 &overflow);
-	  if (PyErr_Occurred () != nullptr)
-	    gdbpy_handle_exception ();
-	  if (overflow == 0)
-	    {
-	      uiout->field_signed (field_name, val);
-	      return;
-	    }
-	  /* Fall through to the string case on overflow.  */
-	}
-
-      gdb::unique_xmalloc_ptr<char> string (gdbpy_obj_to_string (result));
-      if (string == nullptr)
-	gdbpy_handle_exception ();
-      uiout->field_string (field_name, string.get ());
-    }
-}
-
-/* Serialize RESULT and print it in MI format to the current_uiout.
-
-   This function handles the top-level result initially returned from the
-   invoke method of the Python command implementation.  At the top-level
-   the result must be a dictionary.  The values within this dictionary can
-   be a wider range of types.  Handling the values of the top-level
-   dictionary is done by serialize_mi_result_1, see that function for more
-   details.
-
-   If anything goes wrong while parsing and printing the MI output then an
-   error is thrown.  */
-
-static void
-serialize_mi_result (PyObject *result)
-{
-  /* At the top-level, the result must be a dictionary.  */
-
-  if (!PyDict_Check (result))
-    gdbpy_error (_("Result from invoke must be a dictionary"));
-
-  PyObject *key, *value;
-  Py_ssize_t pos = 0;
-  while (PyDict_Next (result, &pos, &key, &value))
-    {
-      gdb::unique_xmalloc_ptr<char> key_string
-	(py_object_to_mi_key (key));
-      serialize_mi_result_1 (value, key_string.get ());
-    }
-}
-
 /* Called when the MI command is invoked.  PARSE contains the parsed
    command line arguments from the user.  */
 
@@ -381,14 +209,19 @@ mi_command_py::invoke (struct mi_parse *parse) const
 
   gdb_assert (this->m_pyobj != nullptr);
   gdb_assert (PyErr_Occurred () == nullptr);
-  gdbpy_ref<> result
+  gdbpy_ref<> results
     (PyObject_CallMethodObjArgs ((PyObject *) this->m_pyobj.get (), invoke_cst,
 				 argobj.get (), nullptr));
-  if (result == nullptr)
+  if (results == nullptr)
     gdbpy_handle_exception ();
 
-  if (result != Py_None)
-    serialize_mi_result (result.get ());
+  if (results != Py_None)
+    {
+      /* At the top-level, the results must be a dictionary.  */
+      if (!PyDict_Check (results.get ()))
+	gdbpy_error (_("Result from invoke must be a dictionary"));
+      serialize_mi_results (results.get ());
+    }
 }
 
 /* See declaration above.  */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 93217375cc5..60b795ff98c 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -486,6 +486,19 @@ struct gdbarch *arch_object_to_gdbarch (PyObject *obj);
 extern PyObject *gdbpy_execute_mi_command (PyObject *self, PyObject *args,
 					   PyObject *kw);
 
+/* Serialize RESULTS and print it in MI format to the current_uiout.
+
+   This function handles the top-level results passed as a dictionary.
+   The caller is responsible for ensuring that.  The values within this
+   dictionary can be a wider range of types.  Handling the values of the top-level
+   dictionary is done by serialize_mi_result_1, see that function for more
+   details.
+
+   If anything goes wrong while parsing and printing the MI output then an
+   error is thrown.  */
+
+extern void serialize_mi_results (PyObject *results);
+
 /* 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
-- 
2.40.1

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

[Jan Vrany]

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.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/NEWS                                  |  3 +
 gdb/doc/python.texi                       | 45 ++++++++++++++
 gdb/python/py-mi.c                        | 74 +++++++++++++++++++++++
 gdb/python/python-internal.h              |  5 ++
 gdb/python/python.c                       |  4 ++
 gdb/testsuite/gdb.python/py-mi-notify.exp | 71 ++++++++++++++++++++++
 6 files changed, 202 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/py-mi-notify.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 2f6378f9c7a..c99734ba502 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -348,6 +348,9 @@ show tui mouse-events
      gdb.Progspace that is either being added to GDB, or removed from
      GDB.
 
+  ** 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 a97e4451897..546b4d4b962 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -211,6 +211,7 @@ optional arguments while skipping others.  Example:
 * Recordings In Python::        Accessing recordings from Python.
 * CLI Commands In Python::      Implementing new CLI commands in Python.
 * GDB/MI Commands In Python::   Implementing new @sc{gdb/mi} commands in Python.
+* GDB/MI Notifications In Python:: Implementing new @sc{gdb/mi} notifications in Python.
 * Parameters In Python::        Adding new @value{GDBN} parameters.
 * Functions In Python::         Writing new convenience functions.
 * Progspaces In Python::        Program spaces.
@@ -4804,6 +4805,50 @@ 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.  Use the @code{gdb.notify_mi} function to do that.
+
+@defun gdb.notify_mi (name @r{[}, data@r{]})
+Emit a @sc{gdb/mi} asynchronous notification.  @var{name} is the name of the
+notification, consisting of alphanumeric characters and a hyphen (@code{-}).
+@var{data} is any additional data to be emitted with the notification, passed
+as a Python dictionary. This argument is optional. The dictionary is converted
+to a @sc{gdb/mi} @var{result} records (@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
+
+While using existing notification names (@pxref{GDB/MI Async Records}) with
+@code{gdb.notify_mi} is allowed, users are encouraged to prefix user-defined
+notification with a hyphen (@code{-}) to avoid possible conflict.
+@value{GDBN} will never introduce notification starting with hyphen.
+
+Here is how to emit @code{=-connection-removed} whenever a connection to remote
+GDB server is closed (@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 36bcb6ceece..a7b4f4fa3cf 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.  */
@@ -455,3 +461,71 @@ serialize_mi_results (PyObject *results)
       serialize_mi_result_1 (value, key_string.get ());
     }
 }
+
+/* See python-internal.h.  */
+
+PyObject *
+gdbpy_notify_mi (PyObject *self, PyObject *args, PyObject *kwargs)
+{
+  static const char *keywords[] = { "name", "data", nullptr };
+  char *name = nullptr;
+  PyObject *data = Py_None;
+
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "s|O", keywords,
+					&name, &data))
+    return nullptr;
+
+  /* Validate notification name.  */
+  const int name_len = strlen (name);
+  if (name_len == 0)
+    {
+      PyErr_SetString (PyExc_ValueError, _("MI notification name is empty."));
+      return nullptr;
+    }
+  for (int i = 0; i < name_len; i++)
+    {
+      if (!isalnum (name[i]) && name[i] != '-')
+	{
+	  PyErr_Format
+	    (PyExc_ValueError,
+	     _("MI notification name contains invalid character: %c."),
+	     name[i]);
+	  return nullptr;
+	}
+    }
+
+  /* Validate additional data.  */
+  if (!(data == Py_None || PyDict_Check (data)))
+    {
+      PyErr_Format
+	(PyExc_ValueError,
+	 _("MI notification data must be either None or a dictionary, not %s"),
+	 Py_TYPE (data)->tp_name);
+      return nullptr;
+    }
+
+  SWITCH_THRU_ALL_UIS ()
+    {
+      struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
+
+      if (mi == nullptr)
+	continue;
+
+      target_terminal::scoped_restore_terminal_state term_state;
+      target_terminal::ours_for_output ();
+
+      gdb_printf (mi->event_channel, "%s", name);
+      if (data != Py_None)
+	{
+	  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_results (data);
+	}
+      gdb_flush (mi->event_channel);
+    }
+
+  Py_RETURN_NONE;
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 60b795ff98c..847bed84dfe 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -499,6 +499,11 @@ extern PyObject *gdbpy_execute_mi_command (PyObject *self, PyObject *args,
 
 extern void serialize_mi_results (PyObject *results);
 
+/* Implementation of the gdb.notify_mi function.  */
+
+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..8ba770391c2
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-mi-notify.exp
@@ -0,0 +1,71 @@
+# Copyright (C) 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')" \
+    ".*=-test-notification\r\n\\^done" \
+    "python notification, no additional data parameter"
+
+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"
+
+mi_gdb_test "python gdb.notify_mi('', None)" \
+    ".*\\^error,msg=\".*\"" \
+    "python notification, empty notification name"
+
+mi_gdb_test "python gdb.notify_mi('**invalid**', None)" \
+    ".*\\^error,msg=\".*\"" \
+    "python notification, invalid notification name"
+
+mi_gdb_test "python gdb.notify_mi(\[1,2,3\], None)" \
+    ".*\\^error,msg=\".*\"" \
+    "python notification, non-string notification name"
+
+mi_gdb_test "python gdb.notify_mi()" \
+    ".*\\^error,msg=\".*\"" \
+    "python notification, no arguments passed"
+
+mi_gdb_test "python gdb.notify_mi('thread-group-added', \{'id' : 'i2'\})" \
+    ".*=thread-group-added,id=\"i2\"\r\n\\^done" \
+    "python notification, using existing internal notification name"
-- 
2.40.1

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

[Andrew Burgess]

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

> I hope I addressed all comments raised by Andrew for V2 series

Thanks for all your work on this series.  The looks great now.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

>
> Changes since V2:
>
> * Fixed formatting errors in patch 1 of this series
> * Fixed formatting errors in patch 2 of this series
> * Added report name of incorrect data type into error message
>   as suggested.
> * Moved target_terminal::ours_for_output () right after the
>   check if UI is an MI UI as suggested.
> * Fixed tests.
>
> Changes since V1:
>
> * Moved serialize_mi_result and helpers to py-mi.c as suggested.
>   Also, renamed serialize_mi_result to serialize_mi_results
>   instead of serialize_mi_data and do not rename serialize_mi_result_1.
>
>   The reason for this is that GDB documentation,
>   section GDB/MI Output Syntax, describes "result" being single 'variable = value'
>   pair and "result-record" and "async-output" contains zero or more of these "result"s,
>   so calling top-level serialization function serialize_mi_results seems
>   a better name than serialize_mi_data (used in previous version).
>
> * Made gdb.notify_mi DATA parameter optional as suggested.
>
> * Validate gdb.notify_mi NAME parameter as suggested.
>
> * Updated documentation:
>   * example formatted as black tool would do it,
>   * rephrase some sentences as suggested,
>   * document that DATA parameter is optional,
>   * document restrictions on NAME parameter and
>   * document that users should prefix user-defined notification
>     with hyphen to avoid possible conflicts.
>
> Jan Vrany (2):
>   gdb/python: generalize serialize_mi_result()
>   gdb/python: implement support for sending custom MI async
>     notifications
>
>  gdb/NEWS                                  |   3 +
>  gdb/doc/python.texi                       |  45 +++++
>  gdb/python/py-mi.c                        | 233 ++++++++++++++++++++++
>  gdb/python/py-micmd.c                     | 185 +----------------
>  gdb/python/python-internal.h              |  18 ++
>  gdb/python/python.c                       |   4 +
>  gdb/testsuite/gdb.python/py-mi-notify.exp |  71 +++++++
>  7 files changed, 383 insertions(+), 176 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.python/py-mi-notify.exp
>
> -- 
> 2.40.1

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

[Jan Vraný]

Polite ping.

Thanks! Jan

On Wed, 2023-09-13 at 15:38 +0100, Jan Vrany wrote:
> I hope I addressed all comments raised by Andrew and Eli
> in this version. The main change is added restriction on
> NAME parameter and hopefully more clear wording in
> documentation.
> 
> Thanks, Jan
> 
> Changes since V1:
> 
> * Moved serialize_mi_result and helpers to py-mi.c as suggested.
>   Also, renamed serialize_mi_result to serialize_mi_results
>   instead of serialize_mi_data and do not rename serialize_mi_result_1.
> 
>   The reason for this is that GDB documentation,
>   section GDB/MI Output Syntax, describes "result" being single 'variable = value'
>   pair and "result-record" and "async-output" contains zero or more of these "result"s,
>   so calling top-level serialization function serialize_mi_results seems
>   a better name than serialize_mi_data (used in previous version).
> 
> * Made gdb.notify_mi DATA parameter optional as suggested.
> 
> * Validate gdb.notify_mi NAME parameter as suggested.
> 
> * Updated documentation:
>   * example formatted as black tool would do it,
>   * rephrase some sentences as suggested,
>   * document that DATA parameter is optional,
>   * document restrictions on NAME parameter and
>   * document that users should prefix user-defined notification
>     with hyphen to avoid possible conflicts.
> 
> * Add more tests.
> 
> 
> Jan Vrany (2):
>   gdb/python: generalize serialize_mi_result()
>   gdb/python: implement support for sending custom MI async
>     notifications
> 
>  gdb/NEWS                                  |   3 +
>  gdb/doc/python.texi                       |  45 +++++
>  gdb/python/py-mi.c                        | 230 ++++++++++++++++++++++
>  gdb/python/py-micmd.c                     | 185 +----------------
>  gdb/python/python-internal.h              |  18 ++
>  gdb/python/python.c                       |   4 +
>  gdb/testsuite/gdb.python/py-mi-notify.exp |  71 +++++++
>  7 files changed, 380 insertions(+), 176 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.python/py-mi-notify.exp
> 

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

[Jan Vraný]

Polite ping.

Thanks! Jan

On Thu, 2023-09-21 at 11:50 +0100, Jan Vrany wrote:
> Polite ping.
> 
> Thanks! Jan
> 
> On Wed, 2023-09-13 at 15:38 +0100, Jan Vrany wrote:
> > I hope I addressed all comments raised by Andrew and Eli
> > in this version. The main change is added restriction on
> > NAME parameter and hopefully more clear wording in
> > documentation.
> > 
> > Thanks, Jan
> > 
> > Changes since V1:
> > 
> > * Moved serialize_mi_result and helpers to py-mi.c as suggested.
> >   Also, renamed serialize_mi_result to serialize_mi_results
> >   instead of serialize_mi_data and do not rename serialize_mi_result_1.
> > 
> >   The reason for this is that GDB documentation,
> >   section GDB/MI Output Syntax, describes "result" being single 'variable = value'
> >   pair and "result-record" and "async-output" contains zero or more of these "result"s,
> >   so calling top-level serialization function serialize_mi_results seems
> >   a better name than serialize_mi_data (used in previous version).
> > 
> > * Made gdb.notify_mi DATA parameter optional as suggested.
> > 
> > * Validate gdb.notify_mi NAME parameter as suggested.
> > 
> > * Updated documentation:
> >   * example formatted as black tool would do it,
> >   * rephrase some sentences as suggested,
> >   * document that DATA parameter is optional,
> >   * document restrictions on NAME parameter and
> >   * document that users should prefix user-defined notification
> >     with hyphen to avoid possible conflicts.
> > 
> > * Add more tests.
> > 
> > 
> > Jan Vrany (2):
> >   gdb/python: generalize serialize_mi_result()
> >   gdb/python: implement support for sending custom MI async
> >     notifications
> > 
> >  gdb/NEWS                                  |   3 +
> >  gdb/doc/python.texi                       |  45 +++++
> >  gdb/python/py-mi.c                        | 230 ++++++++++++++++++++++
> >  gdb/python/py-micmd.c                     | 185 +----------------
> >  gdb/python/python-internal.h              |  18 ++
> >  gdb/python/python.c                       |   4 +
> >  gdb/testsuite/gdb.python/py-mi-notify.exp |  71 +++++++
> >  7 files changed, 380 insertions(+), 176 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.python/py-mi-notify.exp
> > 
> 

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

[Jan Vraný]

Polite ping, 

Thanks! Jan


On Wed, 2023-09-27 at 19:56 +0100, Jan Vrany wrote:
> Polite ping.
> 
> Thanks! Jan
> 
> On Thu, 2023-09-21 at 11:50 +0100, Jan Vrany wrote:
> > Polite ping.
> > 
> > Thanks! Jan
> > 
> > On Wed, 2023-09-13 at 15:38 +0100, Jan Vrany wrote:
> > > I hope I addressed all comments raised by Andrew and Eli
> > > in this version. The main change is added restriction on
> > > NAME parameter and hopefully more clear wording in
> > > documentation.
> > > 
> > > Thanks, Jan
> > > 
> > > Changes since V1:
> > > 
> > > * Moved serialize_mi_result and helpers to py-mi.c as suggested.
> > >   Also, renamed serialize_mi_result to serialize_mi_results
> > >   instead of serialize_mi_data and do not rename serialize_mi_result_1.
> > > 
> > >   The reason for this is that GDB documentation,
> > >   section GDB/MI Output Syntax, describes "result" being single 'variable = value'
> > >   pair and "result-record" and "async-output" contains zero or more of these "result"s,
> > >   so calling top-level serialization function serialize_mi_results seems
> > >   a better name than serialize_mi_data (used in previous version).
> > > 
> > > * Made gdb.notify_mi DATA parameter optional as suggested.
> > > 
> > > * Validate gdb.notify_mi NAME parameter as suggested.
> > > 
> > > * Updated documentation:
> > >   * example formatted as black tool would do it,
> > >   * rephrase some sentences as suggested,
> > >   * document that DATA parameter is optional,
> > >   * document restrictions on NAME parameter and
> > >   * document that users should prefix user-defined notification
> > >     with hyphen to avoid possible conflicts.
> > > 
> > > * Add more tests.
> > > 
> > > 
> > > Jan Vrany (2):
> > >   gdb/python: generalize serialize_mi_result()
> > >   gdb/python: implement support for sending custom MI async
> > >     notifications
> > > 
> > >  gdb/NEWS                                  |   3 +
> > >  gdb/doc/python.texi                       |  45 +++++
> > >  gdb/python/py-mi.c                        | 230 ++++++++++++++++++++++
> > >  gdb/python/py-micmd.c                     | 185 +----------------
> > >  gdb/python/python-internal.h              |  18 ++
> > >  gdb/python/python.c                       |   4 +
> > >  gdb/testsuite/gdb.python/py-mi-notify.exp |  71 +++++++
> > >  7 files changed, 380 insertions(+), 176 deletions(-)
> > >  create mode 100644 gdb/testsuite/gdb.python/py-mi-notify.exp
> > > 
> > 
>