Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion

[Markus Armbruster]

"Daniel P. Berrange" <address@hidden> writes:

> Currently the QmpInputVisitor assumes that all scalar
> values are directly represented as their final types.
> ie it assumes an 'int' is using QInt, and a 'bool' is
> using QBool.
>
> This adds an alternative constructor for QmpInputVisitor
> that will set it up such that it expects a QString for
> all scalar types instead.
>
> This makes it possible to use QmpInputVisitor with a
> QDict produced from QemuOpts, where everything is in
> string format.

Can you explain how this is related to the Options visitor?

> Reviewed-by: Marc-André Lureau <address@hidden>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  include/qapi/qobject-input-visitor.h |  41 +++++++++-
>  qapi/qobject-input-visitor.c         | 115 +++++++++++++++++++++++++-
>  tests/test-qobject-input-visitor.c   | 152 
> ++++++++++++++++++++++++++++++++++-
>  3 files changed, 298 insertions(+), 10 deletions(-)
>
> diff --git a/include/qapi/qobject-input-visitor.h 
> b/include/qapi/qobject-input-visitor.h
> index cde328d..aa911cb 100644
> --- a/include/qapi/qobject-input-visitor.h
> +++ b/include/qapi/qobject-input-visitor.h
> @@ -19,12 +19,45 @@
>  
>  typedef struct QObjectInputVisitor QObjectInputVisitor;
>  
> -/*
> - * Return a new input visitor that converts a QObject to a QAPI object.
> +/**
> + * qobject_input_visitor_new:
> + * @obj: the input object to visit
> + * @strict: whether to require that all input keys are consumed
> + *
> + * Create a new input visitor that converts a QObject to a QAPI object.
> + *
> + * Any scalar values in the @obj input data structure should be in the
> + * required type already. i.e. if visiting a bool, the value should
> + * already be a QBool instance.
>   *
> - * Set @strict to reject a parse that doesn't consume all keys of a
> - * dictionary; otherwise excess input is ignored.
> + * If @strict is set to true, then an error will be reported if any
> + * dict keys are not consumed during visitation.
> + *
> + * The returned input visitor should be released by calling
> + * visit_free() when no longer required.
> + *
> + * Returns: a new input visitor
>   */

GDK-Doc style is an egregious waste of screen space and reader
bandwidth.  Much of the text is spent on restating the obvious, drowning
out the parts that are actually interesting.

Compare:

   /*
  - * Return a new input visitor that converts a QObject to a QAPI object.
  + * Create an input visitor that converts @obj to a QAPI object.
    *
  - * Set @strict to reject a parse that doesn't consume all keys of a
  - * dictionary; otherwise excess input is ignored.
  + * 
  + * If @strict is true, the visit is expected to consume all of @obj's
  + * keys.  Else, excess keys are silently ignored.
  + *
  + * Return the input visitor.  It should be destroyed with visit_free().
  + */

Like your version, this is vague on what exactly happens when a strict
visit fails to consume all keys.  It's more explicitly vague, though :)

GDK-Doc is that way so that a fairly dumb tool can generate fairly
usable library reference documentation.  We're not using that tool.
This is not a library.  I loathe having my read bandwidth wasted.

>  Visitor *qobject_input_visitor_new(QObject *obj, bool strict);
>  
> +/**
> + * qobject_string_input_visitor_new:
> + * @obj: the input object to visit
> + *
> + * Create a new input visitor that converts a QObject to a QAPI object.
> + *
> + * Any scalar values in the @obj input data structure should always be
> + * represented as strings. i.e. if visiting a boolean, the value should
> + * be a QString whose contents represent a valid boolean.
> + *
> + * The visitor always operates in strict mode, requiring all dict keys
> + * to be consumed during visitation.
> + *
> + * The returned input visitor should be released by calling
> + * visit_free() when no longer required.
> + *
> + * Returns: a new input visitor
> + */
> +Visitor *qobject_string_input_visitor_new(QObject *obj);
> +
>  #endif
[...]