From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42811) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4EF4-0004k2-BL for qemu-devel@nongnu.org; Wed, 02 Dec 2015 15:43:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4EF0-00056J-4Z for qemu-devel@nongnu.org; Wed, 02 Dec 2015 15:43:10 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50493) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4EEz-00055s-TU for qemu-devel@nongnu.org; Wed, 02 Dec 2015 15:43:06 -0500 References: <1449087634-30846-1-git-send-email-armbru@redhat.com> <1449087634-30846-2-git-send-email-armbru@redhat.com> From: Eric Blake Message-ID: <565F57D3.2000803@redhat.com> Date: Wed, 2 Dec 2015 13:42:59 -0700 MIME-Version: 1.0 In-Reply-To: <1449087634-30846-2-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="NNv6cqn0OPDKaAUT43lp2DwGw0h4tqluS" Subject: Re: [Qemu-devel] [PATCH for-2.5 1/2] tests: Use proper functions types instead of void (*fn) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: marcandre.lureau@redhat.com, afaerber@suse.de, peter.maydell@linaro.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --NNv6cqn0OPDKaAUT43lp2DwGw0h4tqluS Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12/02/2015 01:20 PM, Markus Armbruster wrote: > We have several function parameters declared as void (*fn). This is > just a stupid way to write void *, and the only purpose writing it > like that could serve is obscuring the sin of bypassing the type > system without need. Presumably, someone meant to write 'void (*fn)()' or some other argument list to designate fn as a function pointer; but I agree with the approach you took of using typedefs rather than spelling it out raw. >=20 > The original sin is commit 49ee359: its qtest_add_func() is a wrapper > for g_test_add_func(). Fix the parameter type to match > g_test_add_func()'s. This uncovers type errors in ide-test.c; fix > them. >=20 > Commit 7949c0e faithfully repeated the sin for qtest_add_data_func(). > Fix it the same way, along with a harmless type error uncovered in > vhost-user-test.c. >=20 > Commit 063c23d repeated it for qtest_add_abrt_handler(). The screwy > parameter gets assigned to GHook member func, so change its type to > match. Requires wrapping kill_qemu() to keep the type checker happy. >=20 > Signed-off-by: Markus Armbruster > --- > tests/ide-test.c | 4 ++-- > tests/libqtest.c | 13 +++++++++---- > tests/libqtest.h | 6 +++--- > tests/vhost-user-test.c | 3 ++- > 4 files changed, 16 insertions(+), 10 deletions(-) >=20 > +++ b/tests/libqtest.c > @@ -110,6 +110,11 @@ static void kill_qemu(QTestState *s) > } > } > =20 > +static void kill_qemu_hook_func(void *s) > +{ > + kill_qemu(s); Getting the implicit conversion from void * to QTestState *, without having to cast the actual function signature. Makes sense. > @@ -133,7 +138,7 @@ static void cleanup_sigabrt_handler(void) > sigaction(SIGABRT, &sigact_old, NULL); > } > =20 > -void qtest_add_abrt_handler(void (*fn), const void *data) > +void qtest_add_abrt_handler(GHookFunc fn, const void *data) I had to look it up: /usr/include/glib-2.0/glib/ghook.h:typedef void (*GHookFunc)=09 (gpointer data); which is the same as void (*fn)(void *) > @@ -755,14 +760,14 @@ void qtest_memread(QTestState *s, uint64_t addr, = void *data, size_t size) > g_strfreev(args); > } > =20 > -void qtest_add_func(const char *str, void (*fn)) > +void qtest_add_func(const char *str, GTestFunc fn) > { > gchar *path =3D g_strdup_printf("/%s/%s", qtest_get_arch(), str); > g_test_add_func(path, fn); /usr/include/glib-2.0/glib/gtestutils.h:typedef void (*GTestFunc) (void); > g_free(path); > } > =20 > -void qtest_add_data_func(const char *str, const void *data, void (*fn)= ) > +void qtest_add_data_func(const char *str, const void *data, GTestDataF= unc fn) /usr/include/glib-2.0/glib/gtestutils.h:typedef void (*GTestDataFunc) (gconstpointer user_data); > +++ b/tests/vhost-user-test.c > @@ -172,8 +172,9 @@ static void wait_for_fds(TestServer *s) > g_mutex_unlock(&s->data_mutex); > } > =20 > -static void read_guest_mem(TestServer *s) > +static void read_guest_mem(const void *data) > { > + TestServer *s =3D (void *)data; And here you have to cast away const. At any rate, all the fixes look sane, and the fact that it compiles with now-tighter types is in its favor. Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --NNv6cqn0OPDKaAUT43lp2DwGw0h4tqluS 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/ iQEcBAEBCAAGBQJWX1fTAAoJEKeha0olJ0Nq+9IH/RNhB6I1UrPBj9DcknbyIpkh H/nfuKJuLTFMhZ3qsUj3oMNLxWXbbtpoGAudxGnMzXbLfWmBuBzAP/2T9SYpI7Be lbSUaak4DzRYzFQV9Z5BrlKjek/brpkgu7pml15agGtMSXCri+8cRJwhdkBlfGPU yBdneMOnz/1Jy9p2dBSk5ajBMKlJaVMRrPM/y8ERVv3mf8Z05Fd7YbJXgOgJvDul GX36v759/4aiZDYPMTsBFoXkqVbSbvHnw8MwswX++grVJ9v3vS97jUpPh5dGwbWu Mc2zzGinHhd+tqbr1IGLVevaNzz14upghGddrK4M506w7E6Jcn1U/43yfFNDVZQ= =fC1n -----END PGP SIGNATURE----- --NNv6cqn0OPDKaAUT43lp2DwGw0h4tqluS--