]> Savannah Git Hosting - gnulib.git/commitdiff
posix_spawn-internal: Refactor.
authorBruno Haible <bruno@clisp.org>
Sat, 10 Sep 2022 00:30:14 +0000 (02:30 +0200)
committerBruno Haible <bruno@clisp.org>
Sat, 10 Sep 2022 00:30:14 +0000 (02:30 +0200)
* lib/windows-spawn.h (struct IHANDLE): New type.
(struct inheritable_handles): Combine handles and flags into a single
array.
* lib/windows-spawn.c (init_inheritable_handles, compose_handles_block,
spawnpvech): Update.
* lib/spawni.c (grow_inheritable_handles, shrink_inheritable_handles,
do_open, do_dup2, do_close): Update.

ChangeLog
lib/spawni.c
lib/windows-spawn.c
lib/windows-spawn.h

index 93ec3d78f2132a41e11e825e7549fd12fa484fe9..0a11fb895c40ff3be2ba8a4e8b620d3b403562eb 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2022-09-09  Bruno Haible  <bruno@clisp.org>
 
+       posix_spawn-internal: Refactor.
+       * lib/windows-spawn.h (struct IHANDLE): New type.
+       (struct inheritable_handles): Combine handles and flags into a single
+       array.
+       * lib/windows-spawn.c (init_inheritable_handles, compose_handles_block,
+       spawnpvech): Update.
+       * lib/spawni.c (grow_inheritable_handles, shrink_inheritable_handles,
+       do_open, do_dup2, do_close): Update.
+
        posix_spawn-internal: Optimize DuplicateHandle calls on native Windows.
        * lib/spawni.c (open_handle): Return an inheritable HANDLE.
        (do_open): Don't call DuplicateHandle. Remove curr_process parameter.
index 1850c7ddd5d0e599c7840f3f4cc6e7afd1d90912..ca99c5bff14361229897438351c92384268742bc 100644 (file)
@@ -121,32 +121,22 @@ grow_inheritable_handles (struct inheritable_handles *inh_handles, int newfd)
       size_t new_allocated = 2 * inh_handles->allocated + 1;
       if (new_allocated <= newfd)
         new_allocated = newfd + 1;
-      HANDLE *new_handles_array =
-        (HANDLE *)
-        realloc (inh_handles->handles, new_allocated * sizeof (HANDLE));
-      if (new_handles_array == NULL)
+      struct IHANDLE *new_ih =
+        (struct IHANDLE *)
+        realloc (inh_handles->ih, new_allocated * sizeof (struct IHANDLE));
+      if (new_ih == NULL)
         {
           errno = ENOMEM;
           return -1;
         }
-      unsigned short *new_flags_array =
-        (unsigned short *)
-        realloc (inh_handles->flags, new_allocated * sizeof (unsigned short));
-      if (new_flags_array == NULL)
-        {
-          free (new_handles_array);
-          errno = ENOMEM;
-          return -1;
-        }
       inh_handles->allocated = new_allocated;
-      inh_handles->handles = new_handles_array;
-      inh_handles->flags = new_flags_array;
+      inh_handles->ih = new_ih;
     }
 
-  HANDLE *handles = inh_handles->handles;
+  struct IHANDLE *ih = inh_handles->ih;
 
   for (; inh_handles->count <= newfd; inh_handles->count++)
-    handles[inh_handles->count] = INVALID_HANDLE_VALUE;
+    ih[inh_handles->count].handle = INVALID_HANDLE_VALUE;
 
   return 0;
 }
@@ -156,26 +146,25 @@ grow_inheritable_handles (struct inheritable_handles *inh_handles, int newfd)
 static void
 shrink_inheritable_handles (struct inheritable_handles *inh_handles)
 {
-  HANDLE *handles = inh_handles->handles;
-  unsigned short *flags = inh_handles->flags;
+  struct IHANDLE *ih = inh_handles->ih;
   size_t handles_count = inh_handles->count;
   unsigned int fd;
 
   for (fd = 0; fd < handles_count; fd++)
     {
-      HANDLE handle = handles[fd];
+      HANDLE handle = ih[fd].handle;
 
       if (handle != INVALID_HANDLE_VALUE
-          && (flags[fd] & KEEP_OPEN_IN_CHILD) == 0)
+          && (ih[fd].flags & KEEP_OPEN_IN_CHILD) == 0)
         {
-          if (!(flags[fd] & KEEP_OPEN_IN_PARENT))
+          if (!(ih[fd].flags & KEEP_OPEN_IN_PARENT))
             CloseHandle (handle);
-          handles[fd] = INVALID_HANDLE_VALUE;
+          ih[fd].handle = INVALID_HANDLE_VALUE;
         }
     }
 
   while (handles_count > 3
-         && handles[handles_count - 1] == INVALID_HANDLE_VALUE)
+         && ih[handles_count - 1].handle == INVALID_HANDLE_VALUE)
     handles_count--;
 
   inh_handles->count = handles_count;
@@ -185,17 +174,16 @@ shrink_inheritable_handles (struct inheritable_handles *inh_handles)
 static void
 close_inheritable_handles (struct inheritable_handles *inh_handles)
 {
-  HANDLE *handles = inh_handles->handles;
-  unsigned short *flags = inh_handles->flags;
+  struct IHANDLE *ih = inh_handles->ih;
   size_t handles_count = inh_handles->count;
   unsigned int fd;
 
   for (fd = 0; fd < handles_count; fd++)
     {
-      HANDLE handle = handles[fd];
+      HANDLE handle = ih[fd].handle;
 
       if (handle != INVALID_HANDLE_VALUE
-          && !(flags[fd] & KEEP_OPEN_IN_PARENT))
+          && !(ih[fd].flags & KEEP_OPEN_IN_PARENT))
         CloseHandle (handle);
     }
 }
@@ -397,9 +385,9 @@ do_open (struct inheritable_handles *inh_handles, int newfd,
     }
   if (grow_inheritable_handles (inh_handles, newfd) < 0)
     return -1;
-  if (inh_handles->handles[newfd] != INVALID_HANDLE_VALUE
-      && !(inh_handles->flags[newfd] & KEEP_OPEN_IN_PARENT)
-      && !CloseHandle (inh_handles->handles[newfd]))
+  if (inh_handles->ih[newfd].handle != INVALID_HANDLE_VALUE
+      && !(inh_handles->ih[newfd].flags & KEEP_OPEN_IN_PARENT)
+      && !CloseHandle (inh_handles->ih[newfd].handle))
     {
       errno = EIO;
       return -1;
@@ -428,8 +416,8 @@ do_open (struct inheritable_handles *inh_handles, int newfd,
       return -1;
     }
   free (filename_to_free);
-  inh_handles->handles[newfd] = handle;
-  inh_handles->flags[newfd] =
+  inh_handles->ih[newfd].handle = handle;
+  inh_handles->ih[newfd].flags =
     ((flags & O_APPEND) != 0 ? 32 : 0) | KEEP_OPEN_IN_CHILD;
   return 0;
 }
@@ -442,7 +430,7 @@ do_dup2 (struct inheritable_handles *inh_handles, int oldfd, int newfd,
          HANDLE curr_process)
 {
   if (!(oldfd >= 0 && oldfd < inh_handles->count
-        && inh_handles->handles[oldfd] != INVALID_HANDLE_VALUE))
+        && inh_handles->ih[oldfd].handle != INVALID_HANDLE_VALUE))
     {
       errno = EBADF;
       return -1;
@@ -456,24 +444,24 @@ do_dup2 (struct inheritable_handles *inh_handles, int oldfd, int newfd,
     {
       if (grow_inheritable_handles (inh_handles, newfd) < 0)
         return -1;
-      if (inh_handles->handles[newfd] != INVALID_HANDLE_VALUE
-          && !(inh_handles->flags[newfd] & KEEP_OPEN_IN_PARENT)
-          && !CloseHandle (inh_handles->handles[newfd]))
+      if (inh_handles->ih[newfd].handle != INVALID_HANDLE_VALUE
+          && !(inh_handles->ih[newfd].flags & KEEP_OPEN_IN_PARENT)
+          && !CloseHandle (inh_handles->ih[newfd].handle))
         {
           errno = EIO;
           return -1;
         }
       /* Duplicate the handle, so that a forthcoming do_close action on oldfd
          has no effect on newfd.  */
-      if (!DuplicateHandle (curr_process, inh_handles->handles[oldfd],
-                            curr_process, &inh_handles->handles[newfd],
+      if (!DuplicateHandle (curr_process, inh_handles->ih[oldfd].handle,
+                            curr_process, &inh_handles->ih[newfd].handle,
                             0, TRUE, DUPLICATE_SAME_ACCESS))
         {
           errno = EBADF; /* arbitrary */
           return -1;
         }
-      inh_handles->flags[newfd] =
-        (unsigned char) inh_handles->flags[oldfd] | KEEP_OPEN_IN_CHILD;
+      inh_handles->ih[newfd].flags =
+        (unsigned char) inh_handles->ih[oldfd].flags | KEEP_OPEN_IN_CHILD;
     }
   return 0;
 }
@@ -485,18 +473,18 @@ static int
 do_close (struct inheritable_handles *inh_handles, int fd)
 {
   if (!(fd >= 0 && fd < inh_handles->count
-        && inh_handles->handles[fd] != INVALID_HANDLE_VALUE))
+        && inh_handles->ih[fd].handle != INVALID_HANDLE_VALUE))
     {
       errno = EBADF;
       return -1;
     }
-  if (!(inh_handles->flags[fd] & KEEP_OPEN_IN_PARENT)
-      && !CloseHandle (inh_handles->handles[fd]))
+  if (!(inh_handles->ih[fd].flags & KEEP_OPEN_IN_PARENT)
+      && !CloseHandle (inh_handles->ih[fd].handle))
     {
       errno = EIO;
       return -1;
     }
-  inh_handles->handles[fd] = INVALID_HANDLE_VALUE;
+  inh_handles->ih[fd].handle = INVALID_HANDLE_VALUE;
   return 0;
 }
 
index d38d1dc911173865841f1df94a3711a714207338..e81fc168cf2bdf2be355d80b1b1bc2cdc7d26628 100644 (file)
@@ -346,31 +346,23 @@ init_inheritable_handles (struct inheritable_handles *inh_handles,
   }
   /* Note: handles_count >= 3.  */
 
-  /* Allocate the arrays.  */
+  /* Allocate the array.  */
   size_t handles_allocated = handles_count;
-  HANDLE *handles_array =
-    (HANDLE *) malloc (handles_allocated * sizeof (HANDLE));
-  if (handles_array == NULL)
+  struct IHANDLE *ih =
+    (struct IHANDLE *) malloc (handles_allocated * sizeof (struct IHANDLE));
+  if (ih == NULL)
     {
       errno = ENOMEM;
       return -1;
     }
-  unsigned short *flags_array =
-    (unsigned short *) malloc (handles_allocated * sizeof (unsigned short));
-  if (flags_array == NULL)
-    {
-      free (handles_array);
-      errno = ENOMEM;
-      return -1;
-    }
 
-  /* Fill in the two arrays.  */
+  /* Fill in the array.  */
   {
     HANDLE curr_process = (duplicate ? GetCurrentProcess () : INVALID_HANDLE_VALUE);
     unsigned int fd;
     for (fd = 0; fd < handles_count; fd++)
       {
-        handles_array[fd] = INVALID_HANDLE_VALUE;
+        ih[fd].handle = INVALID_HANDLE_VALUE;
         /* _get_osfhandle
            <https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle>  */
         HANDLE handle = (HANDLE) _get_osfhandle (fd);
@@ -388,26 +380,25 @@ init_inheritable_handles (struct inheritable_handles *inh_handles,
                     if ((hflags & HANDLE_FLAG_INHERIT) != 0)
                       {
                         /* Instead of duplicating it, just mark it as shared.  */
-                        handles_array[fd] = handle;
-                        flags_array[fd] = KEEP_OPEN_IN_PARENT | KEEP_OPEN_IN_CHILD;
+                        ih[fd].handle = handle;
+                        ih[fd].flags = KEEP_OPEN_IN_PARENT | KEEP_OPEN_IN_CHILD;
                       }
                     else
                       {
                         if (!DuplicateHandle (curr_process, handle,
-                                              curr_process, &handles_array[fd],
+                                              curr_process, &ih[fd].handle,
                                               0, TRUE, DUPLICATE_SAME_ACCESS))
                           {
                             unsigned int i;
                             for (i = 0; i < fd; i++)
-                              if (handles_array[i] != INVALID_HANDLE_VALUE
-                                  && !(flags_array[i] & KEEP_OPEN_IN_PARENT))
-                                CloseHandle (handles_array[i]);
-                            free (flags_array);
-                            free (handles_array);
+                              if (ih[i].handle != INVALID_HANDLE_VALUE
+                                  && !(ih[i].flags & KEEP_OPEN_IN_PARENT))
+                                CloseHandle (ih[i].handle);
+                            free (ih);
                             errno = EBADF; /* arbitrary */
                             return -1;
                           }
-                        flags_array[fd] = 0;
+                        ih[fd].flags = 0;
                       }
                   }
                 else
@@ -415,8 +406,8 @@ init_inheritable_handles (struct inheritable_handles *inh_handles,
                     if ((hflags & HANDLE_FLAG_INHERIT) != 0)
                       {
                         /* fd denotes an inheritable descriptor.  */
-                        handles_array[fd] = handle;
-                        flags_array[fd] = KEEP_OPEN_IN_CHILD;
+                        ih[fd].handle = handle;
+                        ih[fd].flags = KEEP_OPEN_IN_CHILD;
                       }
                   }
               }
@@ -427,8 +418,7 @@ init_inheritable_handles (struct inheritable_handles *inh_handles,
   /* Return the result.  */
   inh_handles->count = handles_count;
   inh_handles->allocated = handles_allocated;
-  inh_handles->handles = handles_array;
-  inh_handles->flags = flags_array;
+  inh_handles->ih = ih;
   return 0;
 }
 
@@ -439,9 +429,9 @@ compose_handles_block (const struct inheritable_handles *inh_handles,
   /* STARTUPINFO
      <https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/ns-processthreadsapi-startupinfoa>  */
   sinfo->dwFlags = STARTF_USESTDHANDLES;
-  sinfo->hStdInput  = inh_handles->handles[0];
-  sinfo->hStdOutput = inh_handles->handles[1];
-  sinfo->hStdError  = inh_handles->handles[2];
+  sinfo->hStdInput  = inh_handles->ih[0].handle;
+  sinfo->hStdOutput = inh_handles->ih[1].handle;
+  sinfo->hStdError  = inh_handles->ih[2].handle;
 
   /* On newer versions of Windows, more file descriptors / handles than the
      first three can be passed.
@@ -492,11 +482,11 @@ compose_handles_block (const struct inheritable_handles *inh_handles,
         handles_aligned[fd] = INVALID_HANDLE_VALUE;
         flags[fd] = 0;
 
-        HANDLE handle = inh_handles->handles[fd];
+        HANDLE handle = inh_handles->ih[fd].handle;
         if (handle != INVALID_HANDLE_VALUE
             /* The first three are possibly already passed above.
                But they need to passed here as well, if they have some flags.  */
-            && (fd >= 3 || (unsigned char) inh_handles->flags[fd] != 0))
+            && (fd >= 3 || (unsigned char) inh_handles->ih[fd].flags != 0))
           {
             DWORD hflags;
             /* GetHandleInformation
@@ -511,7 +501,7 @@ compose_handles_block (const struct inheritable_handles *inh_handles,
                        flags[fd] = 1.  But on ReactOS or Wine, adding the bit
                        that indicates the handle type may be necessary.  So,
                        just do it everywhere.  */
-                    flags[fd] = 1 | (unsigned char) inh_handles->flags[fd];
+                    flags[fd] = 1 | (unsigned char) inh_handles->ih[fd].flags;
                     switch (GetFileType (handle))
                       {
                       case FILE_TYPE_CHAR:
@@ -543,8 +533,7 @@ compose_handles_block (const struct inheritable_handles *inh_handles,
 void
 free_inheritable_handles (struct inheritable_handles *inh_handles)
 {
-  free (inh_handles->flags);
-  free (inh_handles->handles);
+  free (inh_handles->ih);
 }
 
 int
@@ -640,12 +629,12 @@ spawnpvech (int mode,
       errno = saved_errno;
       return -1;
     }
-  inh_handles.handles[0] = stdin_handle;
-  inh_handles.flags[0] = KEEP_OPEN_IN_CHILD;
-  inh_handles.handles[1] = stdout_handle;
-  inh_handles.flags[1] = KEEP_OPEN_IN_CHILD;
-  inh_handles.handles[2] = stderr_handle;
-  inh_handles.flags[2] = KEEP_OPEN_IN_CHILD;
+  inh_handles.ih[0].handle = stdin_handle;
+  inh_handles.ih[0].flags = KEEP_OPEN_IN_CHILD;
+  inh_handles.ih[1].handle = stdout_handle;
+  inh_handles.ih[1].flags = KEEP_OPEN_IN_CHILD;
+  inh_handles.ih[2].handle = stderr_handle;
+  inh_handles.ih[2].flags = KEEP_OPEN_IN_CHILD;
 
   /* CreateProcess
      <https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa>  */
index fdb45a4f1568237502a61f1e484e78c434a8905b..5cc770b59d5a35f1d3f9f8e1e0a0fb2321d41f51 100644 (file)
@@ -85,6 +85,24 @@ extern char * compose_envblock (const char * const *envp)
   _GL_ATTRIBUTE_MALLOC _GL_ATTRIBUTE_DEALLOC_FREE;
 
 
+/* An inheritable handle with some additional information.  */
+struct IHANDLE
+{
+  /* Either INVALID_HANDLE_VALUE or an inheritable handle.  */
+  HANDLE handle;
+  /* Only relevant if handle != INVALID_HANDLE_VALUE.
+     It is a bit mask consisting of:
+       - 32 for O_APPEND.
+       - KEEP_OPEN_IN_CHILD if the handle is scheduled to be preserved in the
+         child process.
+       - KEEP_OPEN_IN_PARENT if the handle is shared with (and needs to be kept
+         open in) the parent process.
+   */
+  unsigned short flags;
+  #define KEEP_OPEN_IN_CHILD 0x100
+  #define KEEP_OPEN_IN_PARENT 0x200
+};
+
 /* This struct keeps track of which handles to potentially pass to a subprocess,
    and with which flags.  All of the handles here are inheritable.
    Regarding handle inheritance, see
@@ -98,21 +116,8 @@ struct inheritable_handles
   size_t count;
   /* The number of allocated entries in the two arrays below.  */
   size_t allocated;
-  /* handles[0..count-1] are the occupied entries.
-     handles[fd] is either INVALID_HANDLE_VALUE or an inheritable handle.  */
-  HANDLE *handles;
-  /* flags[0..count-1] are the occupied entries.
-     flags[fd] is only relevant if handles[fd] != INVALID_HANDLE_VALUE.
-     It is a bit mask consisting of:
-       - 32 for O_APPEND.
-       - KEEP_OPEN_IN_CHILD if handles[fd] is scheduled to be preserved in the
-         child process.
-       - KEEP_OPEN_IN_PARENT if handles[fd] is shared with (and needs to be kept
-         open in) the parent process.
-   */
-  unsigned short *flags;
-  #define KEEP_OPEN_IN_CHILD 0x100
-  #define KEEP_OPEN_IN_PARENT 0x200
+  /* ih[0..count-1] are the occupied entries.  */
+  struct IHANDLE *ih;
 };
 
 /* Initializes a set of inheritable handles, filling in all or part of the