Re: [PATCH 1/4] keyval: Parse help options

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 9/29/20 12:26 PM, Kevin Wolf wrote:
>> This adds a new parameter 'help' to keyval_parse() that enables parsing
>> of help options. If NULL is passed, the function behaves the same as
>> before. But if a bool pointer is given, it contains the information
>> whether an option "help" without value was given (which would otherwise
>> either result in an error or be interpreted as the value for an implied
>> key).
>> 
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>
>> +++ b/util/keyval.c
>
> Might be nice to see this before the testsuite changes by tweaking the
> git orderfile.
>
>> @@ -166,7 +166,7 @@ static QObject *keyval_parse_put(QDict *cur,
>>   * On failure, return NULL.
>>   */
>>  static const char *keyval_parse_one(QDict *qdict, const char *params,
>> -                                    const char *implied_key,
>> +                                    const char *implied_key, bool *help,
>>                                      Error **errp)
>>  {
>>      const char *key, *key_end, *s, *end;
>> @@ -179,6 +179,16 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
>>  
>>      key = params;
>>      len = strcspn(params, "=,");
>> +
>> +    if (help && key[len] != '=' && !strncmp(key, "help", len)) {
>
> What if the user typed "help,," to get "help," as the value of the
> implied key?

The value of an implied key cannot contain ','.  This is intentional.

test-keyval.c:

    /* Implied key with empty value (qemu_opts_parse() accepts this) */
    qdict = keyval_parse(",", "implied", &err);
    error_free_or_abort(&err);
    g_assert(!qdict);

    /* Likewise (qemu_opts_parse(): implied key with comma value) */
    qdict = keyval_parse(",,,a=1", "implied", &err);
    error_free_or_abort(&err);
    g_assert(!qdict);

Grammar:

 *   val-no-key   = / [^=,]* /

Aside: should be + instead of *.  Doc bug.  I'll fix it.

[...]