Re: [PATCH 2/2] test-keyval: Tighten test of trailing crap after size

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 11/25/19 7:38 AM, Markus Armbruster wrote:
>> test_keyval_visit_size() should test for trailing crap after size with
>> and without suffix.  It does test the latter: "sz2=16Gi" has size
>> "16G" followed by crap "i".  It fails to test the former "sz1=16E" is
>> a syntactically valid size that overflows uint64_t.  Replace by
>> "sz1=0Z".
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>   tests/test-keyval.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/test-keyval.c b/tests/test-keyval.c
>> index 09b0ae3c68..e331a84149 100644
>> --- a/tests/test-keyval.c
>> +++ b/tests/test-keyval.c
>> @@ -478,7 +478,7 @@ static void test_keyval_visit_size(void)
>>       visit_free(v);
>>         /* Trailing crap */
>> -    qdict = keyval_parse("sz1=16E,sz2=16Gi", NULL, &error_abort);
>> +    qdict = keyval_parse("sz1=0Z,sz2=16Gi", NULL, &error_abort);
>
> Does this actually test both failure cases, or does it abort the parse
> after the first failure (sz1=0Z) without ever hitting the second half
> of the parse (sz2=16Gi)?

Fair question!  Short answer: yes, we check both.

Long answer follows.

       /* Trailing crap */
       qdict = keyval_parse("time1=89ks,time2=ns", NULL, &error_abort);

keyval_parse() must succeed: it takes &error_abort.

        v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
        qobject_unref(qdict);

Can't fail.

        visit_start_struct(v, NULL, NULL, 0, &error_abort);

Must succeed.

        visit_type_size(v, "sz1", &sz, &err);
        error_free_or_abort(&err);

This is where we parse "0Z".  Must fail.  We continue.

        visit_type_size(v, "sz2", &sz, &err);
        error_free_or_abort(&err);

This is where we parse "16Gi".  Must fail.  We continue.

        visit_end_struct(v, NULL);
        visit_free(v);
    }

Clear now?