[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 33/37] tests: add qtest_add_data_func_full
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH v2 33/37] tests: add qtest_add_data_func_full |
Date: |
Fri, 29 Jul 2016 12:48:27 +0400 |
Hi
On Fri, Jul 29, 2016 at 2:16 AM, Eric Blake <address@hidden> wrote:
> On 07/28/2016 08:38 AM, address@hidden wrote:
>> From: Marc-André Lureau <address@hidden>
>>
>> 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 <address@hidden>
>> ---
>> 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
- [Qemu-devel] [PATCH v2 26/37] qjson: free str, (continued)
- [Qemu-devel] [PATCH v2 26/37] qjson: free str, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 27/37] virtio-input: free config list, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 28/37] ipmi: free extern timer, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 29/37] usb: free USBDevice.strings, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 30/37] tests: free a bunch of qmp responses, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 31/37] usb: free leaking path, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 32/37] bus: simplify name handling, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 33/37] tests: add qtest_add_data_func_full, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 34/37] tests: pc-cpu-test leaks fixes, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 35/37] tests: fix rsp leak in postcopy-test, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 37/37] tests: fix postcopy-test leaks, marcandre . lureau, 2016/07/28
- [Qemu-devel] [PATCH v2 36/37] ahci: fix sglist leak on retry, marcandre . lureau, 2016/07/28
- Re: [Qemu-devel] [PATCH v2 00/37] Various memory leak fixes, Eric Blake, 2016/07/28