Re: [PATCH 2/2] hmp: Simplify qom_set

[Markus Armbruster <armbru@redhat.com>]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
>> On 5/20/20 5:11 PM, Dr. David Alan Gilbert (git) wrote:
>> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> > 
>> > Simplify qom_set by making it use qmp_qom_set and the JSON parser.
>> > 
>> > Note that qom-set likes JSON strings quoted with ' not ", e.g.:
>> > 
>> > (qemu) qom-get /machine smm
>> > "auto"
>> > (qemu) qom-set /machine smm 'auto'
>> 
>> Will I get this output using "?
>> 
>> (qemu) qom-set /machine smm "auto"
>> Error: Expecting a JSON value
>
> The error you get is:
>
> (qemu) qom-set /machine smm "auto"
> Error: JSON parse error, invalid keyword 'auto'
>
> I think, having seen alphanumerics, it's expecting a keyword;
> i.e. a true/false making a bool, or a null.

The command parses its argument as JSON.

Before we get there, the HMP core extracts the argument from the line of
input.  The extraction is guided by the command's .args_type, in this
case the 's' in "value:s" in

    {
        .name       = "qom-set",
        .args_type  = "path:s,property:s,value:s",
        [...]
    },

monitor/monitor-internal.h documents type code 's' as

 * 's'          string (accept optional quote)

The implementation boils down to:

1. Skip whitespace.

2. If looking at '"', get the string enclosed in '"', with C-like escape
   sequences \n, \r, \\, \', \".

3. Else, get the string up to the next whitespace.

See get_str().

Therefore, argument "auto" is the same as auto.  Parsing auto as JSON
duly fails.

Argument 'auto' works, but only because qobject_from_json() recognizes
single-quoted strings.  This is as extension over RFC 8259.

Using single quotes falls apart when you want to pass something
containing whitespace.  Then you'd have to use

    "\"ugly and unintuitive\""

or, again relying on the extension

    "'ugly and unintuitive'"

There's a better way, and Paolo pointed it out in

    Subject: Re: [RFC PATCH] qom: Implement qom-get HMP command
    Date: Thu, 21 May 2020 16:24:12 +0200
    Message-ID: <2c148331-78ae-31f7-8702-d65c37a090b0@redhat.com>

Use argument type 'S'.  Documented as

 * 'S'          it just appends the rest of the string (accept optional quote)

but the parenthesis is confusing.  It really just skips whitespace, then
extracts the remainder of the line.  Can't do string with leading
whitespace, but that's just fine for us.

Please squash in:

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 250ddae54d..28256209b5 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1806,7 +1806,7 @@ ERST
 
     {
         .name       = "qom-set",
-        .args_type  = "path:s,property:s,value:s",
+        .args_type  = "path:s,property:s,value:S",
         .params     = "path property value",
         .help       = "set QOM property",
         .cmd        = hmp_qom_set,