GNOME Bugzilla – Bug 704447
Fix build/use of g_child_watch_closure_callback on Windows
Last modified: 2013-07-21 19:53:34 UTC
Hi, The recent commit 72a7e824 added g_child_watch_closure_callback in GObject that made use of a GPid that is acquired during the spawn of a child process. Unfortunately, the GPid type is a HANDLE (a.k.a a void *) rather than an int type on Windows, which would cause the build to break and perhaps bring undesirable results on Windows if one attempts to use that variable directly with a cast. With blessings.
Created attachment 249478 [details] [review] Fix the build [v1]: Handle pid accordingly on Windows
Created attachment 249479 [details] [review] Fix the build[v2]: Acquire the proper process ID first on Windows
Hi, I have came up with 2 proposed patches to attempt to tackle the issue, namely by: -Adding pid as a generic pointer type to the param GValue array on Windows. -Acquiring the proper process ID using GetProcessId on Windows before adding it to the param GValue array As I was having trouble to get any of Dan's test on gobject/closure to pass (including these I think have nothing related to my changes), I can't tell which is the better solution overall. With blessings, thank you for your time!
Comment on attachment 249479 [details] [review] Fix the build[v2]: Acquire the proper process ID first on Windows this one is wrong; the arguments shouldn't change just because you're using a closure
Comment on attachment 249478 [details] [review] Fix the build [v1]: Handle pid accordingly on Windows This one looks right, although it would be nice if we could get the test passing too so we can verify that... How does it fail?
Created attachment 249496 [details] [review] gobject/tests/closure: fix on win32 (The g_closure_unref() was wrong, but was not causing errors on linux for some reason.) ==== ok, with this patch it works for me under wine
Review of attachment 249496 [details] [review]: Hi Dan, Thanks for the quick review and feedback. With my first patch (that was preferred by you) and your patch to the test program, the tests pass on my Windows 8 system when things are built with Visual Studio 2008 x86 and x64 flavors. With blessings.
Created attachment 249507 [details] [review] gsourceclosure.c: Fix on Windows Hi, I am reattaching the first patch as the commit comments in there are a bit wrong as I typed that up there too quickly :) By the way, the same Visual Studio 2008 builds also passed as x64/x86 binaries on my Windows 7 x64 system and as a x86 binary in the XPMode of my Windows 7 system, for further references. With blessings, thank you!
Pushed both of these, however the test still fails for me; I'll reopen bug 704267.