Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 05/04/2017 03:06 AM, Markus Armbruster wrote:
>> Eduardo Habkost <address@hidden> writes:
>> 
>>> On Wed, May 03, 2017 at 06:07:43PM +0200, Markus Armbruster wrote:
>>>> Eduardo Habkost <address@hidden> writes:
>>>>
>>>>> When parsing alternates from a string, there are some limitations in
>>>>> what we can do, but it is a valid use case in some situations. We can
>>>>> support booleans, integer types, and enums.
>> 
>> By the way, the same restrictions apply to the "keyval" variant of the
>> QObject input visitor.  It's a known problem stated here:
>> 
>>     Message-ID: <address@hidden>
>>     https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00046.html
>> 
>> However, I failed to document it properly in the source.
>> 
>
>>>> Begs the question what happens when you violate these restrictions.
>>>
>>> Right now, we don't detect those cases and behavior is undefined.
>>> I think it will be a good idea to give start_alternate() enough
>>> information to detect those cases (by adding a 'const char *const
>>> enum_table[]' parameter).
>> 
>> Alternate types that won't work with the string input visitor can be
>> detected at compile time (by qapi.py), but not their actual use.  Pity.
>> 
>> Do we actually use alternates that violate the restrictions?  If not, we
>> could simply restrict alternates so they work with *all* visitors.  If
>> we ever run into an actual need for alternates that don't, we'll be no
>> worse off than now.
>> 
>> Let's review existing alternates outside tests:
>> 
>> * Qcow2OverlapChecks: struct + enum
>> * BlockdevRef: struct + str
>> * GuestFileWhence: int + enum (all enum members start with a letter)
>> 
>> Restricting alternates looks practical to me.  Eric, what do you think?
>
> As in: we forbid the combination of a scalar (whether 'int', 'number',
> 'bool', and perhaps 'null') with a plain 'str' (since there's no way to
> tell whether '1' should parse as an integer or the string "1"); and
> combining a scalar with an 'enum' requires that all enum members be
> distinct from what could otherwise be parsed as a scalar?

Exactly.

>                                                            I can live
> with such a restriction.

Then let's do it.

Eduardo, are you comfortable implementing this, or would you like me to
do it?