On 09/07/2016 01:57 PM, Marc-André Lureau wrote: >>> I'm afraid this doesn't build with our minimum glib version: >>> >>> /Users/pm215/src/qemu-for-merges/tests/libqtest.c:771:42: error: expected >>> ')' >>> (GTestFixtureFunc) fn, (GTestFixtureFunc) >>> data_free_func);
>>> The GTestFixtureFunc typedef was only introduced in glib 2.28, and our >>> minimum is 2.22. >> >> Argh,.. >> >>> >>> Also, g_test_add_vtable() in glib 2.22 has this prototype: >>> >>> void (*data_test) >>> (void), >>> void (*data_teardown) >>> (void)); >>> >>> but GTestFixtureFunc is typedefed in newer glib as >>> void (*GTestFixtureFunc) (gpointer fixture, gconstpointer user_data); So when glib prototyped it, they changed from (void) to (gpointer, gconstpointer) as the signature. Which means there's no real portable way to cast, other than to create a conditional #define typedef to the appropriate variant for the glib being targeted. >>> >>> so it looks like this function has changed signature somewhere >>> between glib versions, which makes me a bit nervous about using it. Do you use either 'fixture' or 'user_data' in the updated signature? If not, and the older signature of (void) matches how we are using things, then the casting just exists to shut up the compiler (and we are relying on the ABI, rather than C, to happen to do the right thing). If we ARE using either argument, then you'd have to refactor things to pass the arguments through a global rather than as a function parameter. >> >> Perhaps we should get back to the simpler version, only using >> g_test_add_data_func_full() with 2.34: >> https://patchwork.kernel.org/patch/9251373/ >> >> I can update the patch that way with a comment about expected leaks < 2.34. Living with leaks on older glib is starting to sound the most palatable; but it needs more comments than what was listed in that patchwork entry. >> > > Eric, since you suggested some compat code for older versions, would you be > fine with the above plan? I can send a new patch for review. I'm fine with the #if GLIB_CHECK_VERSION(2, 34, 0), but only if there is corresponding comments in both the code and the commit message that documents that we have exhausted all other reasonable possibilities of catering to older glib, and that it doesn't matter if the test leaks. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
