* [Qemu-devel] [PATCH] qapi: add explicit null to string input and output visitors @ 2016-11-19 8:28 Greg Kurz 2016-11-21 16:32 ` Eric Blake 2016-12-13 8:43 ` Markus Armbruster 0 siblings, 2 replies; 5+ messages in thread From: Greg Kurz @ 2016-11-19 8:28 UTC (permalink / raw) To: qemu-devel; +Cc: David Gibson, Markus Armbruster, Michael Roth This may be used for deprecated object properties that are kept for backwards compatibility. Signed-off-by: Greg Kurz <groug@kaod.org> --- This is enough to fix David's patch: [RFCv2 10/12] pseries: Move CPU compatibility property to machine Messag-Id: <1479248275-18889-11-git-send-email-david@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) +{ +} + 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; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qapi: add explicit null to string input and output visitors 2016-11-19 8:28 [Qemu-devel] [PATCH] qapi: add explicit null to string input and output visitors Greg Kurz @ 2016-11-21 16:32 ` Eric Blake 2016-11-21 17:19 ` Greg Kurz 2016-12-13 8:43 ` Markus Armbruster 1 sibling, 1 reply; 5+ messages in thread From: Eric Blake @ 2016-11-21 16:32 UTC (permalink / raw) To: Greg Kurz, qemu-devel; +Cc: Michael Roth, Markus Armbruster, David Gibson [-- Attachment #1: Type: text/plain, Size: 3330 bytes --] 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 <groug@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? > > Messag-Id: <1479248275-18889-11-git-send-email-david@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? > + > 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; > > > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qapi: add explicit null to string input and output visitors 2016-11-21 16:32 ` Eric Blake @ 2016-11-21 17:19 ` Greg Kurz 0 siblings, 0 replies; 5+ messages in thread From: Greg Kurz @ 2016-11-21 17:19 UTC (permalink / raw) To: Eric Blake; +Cc: qemu-devel, Michael Roth, Markus Armbruster, David Gibson [-- Attachment #1: Type: text/plain, Size: 4105 bytes --] On Mon, 21 Nov 2016 10:32:50 -0600 Eric Blake <eblake@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 <groug@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-david@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; > > > > > > > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qapi: add explicit null to string input and output visitors 2016-11-19 8:28 [Qemu-devel] [PATCH] qapi: add explicit null to string input and output visitors Greg Kurz 2016-11-21 16:32 ` Eric Blake @ 2016-12-13 8:43 ` Markus Armbruster 2016-12-13 10:17 ` Greg Kurz 1 sibling, 1 reply; 5+ messages in thread From: Markus Armbruster @ 2016-12-13 8:43 UTC (permalink / raw) To: Greg Kurz; +Cc: qemu-devel, Michael Roth, David Gibson Greg Kurz <groug@kaod.org> writes: > This may be used for deprecated object properties that are kept for > backwards compatibility. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > > This is enough to fix David's patch: > > [RFCv2 10/12] pseries: Move CPU compatibility property to machine > > Messag-Id: <1479248275-18889-11-git-send-email-david@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) > +{ > +} > + I figure this accepts an arbitrary string. The output visitor ... > 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(""); ... produces an empty string. Sure this makes sense? Shouldn't the input visitor reject anything but the empty string? > + } > + 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; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qapi: add explicit null to string input and output visitors 2016-12-13 8:43 ` Markus Armbruster @ 2016-12-13 10:17 ` Greg Kurz 0 siblings, 0 replies; 5+ messages in thread From: Greg Kurz @ 2016-12-13 10:17 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-devel, Michael Roth, David Gibson On Tue, 13 Dec 2016 09:43:41 +0100 Markus Armbruster <armbru@redhat.com> wrote: > Greg Kurz <groug@kaod.org> writes: > > > This may be used for deprecated object properties that are kept for > > backwards compatibility. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > > > This is enough to fix David's patch: > > > > [RFCv2 10/12] pseries: Move CPU compatibility property to machine > > > > Messag-Id: <1479248275-18889-11-git-send-email-david@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) > > +{ > > +} > > + > > I figure this accepts an arbitrary string. The output visitor ... > Yes, I assumed that null means "we don't care"... but maybe I'm wrong. :) > > > 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(""); > > ... produces an empty string. Sure this makes sense? > > Shouldn't the input visitor reject anything but the empty string? > Looking at how null is handled for qobject, I suppose you're right. I'll send a v2. > > + } > > + 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; ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-13 10:17 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-19 8:28 [Qemu-devel] [PATCH] qapi: add explicit null to string input and output visitors Greg Kurz 2016-11-21 16:32 ` Eric Blake 2016-11-21 17:19 ` Greg Kurz 2016-12-13 8:43 ` Markus Armbruster 2016-12-13 10:17 ` Greg Kurz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).