GNOME Bugzilla – Bug 657593
g_test_trap_fork calls close(-1)
Last modified: 2011-08-29 19:21:43 UTC
Created attachment 195060 [details] [review] proposed patch g_test_trap_fork() calls close() with argument -1 which is an invalid file descriptor, leading to errno set to EBDAF and complaints from valgrind (this part bothers me). A patch is attached, closing the fds only if not already closed and set to -1.
Review of attachment 195060 [details] [review]: Looks fine. But there's a ton of close() calls in gtestutils.c - have you checked that all the others are properly protected ?
+ if (stdout_pipe[0] > -1) Shouldn't this test for != instead of > in light of bug 638768 ?
I can see close() calls only in g_test_trap_fork(); these seem all right to me (and the patch at least fixes all calls I can actually hit in my test suite). I'll update the patch to check for equallity. However, I am not sure if other code, such as if (stdout_pipe[1] >= 3) close (stdout_pipe[1]); is correct, considering bug 638768.
Created attachment 195075 [details] [review] proposed patch Use != 1 instead of > -1 for fd validity testing.
Review of attachment 195075 [details] [review]: This patch needs a commit message. See http://live.gnome.org/GnomeLove/SubmittingPatches
Colin, I do not use git. I do not understand git. I hate git. This is just a patch; I have not committed anything anywhere. Thank you for understanding.
The following fix has been pushed: dc38967 Don't call close on invalid fds
Created attachment 195117 [details] [review] Don't call close on invalid fds If an fd is -1, don't call close() on it, since that leads to EBDAF and complaints from valgrind.