Re: [RFC] string-output-visitor: show structs as "<omitted>"

[Markus Armbruster]

Stefan Hajnoczi <stefanha@redhat.com> writes:

> StringOutputVisitor crashes when it visits a struct because
> ->start_struct() is NULL.
>
> Show "<omitted>" instead of crashing. This is necessary because the
> virtio-blk-pci iothread-vq-mapping parameter that I'd like to introduce
> soon is a list of IOThreadMapping structs.
>
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> Can we do better?
>
> I am unfamiliar with StringOutputVisitor, so I wasn't sure how to
> proceed. Is the format or at least the intended use of
> StringOutputVisitor's output defined somewhere?

Bwuahahahaha!

SCNR

>                                                 Does it need to be a
> single line or can the output be multiple lines?

I'm afraid we need to review its users to be sure.

> Or maybe I shouldn't introduce a qdev property with IOThreadMappingList
> as its type in
> https://lore.kernel.org/qemu-devel/ZUoPiFxIIwFq5wMg@redhat.com/?

QOM initially supported only scalar properties.

I think maintaining this restriction will lead to awkward interfaces.
Lists are clearly useful.  And then we'll likely want list of struct,
not multiple lists.

Lifting the restriction will take some work.

On the string visitors, see also my musings in

    Subject: Re: [PATCH v2 1/2] qdev: add IOThreadVirtQueueMappingList property 
type 
    Date: Mon, 11 Dec 2023 16:32:06 +0100
    Message-ID: <87msugah6x.fsf@pond.sub.org>

> ---
>  include/qapi/string-output-visitor.h |  6 +++---
>  qapi/string-output-visitor.c         | 14 ++++++++++++++
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/include/qapi/string-output-visitor.h 
> b/include/qapi/string-output-visitor.h
> index 268dfe9986..762fe3f705 100644
> --- a/include/qapi/string-output-visitor.h
> +++ b/include/qapi/string-output-visitor.h
> @@ -26,9 +26,9 @@ typedef struct StringOutputVisitor StringOutputVisitor;
>   * If everything else succeeds, pass @result to visit_complete() to
>   * collect the result of the visit.
>   *
> - * The string output visitor does not implement support for visiting
> - * QAPI structs, alternates, null, or arbitrary QTypes.  It also
> - * requires a non-null list argument to visit_start_list().
> + * The string output visitor does not implement support for alternates, null,
> + * or arbitrary QTypes.  It also requires a non-null list argument to
> + * visit_start_list().

Mention output for structs is information-free?

>   */
>  Visitor *string_output_visitor_new(bool human, char **result);
>  
> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> index c0cb72dbe4..363dac00fe 100644
> --- a/qapi/string-output-visitor.c
> +++ b/qapi/string-output-visitor.c
> @@ -292,6 +292,18 @@ static bool print_type_null(Visitor *v, const char 
> *name, QNull **obj,
>      return true;
>  }
>  
> +static bool start_struct(Visitor *v, const char *name, void **obj,
> +                         size_t size, Error **errp)
> +{
> +    return true;
> +}
> +
> +static void end_struct(Visitor *v, void **obj)
> +{
> +    StringOutputVisitor *sov = to_sov(v);
> +    string_output_set(sov, g_strdup("<omitted>"));

TODO comment?

> +}
> +
>  static bool
>  start_list(Visitor *v, const char *name, GenericList **list, size_t size,
>             Error **errp)
> @@ -379,6 +391,8 @@ Visitor *string_output_visitor_new(bool human, char 
> **result)
>      v->visitor.type_str = print_type_str;
>      v->visitor.type_number = print_type_number;
>      v->visitor.type_null = print_type_null;
> +    v->visitor.start_struct = start_struct;
> +    v->visitor.end_struct = end_struct;
>      v->visitor.start_list = start_list;
>      v->visitor.next_list = next_list;
>      v->visitor.end_list = end_list;