[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.9 v2] qapi: add explicit null to string in
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH for-2.9 v2] qapi: add explicit null to string input and output visitors |
Date: |
Thu, 15 Dec 2016 21:30:52 +1100 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Thu, Dec 15, 2016 at 10:44:02AM +0100, Markus Armbruster wrote:
> Greg Kurz <address@hidden> writes:
>
> > This may be used for deprecated object properties that are kept for
> > backwards compatibility.
> >
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> > v2: - input visitor to reject non-empty strings
> >
> > This is needed for David's patch to work:
> >
> > [RFCv2 10/12] pseries: Move CPU compatibility property to machine
>
> Recommend to insert this patch less the fixup to David's patch before
> David's patch, then squash in the fixup, if it's needed.
That series of mine needs a bunch of rework for several things
including this. It'll probably be a while before I can get back to
that, so don't hold this up on my account.
>
> > Messag-Id: <address@hidden>
> >
> > Since this v2 changes the behavior to reject non-empty null properties,
> > it is up to getset_compat_deprecated() to ignore the error. The following
> > folded into patch [RFCv2 10/12] does the trick:
> >
> > --- a/target-ppc/translate_init.c
> > +++ b/target-ppc/translate_init.c
> > @@ -8446,7 +8446,7 @@ static void getset_compat_deprecated(Object *obj,
> > Visitor
> > if (!qtest_enabled()) {
> > error_report("CPU 'compat' property is deprecated and has no
> > effect; us
> > }
> > - visit_type_null(v, name, errp);
> > + visit_type_null(v, name, NULL);
>
> Are you sure we want to ignore errors here?
>
> > }
> >
> > static PropertyInfo ppc_compat_deprecated_propinfo = {
> > ---
> > qapi/string-input-visitor.c | 11 +++++++++++
> > qapi/string-output-visitor.c | 14 ++++++++++++++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> > index 8dfa5612522b..9e9d2a1d2865 100644
> > --- a/qapi/string-input-visitor.c
> > +++ b/qapi/string-input-visitor.c
> > @@ -314,6 +314,16 @@ 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)
> > +{
> > + StringInputVisitor *siv = to_siv(v);
> > +
> > + if (siv->string && siv->string[0]) {
> > + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> > + "null");
> > + }
> > +}
>
> When !siv->string, the other parse_type_FOO() generally do
>
> error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> ... a description of the expected type...);
>
> This one doesn't. Why?
>
> Should the conditional be !siv->string || siv->string[0]?
>
> > +
> > static void parse_optional(Visitor *v, const char *name, bool *present)
> > {
> > StringInputVisitor *siv = to_siv(v);
> > @@ -348,6 +358,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;
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature