Re: [PATCH v6 1/5] monitor/hmp: add support for flag argument with value

[Eric Blake]

On Wed, Oct 20, 2021 at 03:54:56PM +0200, Stefan Reiter wrote:
> Adds support for the "-xV" parameter type, where "-x" denotes a flag
> name and the "V" suffix indicates that this flag is supposed to take an
> arbitrary string parameter.
> 
> These parameters are always optional, the entry in the qdict will be
> omitted if the flag is not given.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> 
> v6:
> It wasn't possible to pass the 'connected' parameter to set_password, since 
> the
> code to handle optional parameters couldn't live with a different param (not
> starting with '-') coming up instead - fix that by advancing over the 'value
> flag' modifier in case `*p != '-'`.
> 
> Also change the modifier to 'V' instead of 'S' so it can be distinguished from
> an actual trailing 'S' type param.
> 
> Discovered in testing. I dropped Eric's R-b due to the code change.

Good that you caught it, and correct in dropping my earlier R-b.  But
now that I've read this version:

> +++ b/monitor/monitor-internal.h
> @@ -63,7 +63,8 @@
>   * '.'          other form of optional type (for 'i' and 'l')
>   * 'b'          boolean
>   *              user mode accepts "on" or "off"
> - * '-'          optional parameter (eg. '-f')
> + * '-'          optional parameter (eg. '-f'); if followed by an 'V', it

s/an/a/

> + *              specifies an optional string param (e.g. '-fV' allows '-f 
> foo')
>   *

With the typo fix,
Reviewed-by: Eric Blake <eblake@redhat.com>

[Hmm, I see that you posted v7 in the meantime, and it still has the
typo, but I already typed this mail]

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org