[PATCH v2] string-output-visitor: show structs as "<omitted>"

[PATCH v2] string-output-visitor: show structs as "<omitted>"

[Stefan Hajnoczi]

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.

This patch is a quick fix to solve the crash, but the long-term solution
is replacing StringOutputVisitor with something that can handle the full
gamut of values in QEMU.

Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v2:
- Note that struct fields are omitted in string-output-visitor.h
  [Markus]
- Add a TODO comment in end_struct() [Markus]
---
 include/qapi/string-output-visitor.h |  6 +++---
 qapi/string-output-visitor.c         | 16 ++++++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/include/qapi/string-output-visitor.h b/include/qapi/string-output-visitor.h
index 268dfe9986..b1ee473b30 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.  Struct fields are not shown.  It also requires a
+ * non-null list argument to visit_start_list().
  */
 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..f0c1dea89e 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -292,6 +292,20 @@ 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);
+
+    /* TODO actually print struct fields */
+    string_output_set(sov, g_strdup("<omitted>"));
+}
+
 static bool
 start_list(Visitor *v, const char *name, GenericList **list, size_t size,
            Error **errp)
@@ -379,6 +393,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;
-- 
2.43.0

Re: [PATCH v2] 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.
>
> This patch is a quick fix to solve the crash, but the long-term solution
> is replacing StringOutputVisitor with something that can handle the full
> gamut of values in QEMU.
>
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Okay as a stopgap to unblock your virtio-blk-pci work.  Suggest you
merge this patch together with the patch that needs it.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

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

[Stefan Hajnoczi]

On Tue, 19 Dec 2023 at 09:49, Markus Armbruster <armbru@redhat.com> wrote:
>
> 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.
> >
> > This patch is a quick fix to solve the crash, but the long-term solution
> > is replacing StringOutputVisitor with something that can handle the full
> > gamut of values in QEMU.
> >
> > Cc: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> Okay as a stopgap to unblock your virtio-blk-pci work.  Suggest you
> merge this patch together with the patch that needs it.

Thanks!

Stefan