[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/4] keyval: Parse help options
From: |
Eric Blake |
Subject: |
Re: [PATCH v3 1/4] keyval: Parse help options |
Date: |
Wed, 7 Oct 2020 12:29:40 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 |
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 <kwolf@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 <eblake@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
- [PATCH v3 0/4] qemu-storage-daemon: Remove QemuOpts from --object parser, Kevin Wolf, 2020/10/07
- [PATCH v3 2/4] qom: Factor out helpers from user_creatable_print_help(), Kevin Wolf, 2020/10/07
- [PATCH v3 3/4] qom: Add user_creatable_print_help_from_qdict(), Kevin Wolf, 2020/10/07
- [PATCH v3 4/4] qemu-storage-daemon: Remove QemuOpts from --object parser, Kevin Wolf, 2020/10/07
- [PATCH v3 1/4] keyval: Parse help options, Kevin Wolf, 2020/10/07
- Re: [PATCH v3 1/4] keyval: Parse help options,
Eric Blake <=
- Re: [PATCH v3 1/4] keyval: Parse help options, Markus Armbruster, 2020/10/09