| Message ID | 1490014548-15083-5-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/20/2017 07:55 AM, Markus Armbruster wrote: > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > util/keyval.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/util/keyval.c b/util/keyval.c > index 46cd540..93d5db6 100644 > --- a/util/keyval.c > +++ b/util/keyval.c > @@ -61,6 +61,16 @@ > * "key absent" already means "optional object/array absent", which > * isn't the same as "empty object/array present". > * > + * Design flaw: scalar values can only be strings; there is no way to > + * denote numbers, true, false or null. The special QObject input > + * visitor returned by qobject_input_visitor_new_keyval() mostly hides > + * this by automatically converting strings to the type the visitor > + * expects. Breaks down for alternate types and type 'any', where the > + * visitor's expectation isn't clear. Code visiting such types needs > + * to do the conversion itself, but only when using this keyval > + * visitor. Awkward. Alternate types without a string member don't > + * work at all. We've toyed with the idea of making socket parsing accept an alternate between string and integer (right now it's string due to named port support, even though most users end up accessing an integer causing a lot of in-tree parsing/formatting between integers and strings) - that will be one case that is particularly impacted by this design flaw, if we ever go through with that idea. But enough speculation about the future - your patch as written is accurate for the present. Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake <eblake@redhat.com> writes: > On 03/20/2017 07:55 AM, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> util/keyval.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/util/keyval.c b/util/keyval.c >> index 46cd540..93d5db6 100644 >> --- a/util/keyval.c >> +++ b/util/keyval.c >> @@ -61,6 +61,16 @@ >> * "key absent" already means "optional object/array absent", which >> * isn't the same as "empty object/array present". >> * >> + * Design flaw: scalar values can only be strings; there is no way to >> + * denote numbers, true, false or null. The special QObject input >> + * visitor returned by qobject_input_visitor_new_keyval() mostly hides >> + * this by automatically converting strings to the type the visitor >> + * expects. Breaks down for alternate types and type 'any', where the >> + * visitor's expectation isn't clear. Code visiting such types needs >> + * to do the conversion itself, but only when using this keyval >> + * visitor. Awkward. Alternate types without a string member don't >> + * work at all. > > We've toyed with the idea of making socket parsing accept an alternate > between string and integer (right now it's string due to named port > support, even though most users end up accessing an integer causing a > lot of in-tree parsing/formatting between integers and strings) - that > will be one case that is particularly impacted by this design flaw, if > we ever go through with that idea. I'll reply to this in "Re: Non-flat command line option argument syntax". > But enough speculation about the > future - your patch as written is accurate for the present. > > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks!
diff --git a/util/keyval.c b/util/keyval.c index 46cd540..93d5db6 100644 --- a/util/keyval.c +++ b/util/keyval.c @@ -61,6 +61,16 @@ * "key absent" already means "optional object/array absent", which * isn't the same as "empty object/array present". * + * Design flaw: scalar values can only be strings; there is no way to + * denote numbers, true, false or null. The special QObject input + * visitor returned by qobject_input_visitor_new_keyval() mostly hides + * this by automatically converting strings to the type the visitor + * expects. Breaks down for alternate types and type 'any', where the + * visitor's expectation isn't clear. Code visiting such types needs + * to do the conversion itself, but only when using this keyval + * visitor. Awkward. Alternate types without a string member don't + * work at all. + * * Additional syntax for use with an implied key: * * key-vals-ik = val-no-key [ ',' key-vals ]
Signed-off-by: Markus Armbruster <armbru@redhat.com> --- util/keyval.c | 10 ++++++++++ 1 file changed, 10 insertions(+)