[glib] glib/gspawn-win32-helper.c: Clean up a bit

[Chun-wei Fan <fanchunwei src gnome org>]

commit 5fd3c63ae8ab3923fa7963832dadde1d065a1e48
Author: Chun-wei Fan <fanchunwei src gnome org>
Date:   Tue Nov 5 15:51:08 2013 +0800

    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 files changed, 18 insertions(+), 29 deletions(-)
---
diff --git a/glib/gspawn-win32-helper.c b/glib/gspawn-win32-helper.c
index 7896941..136eb4a 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 <crtdbg.h>
@@ -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.