Re: [PATCHv3 19/36] qemu: json: Add format strings for optional command arguments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/30/2014 02:37 AM, Peter Krempa wrote:
> This patch adds option to specify that a json qemu command argument is
> optional without the need to use if's or ternary operators to pass the
> list. Additionally all the modifier characters are documented to avoid
> user confusion.
> ---
>  src/qemu/qemu_monitor_json.c | 122 +++++++++++++++++++++++++++++--------------
>  1 file changed, 84 insertions(+), 38 deletions(-)
> 

The diffstat is misleading - the bulk of the addition is documentation,
and the bulk of the deletions are compression taking advantage of the
new feature.  Overall, I like the patch!

> +         *
> +         * i: signed integer value
> +         * z: signed integer value, omitted if zero
> +         *
> +         * I: signed long integer value
> +         * Z: integer value, signed, omitted if zero

Looks more consistent as: "signed long integer value, omitted if zero"

> +         *
> +         * u: unsigned integer value
> +         * p: unsigned integer value, omitted if zero
> +         *
> +         * U: unsigned long integer value (see below for quirks)
> +         * P: unsigned long integer value, omitted if zero,

drop the trailing comma

> +         *
> +         * d: double precision floating point number
> +         * b: boolean value

Is it worth a B for a boolean value that is omitted if false?

> +         * n: json null value
> +         * a: json array
> +         */
>          type = key[0];
>          key += 2;
> 

> @@ -3557,7 +3603,7 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon,
> 
>      cmd = qemuMonitorJSONMakeCommand("send-key",
>                                       "a:keys", keys,
> -                                      holdtime ? "U:hold-time" : NULL, holdtime,
> +                                      "P:hold-time", holdtime,
>                                        NULL);

Fix the indentation while you are at it.

ACK with nits addressed.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]