On Mon, 21 Nov 2016 10:32:50 -0600 Eric Blake <ebl...@redhat.com> wrote:
> On 11/19/2016 02:28 AM, Greg Kurz wrote:
> > This may be used for deprecated object properties that are kept for
> > backwards compatibility.
> >
> > Signed-off-by: Greg Kurz <gr...@kaod.org>
> > ---
> >
> > This is enough to fix David's patch:
> >
> > [RFCv2 10/12] pseries: Move CPU compatibility property to machine
>
> Do we need both input and output visitors, or is the problem only caused
> on an output visit?
>
David's patch unifies the get/set property info callbacks into a single
getset_compat_deprecated() function which calls visit_type_null(). So,
even if the operation is a nop for input visitors, we need it anyway,
otherwise visit_type_null() dereferences a NULL pointer.
> >
> > Messag-Id: <1479248275-18889-11-git-send-email-da...@gibson.dropbear.id.au>
> >
> > qapi/string-input-visitor.c | 5 +++++
> > qapi/string-output-visitor.c | 14 ++++++++++++++
> > 2 files changed, 19 insertions(+)
> >
> > diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> > index 8dfa5612522b..4a378d7b67e6 100644
> > --- a/qapi/string-input-visitor.c
> > +++ b/qapi/string-input-visitor.c
> > @@ -314,6 +314,10 @@ static void parse_type_number(Visitor *v, const char
> > *name, double *obj,
> > *obj = val;
> > }
> >
> > +static void parse_type_null(Visitor *v, const char *name, Error **errp)
> > +{
> > +}
>
> In particular, this always succeeds, even though we don't have a
> symmetric way to guarantee that the user can specifically request NULL
> input (the qobject visitor checks for the JSON keyword NULL, but that's
> different from string parsing), so I'm a bit worried that it is not
> doing what we want. If you omit this hunk, does the problem with David's
> patch still go away with just the output visitor change?
>
What we want here is to simply ignore any string the user would pass.
As explained above, this hunk is needed as long as David's patch calls
visit_type_null() for both input and output. Otherwise, it should have
separate functions: get_compat_deprecated() which calls visit_type_null()
and set_compat_deprecated() which does nothing.
> > +
> > static void parse_optional(Visitor *v, const char *name, bool *present)
> > {
> > StringInputVisitor *siv = to_siv(v);
> > @@ -348,6 +352,7 @@ Visitor *string_input_visitor_new(const char *str)
> > v->visitor.type_bool = parse_type_bool;
> > v->visitor.type_str = parse_type_str;
> > v->visitor.type_number = parse_type_number;
> > + v->visitor.type_null = parse_type_null;
> > v->visitor.start_list = start_list;
> > v->visitor.next_list = next_list;
> > v->visitor.end_list = end_list;
> > diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> > index 94ac8211d144..5ec5352ca87c 100644
> > --- a/qapi/string-output-visitor.c
> > +++ b/qapi/string-output-visitor.c
> > @@ -266,6 +266,19 @@ static void print_type_number(Visitor *v, const char
> > *name, double *obj,
> > string_output_set(sov, g_strdup_printf("%f", *obj));
> > }
> >
> > +static void print_type_null(Visitor *v, const char *name, Error **errp)
> > +{
> > + StringOutputVisitor *sov = to_sov(v);
> > + char *out;
> > +
> > + if (sov->human) {
> > + out = g_strdup("<null>");
> > + } else {
> > + out = g_strdup("");
> > + }
> > + string_output_set(sov, out);
> > +}
> > +
> > static void
> > start_list(Visitor *v, const char *name, GenericList **list, size_t size,
> > Error **errp)
> > @@ -351,6 +364,7 @@ Visitor *string_output_visitor_new(bool human, char
> > **result)
> > v->visitor.type_bool = print_type_bool;
> > v->visitor.type_str = print_type_str;
> > v->visitor.type_number = print_type_number;
> > + v->visitor.type_null = print_type_null;
> > v->visitor.start_list = start_list;
> > v->visitor.next_list = next_list;
> > v->visitor.end_list = end_list;
> >
> >
> >
>
pgpbXfDcLmbpU.pgp
Description: OpenPGP digital signature
