[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 1/7] keyval: Fix and clarify grammar
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v4 1/7] keyval: Fix and clarify grammar |
Date: |
Mon, 19 Oct 2020 11:03:03 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
Eric Blake <eblake@redhat.com> writes:
> On 10/11/20 2:34 AM, Markus Armbruster wrote:
>> The grammar has a few issues:
>> * key-fragment = / [^=,.]* /
>> Prose restricts key fragments: they "must be valid QAPI names or
>> consist only of decimal digits". Technically, '' consists only of
>> decimal digits. The code rejects that. Fix the grammar.
>> * val = { / [^,]* / | ',,' }
>> Use + instead of *. Accepts the same language.
>> * val-no-key = / [^=,]* /
>> The code rejects an empty value. Fix the grammar.
>> * Section "Additional syntax for use with an implied key" is
>> confusing. Rewrite it.
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> util/keyval.c | 16 ++++++++++------
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>> diff --git a/util/keyval.c b/util/keyval.c
>> index 13def4af54..82d8497c71 100644
>> --- a/util/keyval.c
>> +++ b/util/keyval.c
>> @@ -16,8 +16,8 @@
>> * key-vals = [ key-val { ',' key-val } [ ',' ] ]
>> * key-val = key '=' val
>> * key = key-fragment { '.' key-fragment }
>> - * key-fragment = / [^=,.]* /
>> - * val = { / [^,]* / | ',,' }
>> + * key-fragment = / [^=,.]+ /
>
> This requires a non-empty string. Good (we don't allow an empty key).
>
>> + * val = { / [^,]+ / | ',,' }
>
> I agree that this has no real change. Previously, you allowed zero or
> more repetitions of a regex that could produce zero characters; now,
> each outer repetition must make progress.
>
>> *
>> * Semantics defined by reduction to JSON:
>> *
>> @@ -71,12 +71,16 @@
>> * Awkward. Note that we carefully restrict alternate types to avoid
>> * similar ambiguity.
>> *
>> - * Additional syntax for use with an implied key:
>> + * Alternative syntax for use with an implied key:
>> *
>> - * key-vals-ik = val-no-key [ ',' key-vals ]
>> - * val-no-key = / [^=,]* /
>> + * key-vals = [ key-val-1st { ',' key-val } [ ',' ] ]
>> + * key-val-1st = val-no-key | key-val
>> + * val-no-key = / [^=,]+ /
>> *
>> - * where no-key is syntactic sugar for implied-key=val-no-key.
>> + * where val-no-key is syntactic sugar for implied-key=val-no-key.
>> + *
>> + * Note that you can't use the sugared form when the value contains
>> + * '=' or ','.
>
> Nor can you use the sugared form when the value is intended to be
> empty
True. Spelling it out wouldn't hurt. Takes a follow-up patch; this one
is already in master.
> (although this may be academic, as your other patches enumerate
> the list of clients, and none of them seem to allow an empty value
> even when desugared).
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
- [PATCH v4 0/7] qemu-storage-daemon: Remove QemuOpts from --object, Markus Armbruster, 2020/10/11
- [PATCH v4 3/7] keyval: Fix parsing of ',' in value of implied key, Markus Armbruster, 2020/10/11
- [PATCH v4 1/7] keyval: Fix and clarify grammar, Markus Armbruster, 2020/10/11
- [PATCH v4 2/7] test-keyval: Demonstrate misparse of ', ' with implied key, Markus Armbruster, 2020/10/11
- [PATCH v4 6/7] qom: Add user_creatable_print_help_from_qdict(), Markus Armbruster, 2020/10/11
- [PATCH v4 7/7] qemu-storage-daemon: Remove QemuOpts from --object parser, Markus Armbruster, 2020/10/11
- [PATCH v4 5/7] qom: Factor out helpers from user_creatable_print_help(), Markus Armbruster, 2020/10/11
- [PATCH v4 4/7] keyval: Parse help options, Markus Armbruster, 2020/10/11