From 5fd3c63ae8ab3923fa7963832dadde1d065a1e48 Mon Sep 17 00:00:00 2001 From: Chun-wei Fan Date: Tue, 5 Nov 2013 15:51:08 +0800 Subject: [PATCH] glib/gspawn-win32-helper.c: Clean up a bit Remove the parts about storing up the fd's in a data structure, but call close() on the fd's. However, retain the _get_osfhandle() check on the fd's when we iterate through the fd's as on fd values in the iteration may well be invalid fd's. As a result, the invalid parameter handler is still needed for newer Microsoft CRTs (8.0/2005+) for _get_osfhandle() to make sure that the program does not abort when we check the validity of fd's to be closed in the loop[1]. [1]: http://msdn.microsoft.com/en-us/library/ks2530z6%28v=vs.80%29.aspx --- glib/gspawn-win32-helper.c | 47 +++++++++++++++----------------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/glib/gspawn-win32-helper.c b/glib/gspawn-win32-helper.c index 78969412d..136eb4a16 100644 --- a/glib/gspawn-win32-helper.c +++ b/glib/gspawn-win32-helper.c @@ -25,8 +25,13 @@ /* For _CrtSetReportMode, we don't want Windows CRT (2005 and later) * to terminate the process if a bad file descriptor is passed into - * _get_osfhandle. The newer MS CRT's are picky - * on double close()'s and bad file descriptors. + * _get_osfhandle(). This is necessary because we use _get_osfhandle() + * to check the validity of the fd before we try to call close() on + * it as attempting to close an invalid fd will cause the Windows CRT + * to abort() this program internally. + * + * Please see http://msdn.microsoft.com/zh-tw/library/ks2530z6%28v=vs.80%29.aspx + * for an explanation on this. */ #if (defined (_MSC_VER) && _MSC_VER >= 1400) #include @@ -161,13 +166,15 @@ protect_wargv (wchar_t **wargv, * This is the (empty) invalid parameter handler * that is used for Visual C++ 2005 (and later) builds * so that we can use this instead of the system automatically - * aborting the process, as the newer MS CRTs are more picky - * about double close()'s and bad/invalid file descriptors. + * aborting the process. * * This is necessary as we use _get_oshandle() to check the validity * of the file descriptors as we close them, so when an invalid file * descriptor is passed into that function as we check on it, we get * -1 as the result, instead of the gspawn helper program aborting. + * + * Please see http://msdn.microsoft.com/zh-tw/library/ks2530z6%28v=vs.80%29.aspx + * for an explanation on this. */ void myInvalidParameterHandler( const wchar_t * expression, @@ -208,10 +215,6 @@ main (int ignored_argc, char **ignored_argv) _startupinfo si = { 0 }; char c; - /* store up the file descriptors to close */ - GSList *fd_toclose = NULL; - GSList *last_item = NULL; - #if (defined (_MSC_VER) && _MSC_VER >= 1400) /* set up our empty invalid parameter handler */ _invalid_parameter_handler oldHandler, newHandler; @@ -267,7 +270,7 @@ main (int ignored_argc, char **ignored_argv) if (fd != 0) { dup2 (fd, 0); - fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd)); + close (fd); } } else @@ -276,7 +279,7 @@ main (int ignored_argc, char **ignored_argv) if (fd != 0) { dup2 (fd, 0); - fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd)); + close (fd); } } @@ -288,7 +291,7 @@ main (int ignored_argc, char **ignored_argv) if (fd != 1) { dup2 (fd, 1); - fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd)); + close (fd); } } else @@ -297,7 +300,7 @@ main (int ignored_argc, char **ignored_argv) if (fd != 1) { dup2 (fd, 1); - fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd)); + close (fd); } } @@ -309,7 +312,7 @@ main (int ignored_argc, char **ignored_argv) if (fd != 2) { dup2 (fd, 2); - fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd)); + close (fd); } } else @@ -318,7 +321,7 @@ main (int ignored_argc, char **ignored_argv) if (fd != 2) { dup2 (fd, 2); - fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd)); + close (fd); } } @@ -338,21 +341,7 @@ main (int ignored_argc, char **ignored_argv) for (i = 3; i < 1000; i++) /* FIXME real limit? */ if (i != child_err_report_fd && i != helper_sync_fd) if (_get_osfhandle (i) != -1) - fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (i)); - - /* ...so we won't get the nasty off-by-1 file descriptor leak */ - fd_toclose = g_slist_append (fd_toclose, NULL); - last_item = g_slist_last (fd_toclose); - - /* now close all the file descriptors as necessary */ - if (fd_toclose != NULL && last_item != NULL) - { - for ( ; fd_toclose != last_item; fd_toclose = fd_toclose->next) - close (GPOINTER_TO_INT(fd_toclose->data)); - } - g_slist_free (last_item); - g_slist_free (fd_toclose); - + close (i); /* We don't want our child to inherit the error report and * helper sync fds.