Re: [Qemu-devel] [PATCH v5 05/46] qapi: Test use of 'number' within alternates

[Eric Blake]

On 09/24/2015 10:29 AM, Markus Armbruster wrote:

>>>> +
>>>> +    /* FIXME: Order of alternate should not affect semantics */
>>>
>>> Inhowfar does it affect semantics?  Or asked differently: what exactly
>>> is wrong with this test now?
>>>
>>>> +    v = visitor_input_test_init(data, "42");
>>>> +    visit_type_AltThree(v, &three, NULL, &error_abort);
>>>> +    g_assert_cmpint(three->kind, ==, ALT_THREE_KIND_N);
>>>> +    g_assert_cmpfloat(three->n, ==, 42);
>>>> +    qapi_free_AltThree(three);
>>>> +    one = NULL;
>>
>>
>> AltTwo and AltThree are ostensibly the same struct (two branches, one
>> for 'str' and one for 'number', just in a different order), but they
>> parsed differently (AltTwo failed, AltThree succeeded).  The bug is
>> fixed later when the order of the branch declaration no longer impacts
>> the result of the parse.
> 
> Then nothing is wrong with this test case, and the FIXME doesn't belong
> here.

Actually, the test for AltThree succeeds only by accident. There are two
bugs at play; when I fix the first bug (order shouldn't matter: AltTwo
and AltThree should parse identically), then the second bug is finally
exposed (integers aren't being parsed as numbers, in either AltTwo or
AltThree). But I can certainly rework the FIXMEs both here and on the
first fix (19/46) to make it more obvious what the second fix (20/46) is
good for.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature