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

[Andrew Burgess <aburgess@redhat.com>]

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