Re: [Qemu-devel] [PATCH v2 33/37] tests: add qtest_add_data_func_full

[Eric Blake]

On 07/29/2016 02:48 AM, Marc-André Lureau wrote:

>>> +#if GLIB_CHECK_VERSION(2, 34, 0)
>>> +    gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
>>> +    g_test_add_data_func_full(path, data, fn, data_free_func);
>>> +    g_free(path);
>>> +#else
>>> +    qtest_add_data_func(str, data, fn);
>>> +#endif
>>
>> The commit message doesn't mention that the code is dependent on glib
>> versions, nor that you are still leaking the data (data_free_func
>> remains uncalled) on older glib.  If it is intentional (under the
>> argument that "anyone running on older glib can't care too much about
>> memory leaks encountered only by the testsuite, and the leaks don't
>> affect main qemu"), then stating that in the commit message would let me
>> feel more comfortable giving an R-b.
> 
> ok
> 
>> Is there anything we can do even in older glib to unconditionally invoke
>> the cleanup function in the right places?
> 
> Yes, calling the undocumented g_test_add_vtable(), with some casts. Is
> that acceptable?

Since the older versions aren't changing, then yes, I would view
back-compat casts along with a note saying "remove this once we can
require new-enough glib" as acceptable.

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

signature.asc
Description: OpenPGP digital signature