* 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.
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.
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;
}
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;
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);
}
}
}
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;
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;
}
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;
{
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;
}
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;
}
}
/* 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);
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
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;
}
}
}
/* 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;
}
/* 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.
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
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:
void
free_inheritable_handles (struct inheritable_handles *inh_handles)
{
- free (inh_handles->flags);
- free (inh_handles->handles);
+ free (inh_handles->ih);
}
int
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> */
_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
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