Hi
On Fri, Jul 29, 2016 at 2:16 AM, Eric Blake <ebl...@redhat.com> wrote:
> On 07/28/2016 08:38 AM, marcandre.lur...@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lur...@redhat.com>
>>
>> Allows to specify a destroy function for the test data.
>
> "Allows to" is not idiomatic English. Alternatives that sound better are
> "Allows $who to specify" (most simply, "Allows one to"), or "Allows
> specifying"
>
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>> ---
>> tests/libqtest.c | 15 ++++++++++++++-
>> tests/libqtest.h | 7 ++++++-
>> 2 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>> index eb00f13..6ec56a6 100644
>> --- a/tests/libqtest.c
>> +++ b/tests/libqtest.c
>> @@ -758,8 +758,21 @@ void qtest_add_func(const char *str, void (*fn)(void))
>> g_free(path);
>> }
>>
>> +void qtest_add_data_func_full(const char *str, void *data,
>> + void (*fn)(const void *),
>> + GDestroyNotify data_free_func)
>> +{
>> +#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?
--
Marc-André Lureau
