From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45614) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bStb8-0002Ap-ST for qemu-devel@nongnu.org; Thu, 28 Jul 2016 18:16:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bStb6-0006Tq-SJ for qemu-devel@nongnu.org; Thu, 28 Jul 2016 18:16:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43482) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bStb6-0006Tj-Kf for qemu-devel@nongnu.org; Thu, 28 Jul 2016 18:16:08 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 53F2185365 for ; Thu, 28 Jul 2016 22:16:08 +0000 (UTC) References: <20160728143808.13707-1-marcandre.lureau@redhat.com> <20160728143808.13707-34-marcandre.lureau@redhat.com> From: Eric Blake Message-ID: <579A8427.1060306@redhat.com> Date: Thu, 28 Jul 2016 16:16:07 -0600 MIME-Version: 1.0 In-Reply-To: <20160728143808.13707-34-marcandre.lureau@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="BJvXsJu7EAHjljmXuo87F6hvNbff2uCmm" Subject: Re: [Qemu-devel] [PATCH v2 33/37] tests: add qtest_add_data_func_full List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --BJvXsJu7EAHjljmXuo87F6hvNbff2uCmm From: Eric Blake To: marcandre.lureau@redhat.com, qemu-devel@nongnu.org Message-ID: <579A8427.1060306@redhat.com> Subject: Re: [PATCH v2 33/37] tests: add qtest_add_data_func_full References: <20160728143808.13707-1-marcandre.lureau@redhat.com> <20160728143808.13707-34-marcandre.lureau@redhat.com> In-Reply-To: <20160728143808.13707-34-marcandre.lureau@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/28/2016 08:38 AM, marcandre.lureau@redhat.com wrote: > From: Marc-Andr=C3=A9 Lureau >=20 > 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" >=20 > Signed-off-by: Marc-Andr=C3=A9 Lureau > --- > tests/libqtest.c | 15 ++++++++++++++- > tests/libqtest.h | 7 ++++++- > 2 files changed, 20 insertions(+), 2 deletions(-) >=20 > 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)(vo= id)) > g_free(path); > } > =20 > +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 =3D 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. Is there anything we can do even in older glib to unconditionally invoke the cleanup function in the right places? > + > void qtest_add_data_func(const char *str, const void *data, > - void (*fn)(const void *)) > + void (*fn)(const void *)) Why the spurious indentation change? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --BJvXsJu7EAHjljmXuo87F6hvNbff2uCmm Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXmoQnAAoJEKeha0olJ0NqEDQH/jvvGw7QScetShMFnLNb7acG idoU+U04WiJm/uunsVKwwr3jdGFFbFmhsWHLR7+Q0GnSkrnjc/rev/nruMmAiUtM pCc+0GXXRNlx2UsmUr5tDGwj/f1/LvYBFPJU7ubWM7rHHN38tq91pv8YftZRxVdR uNgK61lwI4jlmU7atlCbfTWFGQepAe/wrN/fVDzG6LFjtbN9RGL/CfGYWc+uwLoB +hehmUW6sk9UtL4n+jJs1+tbQJcK3C27wCmJWbR3P9fJLbo5uKZwZdmgdXsZg6FP q0vrejWIy1G4A8I/iPVLT1UwEPnTjjQ6Sb/kkPW70LV7f/qJ90D1pS7ZcHB/0xw= =s1ZW -----END PGP SIGNATURE----- --BJvXsJu7EAHjljmXuo87F6hvNbff2uCmm--