On 10/7/20 11:49 AM, Kevin Wolf wrote: > This adds a special meaning for 'help' and '?' as options to the keyval > parser. Instead of being an error (because of a missing value) or a > value for an implied key, they now request help, which is a new boolean > ouput of the parser in addition to the QDict.
output > > A new parameter 'p_help' is added to keyval_parse() that contains on > return whether help was requested. If NULL is passed, requesting help > results in an error and all other cases work like before. > > Turning previous error cases into help is a compatible extension. The > behaviour potentially changes for implied keys: They could previously > get 'help' as their value, which is now interpreted as requesting help. > > This is not a problem in practice because 'help' and '?' are not a valid > values for the implied key of any option parsed with keyval_parse(): > > * audiodev: union Audiodev, implied key "driver" is enum AudiodevDriver, > "help" and "?" are not among its values > > * display: union DisplayOptions, implied key "type" is enum > DisplayType, "help" and "?" are not among its values > > * blockdev: union BlockdevOptions, implied key "driver is enum > BlockdevDriver, "help" and "?" are not among its values > > * export: union BlockExport, implied key "type" is enum BlockExportType, > "help" and "?" are not among its values > > * monitor: struct MonitorOptions, implied key "mode"is enum MonitorMode, missing space > "help" and "?" are not among its values > > * nbd-server: struct NbdServerOptions, no implied key. Including the audit is nice (and thanks to Markus for helping derive the list). > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > +++ b/util/keyval.c > @@ -14,7 +14,7 @@ > * KEY=VALUE,... syntax: > * > * key-vals = [ key-val { ',' key-val } [ ',' ] ] > - * key-val = key '=' val > + * key-val = 'help' | key '=' val Maybe: = 'help' | '?' | key = '=' val > * key = key-fragment { '.' key-fragment } > * key-fragment = / [^=,.]* / > * val = { / [^,]* / | ',,' } > @@ -73,10 +73,14 @@ > * > * Additional syntax for use with an implied key: > * > - * key-vals-ik = val-no-key [ ',' key-vals ] > + * key-vals-ik = 'help' | val-no-key [ ',' key-vals ] and again for '?' here. Actually, this should probably be: key-vals-ik = 'help' [ ',' key-vals ] | '?' [ ',' key-vals ] | val-no-key [ ',' key-vals ] > * val-no-key = / [^=,]* / This is now slightly inaccurate, but I don't know how to concisely express the regex [^=,]* but not '?' or 'help', and the prose covers the ambiguity. > @@ -410,6 +442,14 @@ QDict *keyval_parse(const char *params, const char > *implied_key, > implied_key = NULL; > } > > + if (p_help) { > + *p_help = help; > + } else if (help) { > + error_setg(errp, "Help is not available for this option"); > + qobject_unref(qdict); > + return NULL; > + } This part is a definite improvement over v2. I'm assuming Markus can help touch up the comments and spelling errors, so: Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature