Re: [PATCH 05/25] keyval: simplify keyval_parse_one

[Paolo Bonzini]

On 22/01/21 14:48, Markus Armbruster wrote:
>    --nbd .key=
>
>      master:       Invalid parameter '..key'
>      your patch:   same
>
>      Likweise.
>
>    If I omit the '=', your patch's message changes to
>
>                    No implicit parameter name for value 'key..'
>
>    I consider that worse than before, because it's talking about
>    something outside the user's control (lack of an implict parameter
>    name) where it should instead tell the user what needs fixing in the
>    input.

I think whether it's better or worse depends on the specific erroneous 
command line (think "--nbd /path/to/file.qcow2"), but I can certainly 
change it.

> Your patch also adds an "Expected parameter at end of string" error.
> Can you tell me how to trigger it?

It is meant for "--nbd ''" but it is effectively dead code due to the 
"while (*s)" in the caller.  Possibilities:

1) leave it in as dead code

2) replace it with an assert

3) change the caller to use a do...while in such a way that it triggers 
it (and be careful not to change the grammar).

> I believe your grammar is ambiguous.  Your code seems to pick the sane
> alternative.  If I'm wrong, you need to enlighten me.  If I'm right, you
> need to fix your grammar.

Will do.  Can I change the EBNF to use "+" and "*" for simplicity and 
clarity?

> Hmm, bartering opportunity...  May I have your support for me
> eliminating anti-social device names in exchange?  

I do not mind removing them.  What's the barter exactly like? :)

Paolo