-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
According to Bruno Haible on 8/22/2009 5:59 AM:
> 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().
I've pushed the first one (after rebasing on top of today's changes),
along with adding trivial O_TTY_INIT support (below). The second one in
my original series (fcntl-tests) doesn't make sense until we figure out
how to fix the third one for mingw (as otherwise, I will be checking in a
test that is broken under mingw).
So maybe at this point I will regroup a bit and figure out how to replace
broken fcntl on Unix-y systems, and focus on F_DUPFD_CLOEXEC first. I'm
hoping that it is generally safe to do the following with varargs, even on
64-bit platforms where int and void* are different sizes (since for
unknown platform-specific F_* flags, I can't possibly know what type the
third argument to fcntl is)? Here, f1 would be the system fcntl(), f2 our
wrapper, and f3 the caller. My hope is that on a 64-bit platform, even
though f2 reads more bits into its intermediate p than what the caller
passes, the relevant 32-bits are placed back in vararg form in the correct
pattern so that f1 will be reading them with the correct int type.
#include <stdarg.h>
int
f1 (int a, ...)
{
va_list arg;
int i;
va_start (arg, a);
i = va_arg (arg, int);
va_end (arg);
return i;
}
int
f2 (int a, ...)
{
va_list arg;
void *p;
va_start (arg, a);
p = va_arg (arg, void *);
va_end (arg);
return f1 (a, p);
}
int f3 (void)
{
return f2 (1, 1);
}
>
> 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)
I agree that a non-recursive function is better. Actually, I'd like to
see F_DUPFD and F_DUPFD_CLOEXEC be the preferred interfaces (those are
both POSIX) rather than dup_noinherit (or dup_ex); dup(n) is trivially
fcntl(n,F_DUPFD,0), so all we are missing is mapping dup_noinherit to
fcntl(n,F_DUPFD_CLOEXEC,0). At any rate, that means the helper function
on mingw should probably take a bool argument, to be shared by both
F_DUPFD variants, and call DuplicateHandle (rather than dup) with the
correct InheritHandle argument all the way through the chain, then use a
single _open_osfhandle at the end (and not my approach of attempting
F_SETFD at the end).
I'll see what I can do, once I get time (it won't be until the middle of
next week).
> {
> unsigned char fds_to_close[4096 / 8];
4096 seems magic - can we justify it with <limits.h>, with OPEN_MAX or the
like? 8 should be replaced by CHAR_BIT, even though we know all clients
of this code will have 8-bit char, since gnulib is trying to set the
example (when possible) about how to be portable even to non-POSIX systems
with larger char.
> 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.
Ouch. Good call. Paolo's comment also holds about setting FD_CLOEXEC
being easier than clearing, if we are okay with child processes starting
with an open fd tied to an invalid handle.
> O_APPEND unknown!
Is there seriously no way to ascertain whether an fd on Woe32 is in append
mode? Wow, that will make implementing F_GETFL tough.
> 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.
fcntl still makes sense for at least F_DUPFD{,_CLOEXEC}, F_GETFD. It just
means that F_SETFD might be asymmetric (can set but not clear CLOEXEC) or
missing (ie. we conscientiously choose to only support methods that use
the appropriate open/dup mechanisms to create new fds with the correct
attributes up front rather than modifying an existing fd).
We've already overridden open() and friends for mingw, so we could
implement O_APPEND tracking for all fds created within our own process
(but not those inherited from a parent process). I don't know if that is
helpful, or worse because we would then be inconsistent.
- --
Don't work too hard, make some time for fun as well!
Eric Blake e...@byu.net
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkqQuB4ACgkQ84KuGfSFAYAoRQCfZv3of9Nk0SAuO2H+KYvU+Kss
mPAAniwVnLYURmuh8a8KALsWDJlEDGUp
=Vn9N
-----END PGP SIGNATURE-----
From 7d8f602d6f6c1ac2633c23140c93aaca098c6a2a Mon Sep 17 00:00:00 2001
From: Eric Blake <e...@byu.net>
Date: Sat, 22 Aug 2009 21:24:37 -0600
Subject: [PATCH] fcntl-h: add O_TTY_INIT support
* lib/fcntl.in.h (O_TTY_INIT): Support another POSIX macro.
* tests/test-fcntl-h.c (o): Test it.
* doc/posix-headers/fcntl.texi (fcntl.h): Update documentation.
Signed-off-by: Eric Blake <e...@byu.net>
---
ChangeLog | 5 +++++
doc/posix-headers/fcntl.texi | 6 +++---
lib/fcntl.in.h | 6 +++++-
tests/test-fcntl-h.c | 2 +-
4 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 4486d1a..bf65748 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2009-08-23 Eric Blake <e...@byu.net>
+ fcntl-h: add O_TTY_INIT support
+ * lib/fcntl.in.h (O_TTY_INIT): Support another POSIX macro.
+ * tests/test-fcntl-h.c (o): Test it.
+ * doc/posix-headers/fcntl.texi (fcntl.h): Update documentation.
+
fcntl-h: rename from fcntl, in preparation for fcntl(2)
* modules/fcntl: Move <fcntl.h> header replacement...
* modules/fcntl-h: ...to new name, so as not to collide with
diff --git a/doc/posix-headers/fcntl.texi b/doc/posix-headers/fcntl.texi
index f2d5fb2..6e770a3 100644
--- a/doc/posix-headers/fcntl.texi
+++ b/doc/posix-headers/fcntl.texi
@@ -9,8 +9,8 @@ fcntl.h
@itemize
@item
@samp{O_NOCTTY}, @samp{O_DSYNC}, @samp{O_NONBLOCK}, @samp{O_RSYNC},
-...@samp{o_sync}, @samp{O_DIRECTORY}, and @samp{O_NOFOLLOW} are not
-defined on some platforms.
+...@samp{o_sync}, @samp{O_DIRECTORY}, @samp{O_NOFOLLOW}, and
+...@samp{o_tty_init} are not defined on some platforms.
@item
@samp{O_BINARY}, @samp{O_TEXT} (not specified by POSIX, but essential for
@@ -37,7 +37,7 @@ fcntl.h
replacement is not atomic on these platforms.
@item
-...@samp{o_tty_init}, @samp{O_SEARCH}, and @samp{O_EXEC} are not defined
+...@samp{o_search} and @samp{O_EXEC} are not defined
on some platforms.
@item
diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h
index a688fb4..8b92521 100644
--- a/lib/fcntl.in.h
+++ b/lib/fcntl.in.h
@@ -1,6 +1,6 @@
/* Like <fcntl.h>, but with non-working flags defined to 0.
- Copyright (C) 2006-2008 Free Software Foundation, Inc.
+ Copyright (C) 2006-2009 Free Software Foundation, Inc.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -130,6 +130,10 @@ extern void _gl_register_fd (int fd, const char *filename);
# define O_SYNC 0
#endif
+#ifndef O_TTY_INIT
+# define O_TTY_INIT 0
+#endif
+
/* For systems that distinguish between text and binary I/O.
O_BINARY is usually declared in fcntl.h */
#if !defined O_BINARY && defined _O_BINARY
diff --git a/tests/test-fcntl-h.c b/tests/test-fcntl-h.c
index 127f497..649c44a 100644
--- a/tests/test-fcntl-h.c
+++ b/tests/test-fcntl-h.c
@@ -22,7 +22,7 @@
/* Check that the various O_* macros are defined. */
int o = O_DIRECT | O_DIRECTORY | O_DSYNC | O_NDELAY | O_NOATIME | O_NONBLOCK
- | O_NOCTTY | O_NOFOLLOW | O_NOLINKS | O_RSYNC | O_SYNC
+ | O_NOCTTY | O_NOFOLLOW | O_NOLINKS | O_RSYNC | O_SYNC | O_TTY_INIT
| O_BINARY | O_TEXT;
/* Check that the various SEEK_* macros are defined. */
--
1.6.3.3.334.g916e1
