Re: [Qemu-devel] [PATCH 1/2] qapi: Stub out StringOutputVisitor struct support

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Avoid a segfault when visiting, e.g., the QOM rtc-time property,
> > by implementing the struct callbacks and raising an Error.
> >
> > Signed-off-by: Andreas Färber <afaerber@suse.de>
> >
> > Updated for changed interface:
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  qapi/string-output-visitor.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> > index 94ac821..4e7e97f 100644
> > --- a/qapi/string-output-visitor.c
> > +++ b/qapi/string-output-visitor.c
> > @@ -12,6 +12,7 @@
> >  
> >  #include "qemu/osdep.h"
> >  #include "qemu-common.h"
> > +#include "qapi/error.h"
> >  #include "qapi/string-output-visitor.h"
> >  #include "qapi/visitor-impl.h"
> >  #include "qemu/host-utils.h"
> > @@ -266,6 +267,16 @@ static void print_type_number(Visitor *v, const char *name, double *obj,
> >      string_output_set(sov, g_strdup_printf("%f", *obj));
> >  }
> >  
> > +static void start_struct(Visitor *v, const char *name, void **obj, size_t size,
> > +           Error **errp)
> > +{
> > +    error_setg(errp, "struct type not implemented");
> > +}
> > +
> > +static void end_struct(Visitor *v, void **obj)
> > +{
> > +}
> > +
> 
> This is just one of the several things this visitor doesn't implement.
> See the comment in string-output-visitor.h.
> 
> String input visitor and options visitor have similar holes; see the
> comments in string-input-visitor.h and opts-visitor.h.
> 
> Should we change all of them together to report errors instead of crash?
> With common "error out because this isn't implemented" methods?

In that case wouldn't it be best to change visit_start_struct/visit_end_struct
to do the check (Like visit_check_struct does).

Dave

> 
> >  static void
> >  start_list(Visitor *v, const char *name, GenericList **list, size_t size,
> >             Error **errp)
> > @@ -356,6 +367,8 @@ Visitor *string_output_visitor_new(bool human, char **result)
> >      v->visitor.end_list = end_list;
> >      v->visitor.complete = string_output_complete;
> >      v->visitor.free = string_output_free;
> > +    v->visitor.start_struct = start_struct;
> > +    v->visitor.end_struct = end_struct;
> >  
> >      return &v->visitor;
> >  }
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK