Hi Eric,
> First order of business - we need two modules based on the fcntl name, one
> for the header ... and one for the function.
Yes, absolutely.
Your 3 patches look all fine, except for the mingw replacement of fcntl().
+static int
+dupfd (int fd, int desired_fd)
+{
+ /* Although this looks like unbounded recursion, mingw only supports
+ a maximum of 2047, so stack overflow is unlikely. */
+ int duplicated_fd = dup (fd);
+ if (duplicated_fd < 0 || desired_fd <= duplicated_fd)
+ return duplicated_fd;
+ else
+ {
+ int r = dupfd (fd, desired_fd);
+ int e = errno;
+ close (duplicated_fd);
+ errno = e;
+ return r;
+ }
+}
I would not risk a recursion depth of 2047. It's time to make this function
non-recursive. How about this?
static int
dupfd (int fd, int desired_fd)
{
unsigned char fds_to_close[4096 / 8];
unsigned int fds_to_close_bound = 0;
int result;
/* Call dup(), keeping the returned fds open, until a result is
>= desired_fd. */
for (;;)
{
int duplicated_fd = dup (fd);
unsigned int index;
if (duplicated_fd < 0 || desired_fd <= duplicated_fd)
{
result = duplicated_fd;
break;
}
index = (unsigned int) duplicated_fd / 8;
if (index >= fds_to_close_bound)
{
if (index >= fds_to_close)
/* Need to increase the size of fds_to_close. */
abort ();
memset (fds_to_close + fds_to_close_bound, 0, index + 1 -
fds_to_close_bound);
fds_to_close_bound = index + 1;
}
fds_to_close[index] |= 1 << ((unsigned int) duplicated_fd % 8);
}
/* Close the previous fds that turned out to be too small. */
{
int saved_errno = errno;
unsigned int duplicated_fd;
for (duplicated_fd = 0; duplicated_fd < fds_to_close_bound * 8;
duplicated_fd++)
if ((fds_to_close[duplicated_fd / 8] >> (duplicated_fd % 8)) & 1)
close (duplicated_fd);
errno = saved_errno;
}
return result;
}
Then about F_SETFD:
+ case F_SETFD:
+ {
+ int value = va_arg (arg, int);
+ if (value != 0 && value != FD_CLOEXEC)
+ errno = EINVAL;
+ else
+ {
+ HANDLE handle = (HANDLE) _get_osfhandle (fd);
+ DWORD flags = value ? 0 : HANDLE_FLAG_INHERIT;
+ if (handle == INVALID_HANDLE_VALUE
+ || SetHandleInformation (handle, HANDLE_FLAG_INHERIT,
+ flags) == 0)
+ errno = EBADF;
+ else
+ result = 0;
+ }
This code does not preserve the invariant that for any open fd, the flags
stored inside MSVCRT for fd contain the bit FNOINHERIT if and only if
the handle _get_osfhandle (fd) has the InheritHandle flag set to FALSE.
Recall that for an fd to be passed to a child process, two things must
occur:
1) _dospawn must pass the handle as part of a certain array to CreateProcess;
this is done only if the (hidden) flags contain FNOINHERIT, and
2) CreateProcess must transform the handle valid for the parent to a handle
valid for the child. This is done if the InheritHandle flag is set.
If it is not set, the child process gets an INVALID_HANDLE_VALUE.
But the flags stored inside MSVCRT for fd cannot be directly accessed.
The only way they can be influenced is through the _open_osfhandle
function. So, the only viable implementation of F_SETFD would be to
1. Determine the flags for _open_osfhandle.
GetHandleInformation, then translate
!HANDLE_FLAG_INHERIT -> O_NOINHERIT,
setmode(O_BINARY) == O_TEXT -> O_TEXT and redo setmode(O_TEXT),
O_APPEND unknown!
2. Use dup and close to move the fd to a different value,
3. Use repeated equivalents of w32spawn.h:dup_noinherit to copy the
moved fd to the right place.
4. Close fds that were returned by dup_noinherit but were the wrong
value.
But this cannot work with O_APPEND and is quite a hack.
I therefore think that instead of offering the POSIXy fcntl call,
gnulib should offer functions like dup, dup2, dup_safer, fd_safer,
that take a flags argument as additional parameter.
int dup_ex (int fd, int flags);
int dup2_ex (int oldfd, int newfd, int flags);
int dup_safer_ex (int fd, int flags);
int fd_safer_ex (int fd, int flags);
The possible values in flags include not only O_CLOEXEC alias O_NOINHERIT,
but also O_BINARY / O_TEXT and O_APPEND.
Bruno
