qemu-devel
[Top][All Lists]
Advanced

[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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]