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