Re: [Qemu-devel] [PATCH v15 15/23] qmp: Support explicit null during visits

[Eric Blake]

On 04/28/2016 10:50 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> Implement the new type_null() callback for the qmp input and
>> output visitors. While we don't yet have a use for this in QAPI
>> input (the generator will need some tweaks first), some
>> potential usages have already been discussed on the list.
>> Meanwhile, the output visitor could already output explicit null
>> via type_any, but this gives us finer control.
>>
>> At any rate, it's easy to test that we can round-trip an explicit
>> null through manual use of visit_type_null() wrapped by a virtual
>> visit_start_struct() walk, even if we can't do the visit in a
>> QAPI type.  Repurpose the test_visitor_out_empty test,
>> particularly since a future patch will tighten semantics to
>> forbid use of qmp_output_get_qobject() without at least one
>> intervening visit_type_*.
>>
>> Signed-off-by: Eric Blake <address@hidden>
> [...]
>> diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
>> index c039806..3b6ae92 100644
>> --- a/tests/test-qmp-input-visitor.c
>> +++ b/tests/test-qmp-input-visitor.c
>> @@ -279,6 +279,30 @@ static void test_visitor_in_any(TestInputVisitorData 
>> *data,
>>      qobject_decref(res);
>>  }
>>
>> +static void test_visitor_in_null(TestInputVisitorData *data,
>> +                                 const void *unused)
>> +{
>> +    Visitor *v;
>> +    Error *err = NULL;
>> +    char *tmp;
>> +
>> +    /*
>> +     * FIXME: Since QAPI doesn't know the 'null' type yet, we can't
>> +     * test visit_type_null() by reading into a QAPI struct then
>> +     * checking that it was populated correctly.  The best we can do
>> +     * for now is ensure that we consumed null from the input, proven
>> +     * by the fact that we can't re-read the key.
>> +     */
>> +
>> +    v = visitor_input_test_init(data, "{ 'a': null }");
>> +    visit_start_struct(v, NULL, NULL, 0, &error_abort);
>> +    visit_type_null(v, "a", &error_abort);
>> +    visit_type_str(v, "a", &tmp, &err);
>> +    g_assert(err);
> 
> I'd prefer error_free_or_abort().  Can do on commit.

Not just prefer, but needed to avoid a memleak.  Thanks for the good catch.

Also, I realized I don't do any negative test.  Maybe this test could be
expanded as:

v = visitor_input_test_init(data, "{ 'a': null, 'b': '' }");
visit_start_struct(v, NULL, NULL, 0, &error_abort);
visit_type_null(v, "a", &error_abort);
visit_type_null(v, "a", &tmp, &err);
error_free_or_abort();
visit_type_null(v, "b", &err);
error_free_or_abort();

> 
>> +    g_assert(!tmp);
>> +    visit_end_struct(v, &error_abort);
>> +}
>> +
>>  static void test_visitor_in_union_flat(TestInputVisitorData *data,
>>                                         const void *unused)
>>  {
> [...]
> 

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

signature.asc
Description: OpenPGP digital signature