Re: O_CLOEXEC support
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Paolo Bonzini on 8/21/2009 1:34 AM: (or, is mingw subject to a compile-time maximum of open fds where we can just return EMFILE up front?). EMFILE is more appropriate that EBADF; POSIX states this for fcntl: [EMFILE] The argument cmd is F_DUPFD or F_DUPFD_CLOEXEC and all file descriptors available to the process are currently open, or no file descriptors greater than or equal to arg are available. The maximum valid file descriptor number for MSVCRT (and hence mingw) is 2047: Makes sense. Based on stack usage for one iteration, that won't use more than a couple pages of stack, if someone ever tries to use an fd that large. and above the limit dup2 fails with EBADF. I didn't find any documentation but the limit is already quite high and so it is unlikely to increase: for Linux, the limit is 1024 (and it also fails with EBADF). Well, regardless of what POSIX recommends for this failure case, I'm comfortable returning EBADF if that's what the recursion ended with. You can compute this at configure time and use a bitmask too, but I think it is overkill. Agreed. - -- 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/ iEYEARECAAYFAkqOiqAACgkQ84KuGfSFAYAClgCgkcv/v28pcA3ZMxroVrUHymFg UkcAnR5WVwb7rvEMALSRZ+L/LZv9rhZn =KKXs -END PGP SIGNATURE-
Re: [PATCH] popen-safer: test O_CLOEXEC at run-time.
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Paolo Bonzini on 8/21/2009 1:42 AM:
This patch tests for the presence of O_CLOEXEC at run-time.
The technique I use allows to use only two system calls instead of
three on system where O_CLOEXEC does not work, at least after the
first call to open_noinherit.
Sounds good to me, except for a couple of nits:
+#ifdef O_CLOEXEC
+ /* 0 = unknown, 1 = yes, -1 = no. */
+ static bool have_cloexec;
This has to be an int, not a bool, per the documented semantics you gave it.
+ if (have_cloexec = 0)
+{
+ fd = open (name, flags | O_CLOEXEC);
^
Not sure how that 8-bit character got there.
- --
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/
iEYEARECAAYFAkqOi7IACgkQ84KuGfSFAYBgyACeM7KtVd4iWuXX5XMK0/+Mh1n+
VREAoJLoCuP0DL1ewOBB+TDH1agkYw9B
=VXK4
-END PGP SIGNATURE-
Re: abbreviating gnulib-generated make output
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 8/21/2009 3:28 AM: And with the following patch, if you add this line to your project's configure.ac file, AM_SILENT_RULES([yes]) # make --enable-silent-rules the default. Or you can populate your config.site file with: enable_silent_rules=yes This change affects many modules/ files, not all mine, so I'll wait for feedback before pushing. OK for all of my modules in the list. - -- 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/ iEYEARECAAYFAkqOkq8ACgkQ84KuGfSFAYC5DACfTVFnlR+gM6mSjZP6lBIPwPm+ 92QAmwWg+aAmN+BL79B2haF9tqgZkIrM =hXLi -END PGP SIGNATURE-
Re: [PATCH] exclude-tests: Handle Windows EOL.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Simon Josefsson on 8/20/2009 7:22 AM: Pushed. See http://autobuild.josefsson.org/gnulib/log-200908200817225767000.txt + + * tests/test-exclude1.sh: Handle Windows EOL. Rather than adding a tr process, would it instead make sense to make test-exclude.c use the binary-io module to ensure binary output, so that no \r are generated in the first place? - -- 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/ iEYEARECAAYFAkqOnIcACgkQ84KuGfSFAYBI6ACffjCmnaR47O0DaGTJfIT9Av/Q amgAnjrO2yjsycpCVSk0J3P6t+yhYx7z =Ih5w -END PGP SIGNATURE-
Re: popen bugs, popen_safer
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Bruno Haible on 8/20/2009 2:14 AM:
This is becoming confusing: Why should the 'fopen' test suddenly test
fopen_safer,
not fopen??
I would find it better to introduce a new test module fopen-safer-tests, that
shares
the bulk of the code with fopen-tests. File structure:
test-fopen.h : all common code, starting with the definition of ASSERT,
test-fopen.c : just the includes and #include test-fopen.h
test-fopen-safer.c : just the includes, then #include stdio--.h, then
#include test-fopen.h
See test-snprintf-posix.h, test-snprintf-posix.c, test-vsnprintf-posix.c for
an example of this coding pattern.
Likewise for test-open.c and fcntl--.h.
Likewise for test-popen.c and stdio--.h.
Done.
- --
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/
iEYEARECAAYFAkqOoz4ACgkQ84KuGfSFAYAEvACeL0hJEWru3V2v44O2gjKdmoRa
kn0AoJq39SDjqOJmrtWCvvxIRkyPVKUR
=xnLZ
-END PGP SIGNATURE-
From 41fc74b9ab67321453d103d056a6e8eb8897042a Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Thu, 20 Aug 2009 23:19:20 -0600
Subject: [PATCH 1/3] test-fopen-safer: split from test-fopen
* tests/test-fopen.c (main): Move...
* tests/test-fopen.h: ...into new file.
* tests/test-fopen-safer.c: New file.
* modules/fopen-tests (Files): Add test-fopen.h.
* modules/fopen-safer-tests: New file.
Suggested by Bruno Haible.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog | 10 ++
modules/fopen-safer-tests | 11 +++
modules/fopen-tests |1 +
tests/test-fopen-safer.c | 23 +++
tests/test-fopen.c| 28 +---
tests/test-fopen.h| 44
6 files changed, 90 insertions(+), 27 deletions(-)
create mode 100644 modules/fopen-safer-tests
create mode 100644 tests/test-fopen-safer.c
create mode 100644 tests/test-fopen.h
diff --git a/ChangeLog b/ChangeLog
index 565d70a..1530f82 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2009-08-21 Eric Blake e...@byu.net
+
+ test-fopen-safer: split from test-fopen
+ * tests/test-fopen.c (main): Move...
+ * tests/test-fopen.h: ...into new file.
+ * tests/test-fopen-safer.c: New file.
+ * modules/fopen-tests (Files): Add test-fopen.h.
+ * modules/fopen-safer-tests: New file.
+ Suggested by Bruno Haible.
+
2009-08-21 Paolo Bonzini bonz...@gnu.org
popen-safer: test O_CLOEXEC at run-time.
diff --git a/modules/fopen-safer-tests b/modules/fopen-safer-tests
new file mode 100644
index 000..21116e6
--- /dev/null
+++ b/modules/fopen-safer-tests
@@ -0,0 +1,11 @@
+Files:
+tests/test-fopen.h
+tests/test-fopen-safer.c
+
+Depends-on:
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-fopen-safer
+check_PROGRAMS += test-fopen-safer
diff --git a/modules/fopen-tests b/modules/fopen-tests
index cf87389..fb0ee1d 100644
--- a/modules/fopen-tests
+++ b/modules/fopen-tests
@@ -1,4 +1,5 @@
Files:
+tests/test-fopen.h
tests/test-fopen.c
Depends-on:
diff --git a/tests/test-fopen-safer.c b/tests/test-fopen-safer.c
new file mode 100644
index 000..701af35
--- /dev/null
+++ b/tests/test-fopen-safer.c
@@ -0,0 +1,23 @@
+/* Test of opening a file stream.
+ Copyright (C) 2007-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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see http://www.gnu.org/licenses/. */
+
+/* Written by Bruno Haible br...@clisp.org, 2007. */
+
+#include config.h
+
+#include stdio--.h
+
+#include test-fopen.h
diff --git a/tests/test-fopen.c b/tests/test-fopen.c
index b9e3694..473d274 100644
--- a/tests/test-fopen.c
+++ b/tests/test-fopen.c
@@ -19,31 +19,5 @@
#include config.h
#include stdio.h
-#include stdlib.h
-#if GNULIB_FOPEN_SAFER
-# include stdio--.h
-#endif
-
-#define ASSERT(expr) \
- do\
-{ \
- if (!(expr)) \
-{ \
- fprintf (stderr, %s
Re: fcntl module
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 8/22/2009 5:59 AM: 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); What's the difference between dup2_ex and dup3, other than the set of valid flags accepted? - -- 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/ iEYEARECAAYFAkqQTBwACgkQ84KuGfSFAYDHkQCg1VZVqhNCDXru5xT56eAp7WHG XTYAn2p+ndaLqcQEc5iFCR1a/lsoapow =E6O3 -END PGP SIGNATURE-
Re: new module 'pipe2'
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Bruno Haible on 8/22/2009 10:46 AM:
/* Native Woe32 API. */
# include io.h
int
pipe2 (int fd[2], int flags)
{
/* Check the supported flags. */
if ((flags ~(O_CLOEXEC | O_BINARY | O_TEXT)) != 0)
Is there any way to support NONBLOCK in Woe32?
if ((fcntl_flags = fcntl (fd[1], F_GETFL, 0)) 0
|| fcntl (fd[1], F_SETFL, fcntl_flags | O_NONBLOCK) 0
Not safe; per POSIX, F_SETFL success must be checked via != -1 (not 0).
Multiple instances. Also, should we offer O_BINARY/O_TEXT support on
cygwin, using setmode()? (On Linux, O_BINARY and O_TEXT are both 0, so we
have implicitly already done so).
- --
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/
iEYEARECAAYFAkqQe74ACgkQ84KuGfSFAYDzsACgjocHTvSSFTtm1xncZYxhOZ9I
xgQAn2ow56bWjjRXxViXnO/nDH6Lvhh7
=GzFK
-END PGP SIGNATURE-
Re: proposal: module 'accept4'
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Bruno Haible on 8/22/2009 3:39 PM:
Hi,
Here's the proposed module for function 'accept4'.
It does not define the macro SOCK_CLOEXEC. I think this macro should be
replaced by a separate module, that would then also influence socket(),
socketpair() - and what about connect()?
socket and socketpair both (need to) honor SOCK_CLOEXEC; but connect does
not create a new fd (it just modifies an existing one) so it doesn't need
any change.
nfd = _open_osfhandle ((long) new_handle,
O_NOINHERIT | (flags (O_TEXT | O_BINARY)));
if (nfd 0)
{
int saved_errno = errno;
close (fd);
errno = saved_errno;
return -1;
}
close (fd);
return nfd;
This means that nfd fd, so there is a gap in the fd sequence which is
unexpected per POSIX rules. Is it worth the hassle of using
dup2(nfd,fd),close(fd) instead of close(fd)?
if (flags SOCK_CLOEXEC)
{
int fcntl_flags;
if ((fcntl_flags = fcntl (fd, F_GETFD, 0)) 0
|| fcntl (fd, F_SETFD, fcntl_flags | FD_CLOEXEC) 0)
Success for F_SETFD is like F_SETFL - you must compare against explicit -1.
Otherwise, looks sane to me. Do you have a test case in mind?
- --
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/
iEYEARECAAYFAkqQrigACgkQ84KuGfSFAYCTxgCcD66ZPeAuT+x3Chag/LRP6ZOn
YhIAoI1jr91zmRgeqrosAFHElwdyJhQ1
=SbKS
-END PGP SIGNATURE-
Re: proposal: module 'mkostemp'
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 8/22/2009 5:20 PM: Here is a proposal to add a module 'mkostemp', using the source code changes from glibc. It also reduces the diffs to glibc, by incorporating parts of this glibc commit: Nice to do some resyncing. Overall, I'm in favor of this improvement. /* Generate a unique temporary file name from TEMPLATE. The last six characters of TEMPLATE must be XX; they are replaced with a string that makes the file name unique. Then open the file and return a fd. */ int mkostemp (template, flags) char *template; int flags; I take it the failure to mention 'flags' in the comment is a glibc oversight? +#if @GNULIB_MKOSTEMP@ +# if !...@have_mkostemp@ +/* Create a unique temporary file from TEMPLATE. + The last six characters of TEMPLATE must be XX; + they are replaced with a string that makes the file name unique. + The file is then created, with the specified flags, ensuring it didn't exist + before. Should we mention which FLAGS are portable in the gnulib version (O_CLOEXEC, O_BINARY, O_TEXT), as you did for accept4? - fd = large_open (tmpl, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR); + fd = __open (tmpl, +(flags ~0777) | O_RDWR | O_CREAT | O_EXCL, +S_IRUSR | S_IWUSR); Wow - that means any other flags larger than 9 bits, like the sticky bits at 07000, or even implementation-specific O_* values, are blindly passed through. Do we really want that? - -- 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/ iEYEARECAAYFAkqQr38ACgkQ84KuGfSFAYDEzwCfZr8IvFfks9/2T/jrJw6lLqVv 5iMAoIQzPdddj3FfAYpjMNR42LiTKd6O =7wXs -END PGP SIGNATURE-
Re: fcntl module
-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
Re: proposal: module 'accept4'
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Paolo Bonzini on 8/23/2009 10:22 AM: This can be committed as is but it is not enough. You have to always use a replacement (thus define the emulation as rpl_accept4) and check whether accept4 works at runtime (and similarly for pipe2) and fall back to accept+fcntl if it gives ENOSYS. Is a configure-time AS_RUN_IFELSE test good enough to find which systems actually support SOCK_CLOEXEC? If I'm understanding correctly, the runtime test is only needed on systems where SOCK_CLOEXEC is defined, but the kernel is too old to support it. Or are you saying that it is possible to compile on a newer glibc, then run with the same libc.so but against an older kernel, where the result of a configure-time test is no longer accurate? - -- 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/ iEYEARECAAYFAkqRcjwACgkQ84KuGfSFAYBs9ACeKXESOj14U9WexWg1szqYQucJ tsYAoKO5d3v+xNPFImetH1dZPR14ghkF =MXDA -END PGP SIGNATURE-
Re: new module 'dup3'
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Bruno Haible on 8/23/2009 4:17 PM:
This implements the 'dup3' replacement function.
Paolo's issue of use of older Linux kernels is not yet handled.
Looks good.
#if (defined _WIN32 || defined __WIN32__) ! defined __CYGWIN__
/* Native Woe32 API. */
if (flags O_CLOEXEC)
{
...
}
if (dup2 (oldfd, newfd) 0)
return -1;
In the case where oldfd was previously marked O_NOINHERIT, dup2() on mingw
appears to create newfd as O_NOINHERIT as well. I think we have to rework
this patch to use DuplicateHandle/_open_osfhandle on both control paths.
- --
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/
iEYEARECAAYFAkqRyaMACgkQ84KuGfSFAYD04ACgtPuvd5z1sPxV7fvpXooJ5+5Q
jsIAoKkOwf/d5IshB71WvMUlTCddqJTJ
=Cegd
-END PGP SIGNATURE-
Re: proposal: module 'accept4'
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 8/23/2009 4:58 PM: Tolerate declared but missing pipe2 syscall. * lib/pipe2.c (pipe2): Invoke original pipe2 function first, if available. Shouldn't we follow the lead of popen-safer, and cache whether pipe2 exists, so that we aren't wasting time on a guaranteed ENOSYS on all subsequent invocations? - -- 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/ iEYEARECAAYFAkqR04IACgkQ84KuGfSFAYCNJQCgmnYnHWLrVp2IOFp8V6YwyMMN BF4AoL6oTzQRlu76DPmdTjl2lDOxW8VD =OgL+ -END PGP SIGNATURE-
Re: fcntl module
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 8/23/2009 6:01 PM: Now that we have pipe2, accept4, the next step is to combine them with the *-safer functionality. This overlaps with F_DUPFD and F_DUPFD_CLOEXEC. So, the actual better primitives are dup_ex (int fd, int flags, int minimum) I almost wonder if it is better to change the signature of the existing dup_safer to take extra parameters. There are a few clients of dup_safer that would have to pass the extra argument, but most users go via unistd--.h which can adjust the #define appropriately. In other words, I see no reason to invent dup_ex, rather just adjust the API of dup_safer. Whether or not we change the signature of dup_safer, I think this would be a better primitive (then we can map dup, dup2, fcntl, and dup_safer onto the same primitive): /* Duplicate FD into a new file descriptor, at least as large as MINIMUM, and first closing any existing fd at minimum if OVERWRITE. FLAGS can contain O_CLOEXEC, O_TEXT, or O_BINARY. */ gl_dup (int fd, int minimum, int flags, bool overwrite) dup (n) - gl_dup (n, 0, 0, false) dup2 (n, m) - gl_dup (n, m, 0, true) dup3 (n, m, flags)- gl_dup (n, m, flags, true) fcntl (n, F_DUPFD, m) - gl_dup (n, m, 0, false) fcntl (n, F_DUPFD_CLOEXEC, m) - gl_dup (n, m, O_CLOEXEC, false) dup_safer (n) - gl_dup (n, 3, 0, false) dup_safer_noinherit (n) - gl_dup (n, 3, O_CLOEXEC, false) pipe2_ex (int fd[2], int flags, int minimum) Seems okay (although maybe the name gl_pipe2 is better than pipe2_ex): pipe (fd) - pipe2_ex (fd, 0, 0) pipe2 (fd, flags) - pipe2_ex (fd, flags, 0) pipe_safer (fd) - pipe2_ex (fd, 0, 3) pipe2_safer (fd, flags) - pipe2_ex (fd, flags, 3) and unistd--.h would automatically pick up pipe2_safer. accept4_ex (int s, struct sockaddr *addr, socklen_t *addrlen, int flags, int minimum) Likewise. Another thought I had would be inventing our own O_SAFER flag (which we then mask off before calling the real underlying functions, but use its presence to guarantee that any fd returned will be 3 or larger). It doesn't map very well to fcntl or dup2, (but those interfaces can already specify a minimum or target fd), and doesn't make sense with dup3 (which specifies a target fd). But it makes life for all the other safer variants possible; and it very much does imply that we'd need a gl_dup primitive for dup/dup_safer/dup_noinherit. For that matter, proposing an O_SAFER to the glibc folks might be worthwhile. At which point, we have things like: open_safer (s, f [, m]) - open (s, f | O_SAFER [, m]) pipe (fd) - pipe2 (fd, 0) pipe_safer (fd) - pipe2 (fd, O_SAFER) pipe2_safer (fd, flags) - pipe2 (fd, flags | O_SAFER) dup (n) - gl_dup (n, 0) dup_safer (n) - gl_dup (n, O_SAFER) dup_safer_noinherit (n) - gl_dup (n, O_SAFER | O_CLOEXEC) I prefer the dup_ex interface to fcntl, because it supports not only O_CLOEXEC but also O_TEXT / O_BINARY and possibly other flags in the future. But at least you can change O_TEXT/O_BINARY after the fact, without risking the security hole that O_CLOEXEC was invented to plug. - -- 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/ iEYEARECAAYFAkqR/O8ACgkQ84KuGfSFAYAUvQCgq8KoWW6YI25I1SxqjdZt5xfF DJ0AoIu9bheU4D2AHSdJW4quKyt/bBK0 =fNJs -END PGP SIGNATURE-
Re: fcntl module
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 8/23/2009 8:37 PM: For that matter, proposing an O_SAFER to the glibc folks might be worthwhile. With kernel support for O_SAFER, our *_safer paradigms could even be more efficient (no fd_safer after the fact, so fewer kernel calls and less temporary fd pressure). Plus, the kernel folks have so far avoided having to create dup + flags: dup (n) - fcntl (n, F_DUPFD, 0) dup_safer (n) - fcntl (n, F_DUPFD, 3) dup_noinherit (n) - fcntl (n, F_DUPFD_CLOEXEC, 0) dup_safer_noinherit (n) - fcntl (n, F_DUPFD_CLOEXEC, 3) so everywhere that we care about *_safer, we either have a place for a new flag or we can already specify a minimum. - -- 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/ iEYEARECAAYFAkqSBgQACgkQ84KuGfSFAYDOdwCfUetXfbJHSWgjDKPbfyADsQ1x JmIAn2lXAMm4FP7SnAeqHRTv9b8THgHM =AVya -END PGP SIGNATURE-
Re: O_SAFER
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Paolo Bonzini on 8/24/2009 5:20 AM: Yes, but better call it O_NONSTD O_NOSTDFD? I'm in the middle of writing an RFC email to lkml (with bug-gnulib in cc); I'm using the name O_NOSTD for now, but mentioning the other names we've thought of. We get some problem with unistd--.h and fcntl--.h: How do we define open() such that open (s, f [, m]) ::= open (s, f | O_SAFER [, m]) That becomes a bit hairy. It would be done as follows (leaving the declarations pretty much as it already is, and just changing the implementation of open_safer to calling open with a new flag rather than calling fd_safer(open)): fcntl.h/open.c - declares open, implements open (s, O_SAFER [, m]) fcntl_safer.h/open-safer.c - decares open_safer (s, f [, m]), which calls open (s, f | O_SAFER [, m]) fcntl--.h - #define open open_safer Users who care about manipulating std descriptors (such as open-safer.c) use fcntl_safer.h but NOT fcntl--.h, and must manually distinguish between open (f), open (f|O_SAFER), and open_safer. But the bulk of the users include fcntl--.h, and see no difference. ... what about creat, too? creat already has issues (for example, you can't specify O_TEXT or O_BINARY with creat; you can't specify O_NOCTTY to protect yourself from obtaining a controlling terminal, ...); portable code already uses open() rather than creat(). - -- 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/ iEYEARECAAYFAkqSeeMACgkQ84KuGfSFAYC7BQCggZqT51zTbiexPI8YcmO5njx2 G8AAn1ymtkOtP4at/RpvsJR9AUfIwtLt =Rl/f -END PGP SIGNATURE-
Re: tweak test-dup2
Bruno Haible bruno at clisp.org writes: Let me know if you find these tweaks unreasonable. The first hunk is to ensure that even if a non-empty test-dup2.tmp existed before the test, it will not disturb the test. Looks good to me. Unfortunately... + errno = 0; + ASSERT (dup2 (fd, 1000) == -1); + ASSERT (errno == EBADF); ...this test exposes a bug on cygwin 1.5.x (which fails with EMFILE); and further testing showed that cygwin 1.5.x fcntl(0,F_DUPFD,1000) also has a bug, failing with EBADF instead of the required EINVAL (why dup2 must fail with EBADF when fcntl fails with EINVAL is beyond me). The following patch clears up this issue, and a related compilation warning in pipe2.c. From: Eric Blake e...@byu.net Date: Mon, 24 Aug 2009 16:00:44 -0600 Subject: [PATCH] dup2, pipe2: fix some recent test failures on cygwin 1.5.x * lib/pipe2.c (includes): Add binary-io.h. * lib/dup2.c (rpl_dup2): Correct buggy errno value. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog |6 ++ lib/dup2.c |3 +++ lib/pipe2.c |2 ++ 3 files changed, 11 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index 21e02a2..0b8e2f6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2009-08-24 Eric Blake e...@byu.net + + dup2, pipe2: fix some recent test failures on cygwin 1.5.x + * lib/pipe2.c (includes): Add binary-io.h. + * lib/dup2.c (rpl_dup2): Correct buggy errno value. + 2009-08-23 Bruno Haible br...@clisp.org * lib/dup3.c: Include string.h. diff --git a/lib/dup2.c b/lib/dup2.c index 6d61829..6b6f45d 100644 --- a/lib/dup2.c +++ b/lib/dup2.c @@ -57,6 +57,9 @@ rpl_dup2 (int fd, int desired_fd) result = dup2 (fd, desired_fd); if (result == 0) result = desired_fd; + /* Correct a cygwin 1.5.x errno value. */ + else if (result == -1 errno == EMFILE) +errno = EBADF; return result; } diff --git a/lib/pipe2.c b/lib/pipe2.c index d3b612d..7def1b1 100644 --- a/lib/pipe2.c +++ b/lib/pipe2.c @@ -23,6 +23,8 @@ #include errno.h #include fcntl.h +#include binary-io.h + #if (defined _WIN32 || defined __WIN32__) ! defined __CYGWIN__ /* Native Woe32 API. */ -- 1.6.3.2
Re: pipe-filter self-tests
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Simon Josefsson on 8/24/2009 2:49 PM: Simon Josefsson si...@josefsson.org writes: test-pipe.c:79: assertion failed test-pipe.sh: iteration 4 failed test-pipe.c:79: assertion failed test-pipe.sh: iteration 5 failed test-pipe.c:79: assertion failed test-pipe.sh: iteration 6 failed test-pipe.c:79: assertion failed test-pipe.sh: iteration 7 failed FAIL: test-pipe.sh These tests fails the same way on a Debian x86 system (the gcc compile farm host gcc60) as well, see: http://autobuild.josefsson.org/gnulib/log-20090823221435536.txt That log also shows that test-dup2.c and test-popen.sh are failing; it appears all of the failures are of the case: close(n),dup2(n,n) not returning -1 EBADF. I'm still working on getting a gcc compile farm account; in the meantime, what version of kernel and glibc is on this machine? I suspect that the dup2.m4 test needs to be enhanced. - -- 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/ iEYEARECAAYFAkqTRCIACgkQ84KuGfSFAYA41gCgznJnwj7QR5Jc5zu/EfxugMLw NkYAnRQUB5PcmicOCe93Iz+V2DeVwi4O =T8qt -END PGP SIGNATURE-
Re: [PATCH] open: introduce O_NOSTD
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Davide Libenzi on 8/25/2009 3:53 PM:
Another solution is for the application to sanitize all newly-created
fds: GNU coreutils provides a wrapper open_safer, which does nothing
extra in the common case that open() returned 3 or larger, but calls
fcntl(n,F_DUPFD,3)/close(n) before returning if n was less than 3.
However, this leads to triple the syscall cost for every open() call
if the process starts life with a std fd closed; and if O_CLOEXEC is
not used, still leaves a window of time where the fd can be leaked
through another thread's use of fork/exec.
I think we can say that the vast majority of the software is not going to
notice the proposed open_safer(), performance-wise, since the first three
fds are always filled. So IMO the performance impact argument is a weak one.
If CLOEXEC semantics are needed in the open operation, F_DUPFD_CLOEXEC can
be used to match it.
The current gnulib implementation of open_safer (pre-O_CLOEXEC support) is
(roughly):
/* Wrap open, to guarantee that a successful return value is = 3. */
int open_safer (const char *name, int flags, int mode)
{
int fd = open (name, flags, mode);
if (0 = fd fd = 2)
{
int dup = fcntl (fd, F_DUPFD, 3);
int saved_errno = errno;
close (fd);
errno = saved_errno;
fd = dup;
}
return fd;
}
which has the desired property of no overhead in the common case of all
standard fds open. But it obviously mishandles the O_CLOEXEC flag.
Here's a first cut at supporting it:
int open_safer (const char *name, int flags, int mode)
{
int fd = open (name, flags, mode);
if (0 = fd fd = 2)
{
int dup = fcntl (fd, ((flags O_CLOEXEC)
? F_DUPFD_CLOEXEC : F_DUPFD), 3);
int saved_errno = errno;
close (fd);
errno = saved_errno;
fd = dup;
}
return fd;
}
If the user requested open_safer(O_CLOEXEC), then we still have the
desired property of no syscall overhead and no fd leak. But if the user
intentionally does not pass O_CLOEXEC because they wanted to create an
inheritable fd, but without stomping on standard fds, then this version
still has a window for an fd leak. So let's try this version, which
guarantees no fd leak, while still keeping the semantics of giving the
user an inheritable fd outside the std fd range:
int open_safer (const char *name, int flags, int mode)
{
int fd = open (name, flags | O_CLOEXEC, mode);
if (0 = fd fd = 2)
{
int dup = fcntl (fd, ((flags O_CLOEXEC)
? F_DUPFD_CLOEXEC : F_DUPFD), 3);
int saved_errno = errno;
close (fd);
errno = saved_errno;
fd = dup;
}
else if (!(flags O_CLOEXEC))
{
if ((flags = fcntl (fd, F_GETFD)) 0
|| fcntl (fd, F_SETFD, flags ~FD_CLOEXEC) == -1)
{
int saved_errno = errno;
close (fd);
fd = -1;
errno = saved_errno;
}
}
return fd;
}
This solves the fd leak, and open_safer(O_CLOEXEC) is still cheap in the
common case. But now the case of open_safer without O_CLOEXEC costs 3
syscalls, regardless of whether the standard fds were already open (if we
assumed there were no possibility of new FD_* flags, we could cut the
common-case penalty from three to two by skipping the fcntl(fd,F_GETFD)
and just using fcntl(fd,F_SETFD,0), but that's asking for problems).
While the patch is simple, IMO this is something that can be easily taken
care in glibc layers w/out huge drawbacks.
I hope that my example shows why doing it in the kernel is desirable -
there is no safe way to keep the pre-O_CLOEXEC efficiency using just the
library, but there IS a way to do it with kernel support:
int open_safer (const char *name, int flags, int mode)
{
return open (name, flags | O_NOSTD, mode);
}
Also, remember this statement from Ulrich in the series that first
introduced O_CLOEXEC/O_NONBLOCK support across all the fd creation APIs:
In future there will be other uses. Like a O_* flag to enable the use
of non-sequential descriptors.
http://marc.info/?l=linux-kernelm=121010907127190w=2
- --
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/
iEYEARECAAYFAkqWj/gACgkQ84KuGfSFAYClIgCdEw6/7+PiFhR7aKGyuLUd5RdR
lbAAmgKLqCC5zlhkOm/x4K+Om7nqeD0i
=Ibq4
-END PGP SIGNATURE-
Re: [PATCH] open: introduce O_NOSTD
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Ulrich Drepper on 8/27/2009 8:22 AM: I hope that my example shows why doing it in the kernel is desirable - there is no safe way to keep the pre-O_CLOEXEC efficiency using just the library, but there IS a way to do it with kernel support: You're describing a very special case where the performance implications are really minimal and try to argue that is a good enough reason? I don't think so. If a program really has to do thousands of these safe open calls then it can invest time into opening /dev/null for any of the unallocated descriptors 3. You can even embed this logic in the safer_open function. But that's what I've been trying to explain. There are cases where opening a dummy /dev/null descriptor interferes with behavior, and where you NEED to leave the std descriptors closed. Again, let's look at coreutils, which already is using a version of open_safer in many of its utilities. Consider 'cp -i a b -'. If b does not exist, then there is no reason for cp to prompt, so it never reads from stdin, and the fact that stdin was closed must not interfere with execution. Which means cp cannot just blindly warn if it detects that stdin was closed when it started; it has to wait until conditions warrant reading from stdin. On the other hand, if b exists, coreutils' cp correctly warns the user about the inability to read from stdin: $ cp -i a b - cp: overwrite `b'? cp: error closing file: Bad file descriptor $ echo $? 1 If coreutils were to tie fd 0 to /dev/null, then it can no longer distinguish a read failure when reading the prompt from stdin, and would no longer be able to alert the user to the failed interactive copy. Next, consider 'cp -uv a b -'. Coreutils uses printf() to print status messages, but only if an update warranted a message. Again, that means coreutils wants to know whether stdout is a valid file, and tying fd 1 to /dev/null can get in the way: $ cp -uv a b - $ rm b $ cp -uv a b - cp: write error: Bad file descriptor Now, couple this with the fact that 'cp -r' can visit thousands of files during its course of operation. There is no sane way to tie dummy fds to the standard descriptors, and still be able to distinguish failure to use a standard descriptor but only on the execution paths that actually used that standard descriptor. Hence, there is a definite use case for keeping the std fds closed, rather than putting dummies in their place; but to preserve this setup without kernel support, every single open() (and dup(), pipe(), socket(), ...) must be wrapped to use fcntl/close to move new fds back out of this range. And cp is an example of an existing program that really DOES end up paying a penalty of thousands of fcntl/close calls if started with a standard fd closed, in order to preserve its semantics. Furthermore, while the coreutils' implementation of open_safer operates correctly in a single-threaded environment, it cannot close the race in a multi-threaded context if the application started life with fd 2 closed, since there is a window of time during open_safer where another thread can attempt to use stderr and see thread 1's newly opened file before it can be moved safely out of the way: thread 1 thread 2 open_safer... open - 2 fcntl(2,F_DUPFD,3) fputc(stderr) fflush(stderr) - clobbers thread 1's file close(2) open_safer - 3 - -- 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/ iEYEARECAAYFAkqXzXgACgkQ84KuGfSFAYA+ZwCgvuQ3PpDDLhLozqCUm3qyhIVj aqQAnR+MgdLg5apNEdHdIn9nlr4yRISI =0tcQ -END PGP SIGNATURE-
Re: [PATCH] open: introduce O_NOSTD
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Florian Weimer on 8/27/2009 8:35 AM:
* Eric Blake:
int open_safer (const char *name, int flags, int mode)
{
int fd = open (name, flags | O_CLOEXEC, mode);
if (0 = fd fd = 2)
{
int dup = fcntl (fd, ((flags O_CLOEXEC)
? F_DUPFD_CLOEXEC : F_DUPFD), 3);
int saved_errno = errno;
close (fd);
errno = saved_errno;
fd = dup;
}
else if (!(flags O_CLOEXEC))
{
if ((flags = fcntl (fd, F_GETFD)) 0
|| fcntl (fd, F_SETFD, flags ~FD_CLOEXEC) == -1)
{
int saved_errno = errno;
close (fd);
fd = -1;
errno = saved_errno;
}
}
return fd;
}
This solves the fd leak,
It's still buggy.
In what manner? Are you stating that F_DUPFD_CLOEXEC is not atomic?
You need something like this:
int open_safer(const char *name, int flags, int mode)
{
int opened_fd[3] = {0, 0, 0};
int fd, i, errno_saved;
while (1) {
fd = open(name, flags | O_CLOEXEC, mode);
if (fd 0 || fd 2) {
break;
}
opened_fd[fd] = 1;
}
for (int i = 0; i = 2; ++i) {
if (opened_fd[i]) {
errno_saved = errno;
close(i);
errno = errno_saved;
}
}
return fd;
}
It's untested, so it's probably still buggy.
Your version fails to clear the cloexec bit of the final fd if the
original caller didn't request O_CLOEXEC. If the caller requested
O_CLOEXEC, then your version takes 3, 5, or 7 syscalls depending on how
many std fds were closed, while my version takes 3 syscalls regardless of
how many std fds were closed. Also, your suggestion has a definite race
in that you are calling open() multiple times rather than cloning an
existing fd after the first open(), such that another process could alter
which file is visited between your first and last open().
- --
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/
iEYEARECAAYFAkqX0W0ACgkQ84KuGfSFAYDYdwCeOB8dt0Rx2QYJkfIsfP452AYj
V7QAoL1FACwnRPhhQ2aTh2EB38i+d34o
=ouPs
-END PGP SIGNATURE-
Re: [PATCH] open: introduce O_NOSTD
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Florian Weimer on 8/28/2009 6:52 AM: If the caller requested O_CLOEXEC, then your version takes 3, 5, or 7 syscalls depending on how many std fds were closed, while my version takes 3 syscalls regardless of how many std fds were closed. I really don't see a way around that. You can't pick a descriptor and hope that it's unused. fcntl(,F_DUPFD,3) is NOT like dup2. It picks the next available descriptor that is at least 3. - -- 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/ iEYEARECAAYFAkqX1HEACgkQ84KuGfSFAYAzYQCgnwad5k39bZJqw3Wg1vMED/Fd zjYAmwenJlImBUE33geRdHC6aoVSx7JG =omNI -END PGP SIGNATURE-
Re: [PATCH] open: introduce O_NOSTD
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Florian Weimer on 8/28/2009 6:52 AM: * Eric Blake: Your version fails to clear the cloexec bit of the final fd if the original caller didn't request O_CLOEXEC. Okay, but you can fix that in a race-free manner (but I thought that this was implied by open_safer). The current semantics of gnulib's open_safer is that the result is guaranteed to be 3 or larger. It would require an audit of all gnulib clients of the open_safer method to see whether it also makes sense to change the semantics of open_safer to also guarantee that fds start life with the cloexec bit set. But maybe that is a change worth making in gnulib, with applications intending to give an fd to a child process being required to explicitly clear the cloexec bit. Also, your suggestion has a definite race in that you are calling open() multiple times rather than cloning an existing fd after the first open(), such that another process could alter which file is visited between your first and last open(). Sure, but this is an unobservable differen.ce It is absolutely observable - if the user passed O_CREAT|O_EXCL as part of their flags, then the second open() will inappropriately fail. - -- 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/ iEYEARECAAYFAkqX1egACgkQ84KuGfSFAYDKWACeMM4spqCsmgVVwME9+C/1tdpU g7wAnR9FetGPGr7acXLfLIVvzYZ7tpz3 =VjUY -END PGP SIGNATURE-
need opendir_safer, dirent--.h
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I think we need to implement opendir_safer, alongside all the other *_safer modules. Otherwise, opendir can end up placing an open directory fd in one of the standard slots, and end up interfering with the intent of all the other *_safer wrappers. - -- 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/ iEYEARECAAYFAkqX2GsACgkQ84KuGfSFAYDuxwCdHcVNORe4dAbr9xb2jnq8inQq 2W0AoIyb0/icE7EiauGdb7OMQPw3FRnA =5dUv -END PGP SIGNATURE-
Re: need opendir_safer, dirent--.h
I think we need to implement opendir_safer, alongside all the other
*_safer modules. Otherwise, opendir can end up placing an open directory
fd in one of the standard slots, and end up interfering with the intent of
all the other *_safer wrappers.
And here's at least one use case where it matters:
$ find dir -mindepth 1 -ok echo {} \; -
echo ... dir/sub ?
$ echo $?
0
$ oldfind dir -mindepth 1 -ok echo {} \; -
echo ... dir/sub ? oldfind: error closing file
$ echo $?
1
Oops - the fts-based version calls getline() while fd 0 is tied
to dir, while oldfind was successfully able to recognize that
fd 0 was unreadable. And on platforms where reading a
directory returns data (yes, such fringe platforms still exist),
rather than my platform's choice that read(dir) returns EOF
without error, this could inadvertently end up executing the
-ok command based on whether the binary contents of the
directory resemble 'y'; at any rate, reading from a directory
fd can lead to all sorts of bad behavior.
I tried finding a use case with 'rm -ri -', but there, the query
of whether to descend occurs before the opendir, so fd 0 is
not tied to an open directory at that moment in time, and
the query fails because the read fails, so no further actions
are attempted.
I didn't try finding a case in tar, although I suspect it may
be possible to find one.
--
Eric Blake
--
View this message in context:
http://www.nabble.com/need-opendir_safer%2C-dirent--.h-tp25190069p25193082.html
Sent from the Gnulib mailing list archive at Nabble.com.
Re: ignoring EOPNOTSUPP and ENOTTY
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 8/30/2009 6:03 PM: case EOPNOTSUPP: /* Operation not supported */ case ENOTSUP:/* Operation not supported */ POSIX states that ENOTSUP==EOPNOTSUPP is permissible, so you can't do this with a case statement. - My policy is to ignore only errors that are known to indicate normal situations. If a particular errno has not been seen in the wild so far, don't ignore it. In general, and when in doubt, report errors. I agree - it's not worth ignoring an error unless we have a use case in the wild of someone that generates it where we can work around it; and the set of errors to ignore is highly context-dependent, so a generic function will never be useful to me. - -- 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/ iEYEARECAAYFAkqbFT8ACgkQ84KuGfSFAYCB9QCfbYuzVqpC9zd/YHgRxxaUdZwJ 2joAoKjciJ9mVKkZlAC3ggdv/9SK0XII =djzP -END PGP SIGNATURE-
fchdir on mingw
One of the main reasons the fchdir module was added was so that the openat
module (and friends) would compile on mingw. Well, it turns out that mingw has
the (documented) limitation that open(dir,O_RDONLY) fails with EACCES, so
rpl_open was never registering an fd visiting a directory in the first place,
and the fchdir replacement (and thus openat and fts) failed to run on mingw.
The first patch improves things by adding a fchdir unit test, and teaches
rpl_open how to fake out opening a directory (the end result of
using /dev/null under the hood means that read() on the fd gives the same EOF
results as on Linux; and replacing fstat() means we can hide the fact that we
used a dummy).
Techincally, I probably could have spent more time trying to figure out how to
open a directory HANDLE using CreateFile with FILE_FLAG_BACKUP_SEMANTICS, but I
couldn't quickly get that working. I also wonder how _open_osfhandle would
react to a directory HANDLE. At any rate, since the method in the patch below
passed the unit test, I'm not going to worry about the CreateFile approach.
The second patch fixes the fact that dup3 needs to play nicely with fchdir, and
changes dup2 so that it is only overridden once, not twice, in gnulib sources.
It also simplifies the logic in fchdir by not maintaining a saved_errno for
every known fd, but instead aborting any attempt to open a directory up front
with ENOMEM if we could not enlarge the map or strdup the directory name. It
is still possible to use fcntl(F_DUPFD{,_CLOEXEC}) to go behind fchdir's back
at the moment, but until I finish an fcntl replacement module, that isn't too
much to worry about.
I'm also working on a patch to split fdopendir into its own module (rather than
being intimately tied to openat), as a prerequisite for implementing
opendir_safer and dirent--.h. One improvement possible for fdopendir is that
if fchdir is being emulated by gnulib, we can do a query into the fchdir
metadata table to retrieve the directory name associated with an fd, rather
than having to go through save_cwd/fchdir/opendir/restore_cwd (although the
latter finally does work now on mingw, with just my first patch in place).
Also, it might still be possible to make dirfd work on mingw (right now, it
always returns -1). One way would be by adding a struct rpl_DIR which tracks
an fd associated with a DIR, another way would be by adding a DIR* field to the
fchdir metadata then doing a linear search for DIR pointer equality to find
which fd is tied to a given DIR. Since the maximum fd is not all that big, the
O(n) cost of a reverse lookup is not too bad, and seems easier than an O(1)
lookup by overriding struct DIR. But with either approach, it would mean that
fdopendir would need to change to keep the fd open as long as the DIR is open
(rather than the current fdopendir policy of closing the user's fd up front),
closedir would have to be responsible for closing the fd tied to a DIR, and we
would have to decide whether it was better to always open an fd up front in
opendir or just delay until the first dirfd (if any) for tying an fd to a DIR
created by opendir.
So, here's the two patches I plan to apply soon...
From: Eric Blake e...@byu.net
Date: Mon, 31 Aug 2009 06:09:08 -0600
Subject: [PATCH 1/2] fchdir: port to mingw
* m4/fchdir.m4 (gl_FUNC_FCHDIR): Check for mingw bug.
* lib/open.c (open) [FCHDIR_REPLACEMENT]: If directories can't be
opened, then use a substitute.
* lib/sys_stat.in.h (fstat) [REPLACE_OPEN_DIRECTORY]: Declare
replacement.
* lib/fchdir.c (fstat) [REPLACE_OPEN_DIRECTORY]: Implement it.
(_gl_register_fd): No need to check stat if open already filters
all directories.
(fchdir): Fix error condition to match POSIX.
* modules/fchdir (Depends-on): Add sys_stat.
* doc/posix-functions/open.texi (open): Document the limitation.
* modules/fchdir-tests: New file.
* tests/test-fchdir.c: Likewise.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog | 15 +++
doc/posix-functions/open.texi | 11 -
lib/fchdir.c | 63 ++--
lib/open.c| 29 +-
lib/sys_stat.in.h |4 ++
m4/fchdir.m4 | 13 +-
modules/fchdir|1 +
modules/fchdir-tests | 11 +
tests/test-fchdir.c | 91 +
9 files changed, 211 insertions(+), 27 deletions(-)
create mode 100644 modules/fchdir-tests
create mode 100644 tests/test-fchdir.c
diff --git a/ChangeLog b/ChangeLog
index 6e39cef..749215d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,20 @@
2009-08-31 Eric Blake e...@byu.net
+ fchdir: port to mingw
+ * m4/fchdir.m4 (gl_FUNC_FCHDIR): Check for mingw bug.
+ * lib/open.c (open) [FCHDIR_REPLACEMENT]: If directories can't be
+ opened, then use a substitute.
+ * lib/sys_stat.in.h (fstat) [REPLACE_OPEN_DIRECTORY
Re: fchdir on mingw
Eric Blake ebb9 at byu.net writes:
One improvement possible for fdopendir is that
if fchdir is being emulated by gnulib, we can do a query into the fchdir
metadata table to retrieve the directory name associated with an fd, rather
than having to go through save_cwd/fchdir/opendir/restore_cwd.
Here's two patches for that, to be applied on top of fchdir.
From: Eric Blake e...@byu.net
Date: Sun, 30 Aug 2009 18:48:20 -0600
Subject: [PATCH 1/2] fdopendir: split into its own module
* lib/openat.c (fdopendir): Move...
* lib/fdopendir.c: ...into new file.
* modules/fdopendir: New module.
* m4/fdopendir.m4 (gl_FUNC_FDOPENDIR): New file.
* modules/openat (Depends-on): Add fdopendir.
* m4/openat.m4 (gl_FUNC_OPENAT): No longer need to check for
fdopendir here.
* modules/savedir (Depends-on): Only need fdopendir, not full
openat.
* lib/savedir.c (include): Use dirent.h, not openat.h.
* lib/openat.h (fdopendir): Drop prototype.
* lib/dirent.in.h (fdopendir): Provide prototype.
* m4/dirent_h.m4 (gl_DIRENT_H_DEFAULTS): Add replacements.
* modules/dirent (Makefile.am): Substitute them.
* MODULES.html.sh (File system functions): Mention it.
* doc/posix-functions/fdopendir.texi (fdopendir): Likewise.
* modules/fdopendir-tests: New file.
* tests/test-fdopendir.c: Likewise.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog | 20
MODULES.html.sh|1 +
doc/posix-functions/fdopendir.texi |2 +-
lib/dirent.in.h| 17 ++
lib/fdopendir.c| 96
lib/openat.c | 68 -
lib/openat.h |6 +--
lib/savedir.c |3 +-
m4/dirent_h.m4 | 10 ++--
m4/fdopendir.m4| 21
m4/openat.m4 |3 +-
modules/dirent |2 +
modules/fdopendir | 30 +++
modules/fdopendir-tests| 13 +
modules/openat |1 +
modules/savedir|2 +-
tests/test-fdopendir.c | 76
17 files changed, 288 insertions(+), 83 deletions(-)
create mode 100644 lib/fdopendir.c
create mode 100644 m4/fdopendir.m4
create mode 100644 modules/fdopendir
create mode 100644 modules/fdopendir-tests
create mode 100644 tests/test-fdopendir.c
diff --git a/ChangeLog b/ChangeLog
index 869567e..b9b3041 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,25 @@
2009-08-31 Eric Blake e...@byu.net
+ fdopendir: split into its own module
+ * lib/openat.c (fdopendir): Move...
+ * lib/fdopendir.c: ...into new file.
+ * modules/fdopendir: New module.
+ * m4/fdopendir.m4 (gl_FUNC_FDOPENDIR): New file.
+ * modules/openat (Depends-on): Add fdopendir.
+ * m4/openat.m4 (gl_FUNC_OPENAT): No longer need to check for
+ fdopendir here.
+ * modules/savedir (Depends-on): Only need fdopendir, not full
+ openat.
+ * lib/savedir.c (include): Use dirent.h, not openat.h.
+ * lib/openat.h (fdopendir): Drop prototype.
+ * lib/dirent.in.h (fdopendir): Provide prototype.
+ * m4/dirent_h.m4 (gl_DIRENT_H_DEFAULTS): Add replacements.
+ * modules/dirent (Makefile.am): Substitute them.
+ * MODULES.html.sh (File system functions): Mention it.
+ * doc/posix-functions/fdopendir.texi (fdopendir): Likewise.
+ * modules/fdopendir-tests: New file.
+ * tests/test-fdopendir.c: Likewise.
+
fchdir: simplify error handling, and support dup3
* modules/fchdir (Depends-on): Use strdup-posix, not strdup. Add
stdbool, malloc-posix, realloc-posix.
diff --git a/MODULES.html.sh b/MODULES.html.sh
index 3c54cec..027a0bc 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -2451,6 +2451,7 @@ func_all_modules ()
func_module dirfd
func_module double-slash-root
func_module euidaccess
+ func_module fdopendir
func_module file-type
func_module fileblocks
func_module filemode
diff --git a/doc/posix-functions/fdopendir.texi b/doc/posix-
functions/fdopendir.texi
index 1d9827c..81b6991 100644
--- a/doc/posix-functions/fdopendir.texi
+++ b/doc/posix-functions/fdopendir.texi
@@ -4,7 +4,7 @@ fdopendir
POSIX specification: @url
{http://www.opengroup.org/onlinepubs/9699919799/functions/fdopendir.html}
-Gnulib module: openat
+Gnulib module: fdopendir
Portability problems fixed by Gnulib:
@itemize
diff --git a/lib/dirent.in.h b/lib/dirent.in.h
index 15f0245..8930765 100644
--- a/lib/dirent.in.h
+++ b/lib/dirent.in.h
@@ -58,6 +58,23 @@ extern int dirfd (DIR const *dir);
dirfd (d))
#endif
+#if @GNULIB_FDOPENDIR@
+# if !...@have_fdopendir@
+/* Open a directory stream visiting the given directory file
+ descriptor. Return NULL and set errno if fd is not visiting a
+ directory. On success, this function consumes fd
Re: Use $(MKDIR_P) instead of @MKDIR_P@
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Paolo Bonzini on 8/31/2009 7:02 PM: I usually modify CC instead. I never tried for gnulib, but it would look like this: MINGW=$(cygpath -u '\mingw') export PATH=$MINGW/bin:$PATH ./configure --host=i586-pc-mingw32 --prefix=$MINGW \ CC=gcc -mno-cygwin CXX=g++ -mno-cygwin \ I also prefer to modify CC, not CFLAGS, particularly since in cygwin 1.7, there will eventually be a true cross compiler, and all I will have to do is rename CC='gcc -mno-cygwin' to CC=gcc-i686-pc-mingw, without touching CFLAGS. - -- 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/ iEYEARECAAYFAkqcc8UACgkQ84KuGfSFAYADMwCdGjFLUf4pG3ScRKJnkbumm47r JQkAn0I0U5wju+V9qMUBsA5ybCPUvP8v =Ytuj -END PGP SIGNATURE-
Re: fchdir on mingw
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 8/31/2009 5:38 PM: Notice that this changes the status of fdopendir - whereas it was previously non-multithread safe and could call _exit, the mingw version of fdopendir is now threadsafe and avoids _exit. Well, not entirely threadsafe. There is a window between when rpl_open() does a stat() to learn that name is a directory, and when fchdir/fdopendir (via chdir()/opendir()) uses that name later on. It's not as pronounced on mingw, where you can't rename an in-use directory (and we could even open up a directory HANDLE to prevent users from changing it behind our backs), but it probably is asking for problems on Unix if another process is actively changing the same hierarchy as what we just tied to the fchdir metadata table. So on second thought, for systems with fchdir and which can open() directories without help, I'd rather stick with save_cwd/fchdir/opendir/restore_cwd rather than risk an optimized fdopendir that exposes the user to a multi-process data race. At least with the fchdir approach to fdopendir, you have limited your problems only to a race between threads in a single process, rather than something exploitable by other processes. - -- 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/ iEYEARECAAYFAkqceEYACgkQ84KuGfSFAYCkQACeJsHocH5U2USkxI9aP/HG2B8f kVoAoIPtnTxbHHFgX7RnX8VQuoEC+7sE =TAb9 -END PGP SIGNATURE-
Re: mingw test cleanups
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 8/31/2009 10:47 AM: I will be pushing these later today as prerequisites to fixing fchdir and splitting fdopendir into its own module. This series avoids some compilation warnings due to missing chmod on mingw, and allows me to run gnulib testsuites under cygwin while cross-compiling to mingw: The above looks fine. However, there are two missing $ signs below: Pushed, with that typo corrected. - -- 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/ iEYEARECAAYFAkqcoe8ACgkQ84KuGfSFAYCArgCfRVery3IwLsT0aY2WqGGTpJoO 75QAnA4du/aoe/0Oh3JXllYeG1FHBZM9 =YuS/ -END PGP SIGNATURE-
Re: fchdir on mingw
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 8/31/2009 3:35 PM: One of the main reasons the fchdir module was added was so that the openat module (and friends) would compile on mingw. Well, it turns out that mingw has the (documented) limitation that open(dir,O_RDONLY) fails with EACCES, so rpl_open was never registering an fd visiting a directory in the first place, and the fchdir replacement (and thus openat and fts) failed to run on mingw. The first patch improves things by adding a fchdir unit test, and teaches rpl_open how to fake out opening a directory (the end result of using /dev/null under the hood means that read() on the fd gives the same EOF results as on Linux; and replacing fstat() means we can hide the fact that we used a dummy). Pushed the first one; I'll wait a bit longer for any more review before pushing the second (since it changes the implementation of fchdir) or the subsequent fdopendir patches. Subject: [PATCH 1/2] fchdir: port to mingw * m4/fchdir.m4 (gl_FUNC_FCHDIR): Check for mingw bug. * lib/open.c (open) [FCHDIR_REPLACEMENT]: If directories can't be opened, then use a substitute. * lib/sys_stat.in.h (fstat) [REPLACE_OPEN_DIRECTORY]: Declare replacement. * lib/fchdir.c (fstat) [REPLACE_OPEN_DIRECTORY]: Implement it. (_gl_register_fd): No need to check stat if open already filters all directories. (fchdir): Fix error condition to match POSIX. * modules/fchdir (Depends-on): Add sys_stat. * doc/posix-functions/open.texi (open): Document the limitation. * modules/fchdir-tests: New file. * tests/test-fchdir.c: Likewise. - -- 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/ iEYEARECAAYFAkqcolwACgkQ84KuGfSFAYBCWwCcCkOO6YkzMbTphFCsHIqbs474 PEAAoJN4VaMMuLSQg4cLTUUftfzSTWK0 =rJu1 -END PGP SIGNATURE-
Re: fix bug in glibc 2.7 macro implementation of strtok_r.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Ben Pfaff on 8/31/2009 6:56 AM: The reason that I'm submitting it is that I discovered this glibc bug myself, and it's been bothering me ever since that gnulib doesn't detect and work around it, because it seems very easy to trigger. Seems like a worthwhile patch to me. However, since it appears that is only triggered based on your choice of -O, are we guaranteed that your .m4 snippet will always flush it out, even if the user later passes a different -O setting via 'make CFLAGS=...'? +++ b/doc/posix-functions/strtok_r.texi @@ -9,6 +9,9 @@ Gnulib module: strtok_r Portability problems fixed by Gnulib: @itemize @item +The macro version of this function segfaults on some inputs on some +versions of glibc. In the bug report, Ulrich says it is fixed upstream; can we list which glibc version that will be (ie. 2.11)? + if test $gl_cv_buggy_strtok_r_macro = yes; then + HAVE_BUGGY_STRTOK_R_MACRO=1 Naming convention - I'm wondering if it would be better to use the name FUNC_STRTOK_R_MACRO_BROKEN. Using category (FUNC), name (STRTOK_R), then characteristic (MACRO_BROKEN) is the form suggested by the autoconf manual. - -- 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/ iEYEARECAAYFAkqdBREACgkQ84KuGfSFAYBh8QCgnk5/l+2vjGJKQLp0X1cHnl+M 1BwAoLbQkP+Eu6GFL6nJn3z8moOU+D6w =eYcQ -END PGP SIGNATURE-
Re: fchdir on mingw
Eric Blake ebb9 at byu.net writes:
+/* Return stat information about FD in STATBUF. Needed when
+ rpl_open() used a dummy file to work around an open() that can't
+ normally visit directories. */
+#if REPLACE_OPEN_DIRECTORY
+int
+rpl_fstat (int fd, struct stat *statbuf)
+{
+ if (0 = fd fd = dirs_allocated dirs[fd].name != NULL)
+return stat (dirs[fd].name, statbuf);
Ouch. Off-by-one error, which can lead to segfault on mingw when fd ==
dirs_allocated. Here's the correction, along with the rest of my rebased
pending patches. Any comments before I apply? Stay tuned for dirent-safer
built on top of this.
Eric Blake (5):
fchdir: fix off-by-one bug in previous patch
fchdir: simplify error handling, and support dup3
fchdir: use more consistent macro convention
fdopendir: split into its own module
fdopendir: optimize on mingw
From 2fc6c30f4fdda7b373a16dd849cfebcc4cc7669f Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Tue, 1 Sep 2009 10:06:44 -0600
Subject: [PATCH 1/5] fchdir: fix off-by-one bug in previous patch
* lib/fchdir.c (rpl_fstat): Use correct bounds.
(_gl_unregister_fd): Delete useless if.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog|6 ++
lib/fchdir.c |5 ++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index d53ed5d..7b67cf4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2009-09-01 Eric Blake e...@byu.net
+
+ fchdir: fix off-by-one bug in previous patch
+ * lib/fchdir.c (rpl_fstat): Use correct bounds.
+ (_gl_unregister_fd): Delete useless if.
+
2009-09-01 Simon Josefsson si...@josefsson.org
* modules/arpa_inet: Use $(MKDIR_P) instead of @mkdi...@.
diff --git a/lib/fchdir.c b/lib/fchdir.c
index cedbcde..170505f 100644
--- a/lib/fchdir.c
+++ b/lib/fchdir.c
@@ -93,8 +93,7 @@ _gl_unregister_fd (int fd)
{
if (fd = 0 fd dirs_allocated)
{
- if (dirs[fd].name != NULL)
- free (dirs[fd].name);
+ free (dirs[fd].name);
dirs[fd].name = NULL;
dirs[fd].saved_errno = ENOTDIR;
}
@@ -126,7 +125,7 @@ _gl_register_fd (int fd, const char *filename)
int
rpl_fstat (int fd, struct stat *statbuf)
{
- if (0 = fd fd = dirs_allocated dirs[fd].name != NULL)
+ if (0 = fd fd dirs_allocated dirs[fd].name != NULL)
return stat (dirs[fd].name, statbuf);
return fstat (fd, statbuf);
}
--
1.6.3.2
From 21fc064f72d5acb6c326022b4b77818703b1c897 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Mon, 31 Aug 2009 14:20:03 -0600
Subject: [PATCH 2/5] fchdir: simplify error handling, and support dup3
* modules/fchdir (Depends-on): Use strdup-posix, not strdup. Add
stdbool, malloc-posix, realloc-posix.
* lib/fchdir.c (struct dir_info_t): Delete saved_errno.
(ensure_dirs_slot): Return false on allocation failure.
(rpl_dup2): Delete.
(_gl_register_dup): New function.
(_gl_unregister_fd, rpl_opendir, rpl_dup): Update callers.
(_gl_register_fd): Close fd on allocation failure.
* lib/fcntl.in.h (_gl_register_fd): Update signature.
* lib/unistd.in.h (_gl_register_dup) [FCHDIR_REPLACEMENT]: New
prototype.
(rpl_dup2_fchdir): Delete prototype.
* lib/open.c (open): Update caller.
* lib/dup2.c (dup2): Track fchdir metadata.
* lib/dup3.c (dup3): Likewise.
* m4/dup2.m4 (gl_REPLACE_DUP2): New macro.
* m4/fchdir.m4 (gl_FUNC_FCHDIR): Use it.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog | 19 ++
lib/dup2.c | 16 -
lib/dup3.c | 11
lib/fchdir.c| 177 ---
lib/fcntl.in.h |2 +-
lib/open.c |4 +-
lib/unistd.in.h |8 +--
m4/dup2.m4 | 12 +++-
m4/fchdir.m4|4 +-
modules/fchdir |5 +-
10 files changed, 141 insertions(+), 117 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 7b67cf4..6a0b18e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,24 @@
2009-09-01 Eric Blake e...@byu.net
+ fchdir: simplify error handling, and support dup3
+ * modules/fchdir (Depends-on): Use strdup-posix, not strdup. Add
+ stdbool, malloc-posix, realloc-posix.
+ * lib/fchdir.c (struct dir_info_t): Delete saved_errno.
+ (ensure_dirs_slot): Return false on allocation failure.
+ (rpl_dup2): Delete.
+ (_gl_register_dup): New function.
+ (_gl_unregister_fd, rpl_opendir, rpl_dup): Update callers.
+ (_gl_register_fd): Close fd on allocation failure.
+ * lib/fcntl.in.h (_gl_register_fd): Update signature.
+ * lib/unistd.in.h (_gl_register_dup) [FCHDIR_REPLACEMENT]: New
+ prototype.
+ (rpl_dup2_fchdir): Delete prototype.
+ * lib/open.c (open): Update caller.
+ * lib/dup2.c (dup2): Track fchdir metadata.
+ * lib/dup3.c (dup3): Likewise.
+ * m4/dup2.m4 (gl_REPLACE_DUP2): New macro.
+ * m4/fchdir.m4 (gl_FUNC_FCHDIR): Use it.
+
fchdir: fix off-by-one bug in previous patch
Re: need opendir_safer, dirent--.h
Eric Blake ebb9 at byu.net writes:
I think we need to implement opendir_safer, alongside all the other
*_safer modules. Otherwise, opendir can end up placing an open directory
fd in one of the standard slots, and end up interfering with the intent of
all the other *_safer wrappers.
And here's at least one use case where it matters:
$ find dir -mindepth 1 -ok echo {} \; -
echo ... dir/sub ?
$ echo $?
0
Here's the preliminary patch series, to be applied on top of my
fchdir/fdopendir series. However, since we are also lacking openat_safer, it
looks like fts will STILL pollute the standard fds. I'll have to add in
another patch for openat-safer, then test with findutils, before calling this
series ready for prime-time.
From: Eric Blake e...@byu.net
Date: Tue, 1 Sep 2009 07:41:28 -0600
Subject: [PATCH 1/2] dirent-safer: new module
* modules/dirent-safer: New file.
* lib/dirent--.h: Likewise.
* lib/dirent-safer.h: Likewise.
* lib/opendir-safer.c: Likewise.
* m4/dirent-safer.m4: Likewise.
* MODULES.html.sh (Enhancements for POSIX:2008): Mention it.
* modules/dirent-safer-tests: New test.
* tests/test-dirent-safer.c: New file.
* lib/fdopendir.c (includes): Ensure fdopendir is also safe.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog | 11 +
MODULES.html.sh|1 +
lib/dirent--.h | 23 ++
lib/dirent-safer.h | 22 +
lib/fdopendir.c|4 ++
lib/opendir-safer.c| 68
m4/dirent-safer.m4 | 11 +
modules/dirent-safer | 28
modules/dirent-safer-tests | 11 +
tests/test-dirent-safer.c | 106
10 files changed, 285 insertions(+), 0 deletions(-)
create mode 100644 lib/dirent--.h
create mode 100644 lib/dirent-safer.h
create mode 100644 lib/opendir-safer.c
create mode 100644 m4/dirent-safer.m4
create mode 100644 modules/dirent-safer
create mode 100644 modules/dirent-safer-tests
create mode 100644 tests/test-dirent-safer.c
diff --git a/ChangeLog b/ChangeLog
index ed1736a..44bd320 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
2009-09-01 Eric Blake e...@byu.net
+ dirent-safer: new module
+ * modules/dirent-safer: New file.
+ * lib/dirent--.h: Likewise.
+ * lib/dirent-safer.h: Likewise.
+ * lib/opendir-safer.c: Likewise.
+ * m4/dirent-safer.m4: Likewise.
+ * MODULES.html.sh (Enhancements for POSIX:2008): Mention it.
+ * modules/dirent-safer-tests: New test.
+ * tests/test-dirent-safer.c: New file.
+ * lib/fdopendir.c (includes): Ensure fdopendir is also safe.
+
fdopendir: optimize on mingw
* lib/unistd.in.h (_gl_directory_name): New prototype.
* lib/fchdir.c (_gl_directory_name): Implement it.
diff --git a/MODULES.html.sh b/MODULES.html.sh
index 027a0bc..bb99642 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -2394,6 +2394,7 @@ func_all_modules ()
func_begin_table
func_module chdir-long
+ func_module dirent-safer
func_module dirname
func_module getopt
func_module iconv_open-utf
diff --git a/lib/dirent--.h b/lib/dirent--.h
new file mode 100644
index 000..088aafd
--- /dev/null
+++ b/lib/dirent--.h
@@ -0,0 +1,23 @@
+/* Like dirent.h, but redefine some names to avoid glitches.
+
+ Copyright (C) 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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see http://www.gnu.org/licenses/. */
+
+/* Written by Eric Blake. */
+
+#include dirent-safer.h
+
+#undef opendir
+#define opendir opendir_safer
diff --git a/lib/dirent-safer.h b/lib/dirent-safer.h
new file mode 100644
index 000..0debe26
--- /dev/null
+++ b/lib/dirent-safer.h
@@ -0,0 +1,22 @@
+/* Invoke dirent-like functions, but avoid some glitches.
+
+ Copyright (C) 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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details
Re: fts: slightly more robust
Jim Meyering jim at meyering.net writes:
I suspect that it'd be very hard to trigger these close failures,
but it's better to be safe.
While we're visiting fts, how about this patch? POSIX 2008 is clear that
opendir should use O_DIRECTORY, so opendirat should probably do likewise. Do
we want to obey the FIXME and make opendirat an independent module?
Meanwhile, fts.c also has a diropen function which uses
O_DIRECTORY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW. Technically, the latter three are
pointless given O_DIRECTORY (a directory is not a tty, is not a symlink, and
does not block). For two of the three, POSIX appears to allow the mix
(O_NOCTTY is silently ignored; and the combination of O_DIRECTORY|O_NOFOLLOW on
a symlink must produce failure, although POSIX is ambiguous whether the failure
will be ELOOP or ENOTDIR). But for the third, POSIX is explicit that the use
of O_NONBLOCK on anything other than a fifo, block, or character device, the
results are unspecified.
I can see why we are passing all the flags - if there is a kernel that supports
some of the flags but not O_DIRECTORY, we have at least minimized other
problems if we indeed attempt to open a non-directory. But do we really want
to be risking the unspecified behavior of O_NONBLOCK on a directory? Or should
we ask the Austin group to clarify that O_NONBLOCK is safely ignored on
directories? And should I be adding any (or all) of these three safety flags
to opendirat?
I audited for other uses of fdopendir that might be lacking a useful
O_DIRECTORY, but didn't find any. In getcwd.c, the fd passed to fdopendir is
always created via openat(fd,..,O_RDONLY), and since .. is already
guaranteed to be a directory, the use of O_DIRECTORY would be redundant. Any
clients of savedir.c's fdsavedir are also candidates, but there weren't any
within gnulib.
From: Eric Blake e...@byu.net
Date: Tue, 1 Sep 2009 14:06:37 -0600
Subject: [PATCH] fts: make directory fds more robust
* lib/fts.c (opendirat): Specify O_DIRECTORY.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |3 +++
lib/fts.c |2 +-
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index a16fc6e..67c3359 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
2009-09-01 Eric Blake e...@byu.net
+ fts: make directory fds more robust
+ * lib/fts.c (opendirat): Specify O_DIRECTORY.
+
dirent-safer: use it
* lib/backupfile.c (includes): Use dirent--.h, since
numbered_backup can write to stderr during readdir.
diff --git a/lib/fts.c b/lib/fts.c
index 6373e44..6522d75 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -310,7 +310,7 @@ static inline DIR *
internal_function
opendirat (int fd, char const *dir)
{
- int new_fd = openat (fd, dir, O_RDONLY);
+ int new_fd = openat (fd, dir, O_RDONLY | O_DIRECTORY);
DIR *dirp;
if (new_fd 0)
--
1.6.3.2
Re: fts: slightly more robust
Jim Meyering jim at meyering.net writes:
While we're visiting fts, how about this patch? POSIX 2008 is clear that
opendir should use O_DIRECTORY, so opendirat should probably do likewise.
There is no need for O_DIRECTORY in that function, since the only thing
it does with the resulting file descriptor is to apply fdopendir, and
fdopendir will fail with ENOTDIR when fd refers to a non-directory.
Yes, fdopendir will reject non-directories, but the point of using O_DIRECTORY
up front is to avoid blocking on a FIFO prior to reaching the fdopendir.
Do we want to obey the FIXME and make opendirat an independent module?
Sure, if there is another user.
Is there?
Not so far, so I won't bother.
O_DIRECTORY|O_NOFOLLOW on
a symlink must produce failure, although POSIX is ambiguous whether the
failure
will be ELOOP or ENOTDIR)
I stand corrected. The POSIX wording is that O_DIRECTORY fails if path does
not name a directory, but elsewhere in POSIX, that same phrase applies to
symlinks that point to directories. In other words:
open(link-to-dir,O_RDONLY|O_DIRECTORY) - passes
open(link-to-file,O_RDONLY|O_DIRECTORY) - fails with ENOTDIR
open(link-to-dir,O_RDONLY|O_DIRECTORY|O_NOFOLLOW) - fails with ELOOP
open(link-to-file,O_RDONLY|O_DIRECTORY|O_NOFOLLOW) - fails with either ELOOP
or ENOTDIR
You might as well add all four,
O_DIRECTORY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW
on the off-chance that O_DIRECTORY makes the earlier openat
fail, and it fails with a more useful errno value than fdopendir.
So blindly adding O_NOFOLLOW for opendirat is wrong.
Here's the latest draft of my patch.
From 0145db62dc0f4fc77ed98119e40d256d95aa21cb Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Tue, 1 Sep 2009 14:06:37 -0600
Subject: [PATCH] fts: make directory fds more robust
* lib/fts.c (O_DIRECTORY): Let fcntl.h take care of this.
(opendirat): Specify O_DIRECTORY, and add fallbacks for safety.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |6 ++
lib/fts.c |7 ++-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index ee81a23..58decf8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2009-09-02 Eric Blake e...@byu.net
+ fts: make directory fds more robust
+ * lib/fts.c (O_DIRECTORY): Let fcntl.h take care of this.
+ (opendirat): Specify O_DIRECTORY, and add fallbacks for safety.
+
+2009-09-02 Eric Blake e...@byu.net
+
backupfile, chdir-long, fts, savedir: make safer
* lib/backupfile.c (includes): Use dirent--.h, since
numbered_backup can write to stderr during readdir.
diff --git a/lib/fts.c b/lib/fts.c
index 7616c6f..ebf66fc 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -228,10 +228,6 @@ static void free_dir (FTS *fts) {}
# define SIZE_MAX ((size_t) -1)
#endif
-#ifndef O_DIRECTORY
-# define O_DIRECTORY 0
-#endif
-
#define ISDOT(a) (a[0] == '.' (!a[1] || (a[1] == '.' !a[2])))
#define STREQ(a, b)(strcmp ((a), (b)) == 0)
@@ -309,7 +305,8 @@ static inline DIR *
internal_function
opendirat (int fd, char const *dir)
{
- int new_fd = openat (fd, dir, O_RDONLY);
+ int new_fd = openat (fd, dir,
+ O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK);
DIR *dirp;
if (new_fd 0)
--
1.6.3.2
Re: need opendir_safer, dirent--.h
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/2/2009 12:55 AM: Here's the preliminary patch series, to be applied on top of my fchdir/fdopendir series. However, since we are also lacking openat_safer, it looks like fts will STILL pollute the standard fds. I'll have to add in another patch for openat-safer, then test with findutils, before calling this series ready for prime-time. And here's openat-safer, plus a rework of making fts safer. With these patches, plus a patch to findutils to consistently use the *_safer functions, there are no more collisions with stdin in my original findutils example (tested with both cygwin 1.5 which lacks openat and fdopendir, and cygwin 1.7 which has both), so I'll be pushing the series once I rebase on top of Ralf's .m4 cleanups. Next, I'll be working on moving the declaration of openat and friends into their respective POSIX 2008 headers (fcntl.h, sys/stat.h, unistd.h), rather than requiring standard programs to use openat.h. And eventually, we'll need to provide an rpl_openat even on Linux that has openat, in order to provide O_CLOEXEC semantics on older kernels compiled against newer glibc, as part of the suite of CLOEXEC improvements. - -- 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/ iEYEARECAAYFAkqewOIACgkQ84KuGfSFAYBtfwCfSVnDoro8sJo7cpVV/yx/ZMrp 5dUAn2lbmSneGHIlhWuoOqDeXw9fYCmv =diWp -END PGP SIGNATURE- From bdd72e423478ccb9fd93d47d136ffae9340b5384 Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Wed, 2 Sep 2009 06:07:54 -0600 Subject: [PATCH 1/2] openat-safer: new module * modules/openat-safer: New file. * lib/openat-safer.c: Likewise. * m4/fcntl-safer.m4 (gl_OPENAT_SAFER): New macro. * lib/fcntl-safer.h (openat_safer): Declare. * lib/fcntl--.h (openat): Override. * MODULES.html.sh (File descriptor based I/O): Mention it. * lib/openat.h: Add double-inclusion guards. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog| 11 +++ MODULES.html.sh |1 + lib/fcntl--.h|8 +++- lib/fcntl-safer.h|6 +- lib/openat-safer.c | 47 +++ lib/openat.h |5 + m4/fcntl-safer.m4|8 +++- modules/openat-safer | 28 8 files changed, 111 insertions(+), 3 deletions(-) create mode 100644 lib/openat-safer.c create mode 100644 modules/openat-safer diff --git a/ChangeLog b/ChangeLog index e9fa04a..2b56545 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2009-09-02 Eric Blake e...@byu.net + + openat-safer: new module + * modules/openat-safer: New file. + * lib/openat-safer.c: Likewise. + * m4/fcntl-safer.m4 (gl_OPENAT_SAFER): New macro. + * lib/fcntl-safer.h (openat_safer): Declare. + * lib/fcntl--.h (openat): Override. + * MODULES.html.sh (File descriptor based I/O): Mention it. + * lib/openat.h: Add double-inclusion guards. + 2009-09-01 Eric Blake e...@byu.net dirent-safer: new module diff --git a/MODULES.html.sh b/MODULES.html.sh index bb99642..bcecc55 100755 --- a/MODULES.html.sh +++ b/MODULES.html.sh @@ -2509,6 +2509,7 @@ func_all_modules () func_begin_table func_module fcntl-safer + func_module openat-safer func_module safe-read func_module safe-write func_module full-read diff --git a/lib/fcntl--.h b/lib/fcntl--.h index 5a6a879..9e311ce 100644 --- a/lib/fcntl--.h +++ b/lib/fcntl--.h @@ -1,6 +1,6 @@ /* Like fcntl.h, but redefine some names to avoid glitches. - Copyright (C) 2005 Free Software Foundation, Inc. + Copyright (C) 2005, 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 @@ -25,3 +25,9 @@ #undef creat #define creat creat_safer + +#if GNULIB_OPENAT_SAFER +# include openat.h /* FIXME - fcntl.h should be sufficient. */ +# undef openat +# define openat openat_safer +#endif diff --git a/lib/fcntl-safer.h b/lib/fcntl-safer.h index 99f3865..2b10ba6 100644 --- a/lib/fcntl-safer.h +++ b/lib/fcntl-safer.h @@ -1,6 +1,6 @@ /* Invoke fcntl-like functions, but avoid some glitches. - Copyright (C) 2005 Free Software Foundation, Inc. + Copyright (C) 2005, 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 @@ -21,3 +21,7 @@ int open_safer (char const *, int, ...); int creat_safer (char const *, mode_t); + +#if GNULIB_OPENAT_SAFER +int openat_safer (int, char const *, int, ...); +#endif diff --git a/lib/openat-safer.c b/lib/openat-safer.c new file mode 100644 index 000
Re: fts: slightly more robust
Eric Blake ebb9 at byu.net writes:
Here's the latest draft of my patch.
While we're at it, I noticed via findutils that fts leaks fds into child
processes. This plugs the fts leak (but completely fixing find also requires a
patch to findutils).
Hmm - POSIX states that fdopendir can, but not must, set the cloexec flag, as
part of consuming the fd. Maybe the gnulib fdopendir module should guarantee
that the cloexec flag is set as part of creating a directory stream, rendering
the first of the three hunks to fts.c redundant? Or maybe keep the fdopendir
module as-is, but create a new module fdopendir-gnu, which goes further and
gives the additional GNU semantics that: fd is changed to cloexec, and dirfd
gives the same fd back (requires tweaking rpl_opendir on mingw to open an fd up
front, and for all other platforms lacking fdopendir it requires writing into
the member variable read by dirfd).
From: Eric Blake e...@byu.net
Date: Wed, 2 Sep 2009 14:44:51 -0600
Subject: [PATCH] fts: avoid leaking fds
* modules/fts (Depends-on): Add cloexec.
* lib/fts.c (opendirat, diropen, fts_build): Set close-on-exec
flag.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |5 +
lib/fts.c | 18 ++
modules/fts |1 +
3 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index e63e020..3933600 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2009-09-02 Eric Blake e...@byu.net
+ fts: avoid leaking fds
+ * modules/fts (Depends-on): Add cloexec.
+ * lib/fts.c (opendirat, diropen, fts_build): Set close-on-exec
+ flag.
+
fts: make directory fds more robust
* lib/fts.c (O_DIRECTORY): Let fcntl.h take care of this.
(opendirat): Specify O_DIRECTORY, and add fallbacks for safety.
diff --git a/lib/fts.c b/lib/fts.c
index ebf66fc..c05eb8b 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -71,6 +71,9 @@ static char sccsid[] = @(#)fts.c 8.6 (Berkeley) 8/14/94;
# include fcntl--.h
# include dirent--.h
# include unistd--.h
+/* FIXME - use fcntl(F_DUPFD_CLOEXEC)/openat(O_CLOEXEC) once they are
+ supported. */
+# include cloexec.h
# include same-inode.h
#endif
@@ -311,6 +314,7 @@ opendirat (int fd, char const *dir)
if (new_fd 0)
return NULL;
+ set_cloexec_flag (new_fd, true);
dirp = fdopendir (new_fd);
if (dirp == NULL)
{
@@ -362,9 +366,12 @@ diropen (FTS const *sp, char const *dir)
int open_flags = (O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK
| (ISSET (FTS_PHYSICAL) ? O_NOFOLLOW : 0));
- return (ISSET (FTS_CWDFD)
- ? openat (sp-fts_cwd_fd, dir, open_flags)
- : open (dir, open_flags));
+ int fd = (ISSET (FTS_CWDFD)
+? openat (sp-fts_cwd_fd, dir, open_flags)
+: open (dir, open_flags));
+ if (0 = fd)
+set_cloexec_flag (fd, true);
+ return fd;
}
FTS *
@@ -1305,7 +1312,10 @@ fts_build (register FTS *sp, int type)
if (nlinks || type == BREAD) {
int dir_fd = dirfd(dirp);
if (ISSET(FTS_CWDFD) 0 = dir_fd)
- dir_fd = dup (dir_fd);
+ {
+ dir_fd = dup (dir_fd);
+ set_cloexec_flag (dir_fd, true);
+ }
if (dir_fd 0 || fts_safe_changedir(sp, cur, dir_fd, NULL)) {
if (nlinks type == BREAD)
cur-fts_errno = errno;
diff --git a/modules/fts b/modules/fts
index f80a827..9509557 100644
--- a/modules/fts
+++ b/modules/fts
@@ -8,6 +8,7 @@ lib/fts-cycle.c
m4/fts.m4
Depends-on:
+cloexec
cycle-check
d-ino
d-type
--
1.6.3.2
errno cleanup
I started this patch while working on openat, but it quickly grew into an independent patch with broader application. In particular, it fixes a performance bug for anyone using chdir-safer and errno in parallel on mingw (where the absence of ELOOP is no longer an accurate indicator of lacking symlinks). I'll be applying this patch soon, to consistently uses the errno module (for everything except for the EACCESS snafu in euidaccess.c which is copied straight out of glibc). Since the errno module does not define ENAMETOOLONG, I'm assuming that it is universally available; hence why the list of lib/*.c files is longer than the list of modules/* edits. From: Eric Blake e...@byu.net Date: Wed, 2 Sep 2009 17:06:43 -0600 Subject: [PATCH] errno: use consistently * lib/c-stack.c (ENOTSUP): errno.h guarantees a definition. * lib/canonicalize-lgpl.c (ENAMETOOLONG): Likewise. * lib/canonicalize.c (ELOOP): Likewise. * lib/inet_ntop.c (EAFNOSUPPORT): Likewise. * lib/inet_pton.c (EAFNOSUPPORT): Likewise. * lib/lchown.c (EOPNOTSUPP): Likewise. * lib/openat-priv.h (ENOSYS, EOPNOTSUPP): Likewise. * lib/savewd.c (ESTALE): Likewise. * lib/settime.c (ENOSYS): Likewise. * lib/utimens.c (ENOSYS): Likewise. * lib/xgethostname.c (ENAMETOOLONG): Likewise. * lib/chdir-safer.c (ELOOP): Likewise. (chdir_no_follow): Use HAVE_READLINK, not ELOOP, as witness. * modules/c-stack (Depends-on): Add errno. * modules/canonicalize (Depends-on): Likewise. * modules/chdir-safer (Depends-on): Likewise. * modules/fdopendir (Depends-on): Likewise. * modules/inet_ntop (Depends-on): Likewise. * modules/inet_pton (Depends-on): Likewise. * modules/lchown (Depends-on): Likewise. * modules/openat (Depends-on): Likewise. * modules/savewd (Depends-on): Likewise. * modules/settime (Depends-on): Likewise. * m4/chdir-safer.m4 (gl_CHDIR_SAFER): Check for readlink. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog | 26 ++ lib/c-stack.c |3 --- lib/canonicalize-lgpl.c |5 + lib/canonicalize.c |3 --- lib/chdir-safer.c | 10 +- lib/inet_ntop.c |6 +- lib/inet_pton.c |6 +- lib/lchown.c| 10 -- lib/openat-priv.h | 22 +- lib/savewd.c|6 +- lib/settime.c | 12 +--- lib/utimens.c | 14 ++ lib/xgethostname.c |8 ++-- m4/chdir-safer.m4 |5 +++-- modules/c-stack |1 + modules/canonicalize|1 + modules/chdir-safer |1 + modules/fdopendir |3 ++- modules/inet_ntop |1 + modules/inet_pton |1 + modules/lchown |1 + modules/openat |1 + modules/savewd |1 + modules/settime |1 + modules/utimens |1 + 25 files changed, 56 insertions(+), 93 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4bb003d..33f1fcc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,31 @@ 2009-09-02 Eric Blake e...@byu.net + errno: use consistently + * lib/c-stack.c (ENOTSUP): errno.h guarantees a definition. + * lib/canonicalize-lgpl.c (ENAMETOOLONG): Likewise. + * lib/canonicalize.c (ELOOP): Likewise. + * lib/inet_ntop.c (EAFNOSUPPORT): Likewise. + * lib/inet_pton.c (EAFNOSUPPORT): Likewise. + * lib/lchown.c (EOPNOTSUPP): Likewise. + * lib/openat-priv.h (ENOSYS, EOPNOTSUPP): Likewise. + * lib/savewd.c (ESTALE): Likewise. + * lib/settime.c (ENOSYS): Likewise. + * lib/utimens.c (ENOSYS): Likewise. + * lib/xgethostname.c (ENAMETOOLONG): Likewise. + * lib/chdir-safer.c (ELOOP): Likewise. + (chdir_no_follow): Use HAVE_READLINK, not ELOOP, as witness. + * modules/c-stack (Depends-on): Add errno. + * modules/canonicalize (Depends-on): Likewise. + * modules/chdir-safer (Depends-on): Likewise. + * modules/fdopendir (Depends-on): Likewise. + * modules/inet_ntop (Depends-on): Likewise. + * modules/inet_pton (Depends-on): Likewise. + * modules/lchown (Depends-on): Likewise. + * modules/openat (Depends-on): Likewise. + * modules/savewd (Depends-on): Likewise. + * modules/settime (Depends-on): Likewise. + * m4/chdir-safer.m4 (gl_CHDIR_SAFER): Check for readlink. + gnulib-comp: reduce configure size * m4/gnulib-common.m4 (AC_REPLACE_FUNCS): Always override, using an idea from autoconf 2.65 while keeping our AC_LIBOBJ overrides. diff --git a/lib/c-stack.c b/lib/c-stack.c index d260b47..1dddeef 100644 --- a/lib/c-stack.c +++ b/lib/c-stack.c @@ -46,9 +46,6 @@ #define _(msgid) gettext (msgid) #include errno.h -#ifndef ENOTSUP -# define ENOTSUP EINVAL -#endif #include signal.h #if ! HAVE_STACK_T ! defined stack_t diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c index 3aaa310..6b5663a 100644 --- a/lib/canonicalize-lgpl.c +++ b/lib
make openat declarations consistent
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Jim, what do you think of this patch, making it possible to avoid including openat.h if you only use the POSIX 2008 interfaces? - -- 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/ iEYEARECAAYFAkqfsgEACgkQ84KuGfSFAYAN0ACdG9ViuIY3IEwQkqvvVt5PZeN5 2r0An3C/ZX8VvOYoPFccpNep+i0ZuQ5x =ZJ0v -END PGP SIGNATURE- From 601bb4476b3e9b8722f79fa05daacb34ae9f6430 Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Wed, 2 Sep 2009 17:14:26 -0600 Subject: [PATCH] openat: declare in POSIX headers * NEWS: Mention this. * modules/openat (configure.ac): Declare witnesses. (Depends-on): Add fcntl-h, sys_stat, unistd. (Include): Mention correct headers. * modules/fcntl-h (Depends-on): Add link-warning. (Files): Add openat.m4. (Makefile.am): Substitute witnesses. * modules/sys_stat (Files, Makefile.am): Likewise. * modules/unistd (Files, Makefile.am): Likewise. * m4/openat.m4 (gl_FUNC_OPENAT, gl_FUNC_FCHOWNAT): Set witnesses. (gl_OPENAT_DEFAULTS): New macro. * m4/fcntl_h.m4 (gl_FCNTL_H_DEFAULTS): Use it. * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Likewise. * m4/sys_stat_h.m4 (gl_SYS_STAT_H_DEFAULTS): Likewise. (SYS_STAT_H): Remove unused variable. * doc/posix-headers/fcntl.texi (fcntl.h): Update content. * lib/fcntl--.h (includes): Remove unneeded header. * lib/openat-safer.c (includes): Likewise. * lib/openat.h (AT_FDCWD, AT_SYMLINK_NOFOLLOW, AT_REMOVEDIR) (openat, fstatat, unlinkat, mkdirat, fchmodat, fchownat): Move to appropriate headers. (__OPENAT_PREFIX): Delete. * lib/fcntl.in.h (openat): Provide declaration. (AT_FDCWD): Fix Solaris bug. (AT_SYMLINK_NOFOLLOW, AT_REMOVEDIR): Provide macros. * lib/sys_stat.in.h (fstatat, mkdirat): Provide declaration. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog| 30 + NEWS | 10 +++- doc/posix-headers/fcntl.texi | 11 - lib/fcntl--.h|1 - lib/fcntl.in.h | 44 + lib/openat-safer.c |1 - lib/openat.h | 45 +- lib/sys_stat.in.h| 35 ++ lib/unistd.in.h | 25 + m4/fcntl_h.m4|2 + m4/openat.m4 | 49 - m4/sys_stat_h.m4 |5 +-- m4/unistd_h.m4 |3 +- modules/fcntl-h |5 modules/openat |7 ++ modules/sys_stat |8 ++- modules/unistd |5 17 files changed, 217 insertions(+), 69 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4eeb067..1a8aca6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,33 @@ +2009-09-03 Eric Blake e...@byu.net + + openat: declare in POSIX headers + * NEWS: Mention this. + * modules/openat (configure.ac): Declare witnesses. + (Depends-on): Add fcntl-h, sys_stat, unistd. + (Include): Mention correct headers. + * modules/fcntl-h (Depends-on): Add link-warning. + (Files): Add openat.m4. + (Makefile.am): Substitute witnesses. + * modules/sys_stat (Files, Makefile.am): Likewise. + * modules/unistd (Files, Makefile.am): Likewise. + * m4/openat.m4 (gl_FUNC_OPENAT, gl_FUNC_FCHOWNAT): Set witnesses. + (gl_OPENAT_DEFAULTS): New macro. + * m4/fcntl_h.m4 (gl_FCNTL_H_DEFAULTS): Use it. + * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Likewise. + * m4/sys_stat_h.m4 (gl_SYS_STAT_H_DEFAULTS): Likewise. + (SYS_STAT_H): Remove unused variable. + * doc/posix-headers/fcntl.texi (fcntl.h): Update content. + * lib/fcntl--.h (includes): Remove unneeded header. + * lib/openat-safer.c (includes): Likewise. + * lib/openat.h (AT_FDCWD, AT_SYMLINK_NOFOLLOW, AT_REMOVEDIR) + (openat, fstatat, unlinkat, mkdirat, fchmodat, fchownat): Move to + appropriate headers. + (__OPENAT_PREFIX): Delete. + * lib/fcntl.in.h (openat): Provide declaration. + (AT_FDCWD): Fix Solaris bug. + (AT_SYMLINK_NOFOLLOW, AT_REMOVEDIR): Provide macros. + * lib/sys_stat.in.h (fstatat, mkdirat): Provide declaration. + 2009-09-02 Eric Blake e...@byu.net errno: use consistently diff --git a/NEWS b/NEWS index 6a364a5..4cd0278 100644 --- a/NEWS +++ b/NEWS @@ -6,7 +6,15 @@ User visible incompatible changes DateModules Changes -2009-09-30 striconveh The functions mem_cd_iconveh and str_cd_iconveh +2009-09-03 openat The include files are standardized to POSIX 2008. +For openat, include fcntl.h
Re: make openat declarations consistent
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/3/2009 6:52 AM: But in the long run, we should provide a replacement function, as we do for all other *at functions. In the *at family, gnulib is still missing: faccessat linkat mknodat mkfifoat readlinkat renameat symlinkat utimensat (well, we use it in utimens if available) I can probably look at doing these; except for renameat (which has two fds to worry about) and utimensat (which requires some time truncation to map onto existing functions), it seems like the rest can reuse the existing at-func.c file for /proc/self/fd and save_cwd/fchdir/func/restore_cwd magic. - -- 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/ iEYEARECAAYFAkqfvlIACgkQ84KuGfSFAYATxQCdEzJXNKIc/Dl1dsq8eJ3xkbZX El4An3hUidBiyc50S28HxjbTlxKhQ+nq =nbg/ -END PGP SIGNATURE-
Re: make openat declarations consistent
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/3/2009 6:52 AM: I'm adjusting it to use faccessat with AT_EACCESS instead. Ouch. Cygwin 1.7 has faccessat but not euidaccess, and it currently has bugs (including no AT_EACCESS support, even though AT_EACCESS is defined). Depending on how quickly those bugs are fixed, both the euidaccess module and my soon-to-come faccessat module will need to deal with them (but I'm hoping that cygwin fixes it first; my argument all along has been that any bug in beta cygwin 1.7.0 is not worth working around in gnulib until cygwin 1.7.1 is a formal release). http://cygwin.com/ml/cygwin/2009-09/msg00109.html That brings up the lack of an faccessat replacement in gnulib. I'll post a module for this shortly. - -- 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/ iEYEARECAAYFAkqgFSgACgkQ84KuGfSFAYAjWwCfc231LdRCO4GBKAzPAASDP8Nl PoMAn3xpcbXX0qp5g378f151n3dOUPI4 =ToNR -END PGP SIGNATURE-
Re: make openat declarations consistent
Eric Blake ebb9 at byu.net writes:
That brings up the lack of an faccessat replacement in gnulib.
I'll post a module for this shortly.
It is feasible for a system to have faccessat but not euidaccess (if they are
complying strictly with POSIX 2008), so we might as well add that to our
euidaccess arsenal.
With regards to the cygwin 1.7 faccessat bug, it would be a separate patch to
add a check for whether faccessat works, and if not, use the stat() fallback
path of euidaccess as well as create rpl_faccessat. But cygwin's faccessat
DOES support AT_SYMLINK_NOFOLLOW to check the access bits of symlinks (not that
it is ever useful in practice), just like Linux, which means we would have to
consider an leuidaccess wrapper that acts like euidaccess except using lstat
instead of stat.
From c71f2501a518d8a22db671a14ebd533aecc12463 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Thu, 3 Sep 2009 11:38:53 -0600
Subject: [PATCH 1/3] openat: make template easier to use
* lib/at-func.c (CALL_FUNC): Allow AT_FUNC_USE_F1_COND and
AT_FUNC_F2 to be undefined.
(VALIDATE_FLAG): New macro; use it to reject bad flags.
(AT_FUNC_USE_F1_COND): Change sense to just flag bit.
* lib/fchmodat.c (AT_FUNC_USE_F1_COND): Adjust.
* lib/fchownat.c (AT_FUNC_USE_F1_COND): Likewise.
* lib/openat.c (AT_FUNC_USE_F1_COND) [fstatat, unlinkat]:
Likewise.
* lib/mkdirat.c (AT_FUNC_F2, AT_FUNC_USE_F1_COND): Delete.
* lib/selinux-at.c (AT_FUNC_F2, AT_FUNC_USE_F1_COND)
[getfileconat, lgetfileconat, setfileconat, lsetfileconat]:
Likewise.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog| 14 ++
lib/at-func.c| 27 +--
lib/fchmodat.c |2 +-
lib/fchownat.c |2 +-
lib/mkdirat.c|2 --
lib/openat.c |4 ++--
lib/selinux-at.c | 16
7 files changed, 39 insertions(+), 28 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index a73cad1..288440e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
2009-09-03 Eric Blake e...@byu.net
+ openat: make template easier to use
+ * lib/at-func.c (CALL_FUNC): Allow AT_FUNC_USE_F1_COND and
+ AT_FUNC_F2 to be undefined.
+ (VALIDATE_FLAG): New macro; use it to reject bad flags.
+ (AT_FUNC_USE_F1_COND): Change sense to just flag bit.
+ * lib/fchmodat.c (AT_FUNC_USE_F1_COND): Adjust.
+ * lib/fchownat.c (AT_FUNC_USE_F1_COND): Likewise.
+ * lib/openat.c (AT_FUNC_USE_F1_COND) [fstatat, unlinkat]:
+ Likewise.
+ * lib/mkdirat.c (AT_FUNC_F2, AT_FUNC_USE_F1_COND): Delete.
+ * lib/selinux-at.c (AT_FUNC_F2, AT_FUNC_USE_F1_COND)
+ [getfileconat, lgetfileconat, setfileconat, lsetfileconat]:
+ Likewise.
+
openat: declare in POSIX headers
* NEWS: Mention this.
* modules/openat (configure.ac): Declare witnesses.
diff --git a/lib/at-func.c b/lib/at-func.c
index c7963fe..620fdaf 100644
--- a/lib/at-func.c
+++ b/lib/at-func.c
@@ -1,5 +1,5 @@
/* Define an at-style functions like fstatat, unlinkat, fchownat, etc.
- Copyright (C) 2006 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
@@ -16,14 +16,27 @@
/* written by Jim Meyering */
-#define CALL_FUNC(F) \
- (AT_FUNC_USE_F1_COND \
+#ifdef AT_FUNC_USE_F1_COND
+# define CALL_FUNC(F) \
+ (flag == AT_FUNC_USE_F1_COND \
? AT_FUNC_F1 (F AT_FUNC_POST_FILE_ARGS)\
: AT_FUNC_F2 (F AT_FUNC_POST_FILE_ARGS))
+# define VALIDATE_FLAG(F) \
+ if (flag ~AT_FUNC_USE_F1_COND) \
+{ \
+ errno = EINVAL; \
+ return -1; \
+}
+#else
+# define CALL_FUNC(F) (AT_FUNC_F1 (F AT_FUNC_POST_FILE_ARGS))
+# define VALIDATE_FLAG(F) /* empty */
+#endif
-/* Call AT_FUNC_F1 or AT_FUNC_F2 (testing AT_FUNC_USE_F1_COND to
- determine which) to operate on FILE, which is in the directory
- open on descriptor FD. If possible, do it without changing the
+/* Call AT_FUNC_F1 to operate on FILE, which is in the directory
+ open on descriptor FD. If AT_FUNC_USE_F1_COND is defined to a value,
+ AT_FUNC_POST_FILE_PARAM_DECLS must inlude a parameter named flag;
+ call AT_FUNC_F2 if FLAG is 0 or fail if FLAG contains more bits than
+ AT_FUNC_USE_F1_COND. If possible, do it without changing the
working directory. Otherwise, resort to using save_cwd/fchdir,
then AT_FUNC_F?/restore_cwd. If either the save_cwd or the restore_cwd
fails, then give a diagnostic and exit nonzero. */
@@ -34,6 +47,8 @@ AT_FUNC_NAME (int fd, char const *file
AT_FUNC_POST_FILE_PARAM_DECLS)
int saved_errno;
int err;
+ VALIDATE_FLAG (flag);
+
if (fd == AT_FDCWD
Re: need opendir_safer, dirent--.h
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 9/2/2009 1:00 PM: And here's openat-safer, plus a rework of making fts safer. Well, if I wouldn't create compilation errors in the process. + * lib/chdir-long.c (includes): Use fcntl--.h, since openat + emulation can write to stderr on failure. + * lib/fts.c (includes) [!_LIBC]: Likewise for opendir and openat. +++ b/lib/fts.c @@ -69,7 +69,7 @@ static char sccsid[] = @(#)fts.c 8.6 (Berkeley) 8/14/94; #if ! _LIBC # include fcntl--.h -# include openat.h +# include dirent--.h We still need openat.h, for the non-standard openat_needs_fchdir. Applying as obvious: - -- 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/ iEYEARECAAYFAkqgmX8ACgkQ84KuGfSFAYAVDgCgvu06lsvan1jxPd3mAyqn2wGD 5lkAn05pR0YXgZ2lDIjkXJ+ZAOalgXN8 =cU5Q -END PGP SIGNATURE- From 2847b62110d877f18a6591d406e064fefff2 Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Thu, 3 Sep 2009 22:34:10 -0600 Subject: [PATCH] fts: fix compilation error * lib/fts.c (includes): Re-add openat.h, for openat_needs_fchdir. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog |4 lib/fts.c |1 + 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index 02e06bd..07fd1fe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2009-09-03 Eric Blake e...@byu.net + fts: fix compilation error + * lib/fts.c (includes): Re-add openat.h, for + openat_needs_fchdir. + faccessat: new module * modules/faccessat: New file. * lib/faccessat.m4: Likewise. diff --git a/lib/fts.c b/lib/fts.c index c05eb8b..2893e66 100644 --- a/lib/fts.c +++ b/lib/fts.c @@ -74,6 +74,7 @@ static char sccsid[] = @(#)fts.c 8.6 (Berkeley) 8/14/94; /* FIXME - use fcntl(F_DUPFD_CLOEXEC)/openat(O_CLOEXEC) once they are supported. */ # include cloexec.h +# include openat.h # include same-inode.h #endif -- 1.6.3.3.334.g916e1
link-follow and mingw
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 This is a pre-requisite to linkat, and implementing POSIX 2008 ln(1) -L and -P options. It will make it possible to decide whether linkat should call plain link, readlink/link, or error out with ENOSYS (on the few remaining systems where link() doesn't allow hardlinks to symlinks), as well as being more accommodating to cross-compilation. It also simplifies the decision for mingw and any other platform that lacks symlinks. I'm going ahead and pushing it, even though it causes a slight semantic change (at least coreutils will need to change an #ifdef to an #if, once it picks up the latest gnulib, now that the variable is tri-stated). Unfortunately, I don't have linkat working yet - if both paths are absolute, or both fds point to the same location (which may require some fstats or even stat(.) in the case of AT_FDCWD), or only one of the two paths is relative, then the existing framework is easy enough to copy. But if both paths are relative, and the fds point to different directories, it requires chdir to one directory (preferably the one closer to /), getcwd to determine the name, then retrying with the use of an absolute name (and hoping not to hit ENAMETOOLONG errors). - -- 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/ iEYEARECAAYFAkqhVSoACgkQ84KuGfSFAYCpDQCePp1M7hiMYPwW+tu0CEv05oyQ 3qkAnR2Qv2wKacJqnn7iHLN5iGqplHI8 =ZjnD -END PGP SIGNATURE- From 1149a55ddf28c24bb4be20778cfe8d00733cdd06 Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Fri, 4 Sep 2009 11:49:02 -0600 Subject: [PATCH] link-follow: accomodate mingw and cross-compilation * m4/link-follow.m4 (gl_AC_FUNC_LINK_FOLLOWS_SYMLINK): Rename... (gl_FUNC_LINK_FOLLOWS_SYMLINK): ...to this. Change cross-compilation results to -1, to make linkat easier to implement when cross-compiling. Trivially support mingw. * modules/link-follow (configure.ac): Call new name. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog |9 + m4/link-follow.m4 | 50 ++ modules/link-follow |2 +- 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index 58a4b35..41c104e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2009-09-04 Eric Blake e...@byu.net + + link-follow: accomodate mingw and cross-compilation + * m4/link-follow.m4 (gl_AC_FUNC_LINK_FOLLOWS_SYMLINK): Rename... + (gl_FUNC_LINK_FOLLOWS_SYMLINK): ...to this. Change + cross-compilation results to -1, to make linkat easier to + implement when cross-compiling. Trivially support mingw. + * modules/link-follow (configure.ac): Call new name. + 2009-09-03 Eric Blake e...@byu.net faccessat: compile replacement diff --git a/m4/link-follow.m4 b/m4/link-follow.m4 index 0c1ea36..eb98c3b 100644 --- a/m4/link-follow.m4 +++ b/m4/link-follow.m4 @@ -1,4 +1,4 @@ -# serial 12 +# serial 13 dnl Run a program to determine whether link(2) follows symlinks. dnl Set LINK_FOLLOWS_SYMLINKS accordingly. @@ -7,14 +7,27 @@ dnl Set LINK_FOLLOWS_SYMLINKS accordingly. # gives unlimited permission to copy and/or distribute it, # with or without modifications, as long as this notice is preserved. -AC_DEFUN([gl_AC_FUNC_LINK_FOLLOWS_SYMLINK], +dnl This macro can be used to emulate POSIX linkat. If +dnl LINK_FOLLOWS_SYMLINKS is 0, link matches linkat(,0), and +dnl linkat(,AT_SYMLINK_FOLLOW) requires a readlink. If it is 1, +dnl link matches linkat(,AT_SYMLINK_FOLLOW), and there is no way +dnl to do linkat(,0) on symlinks (on all other file types, +dnl link() is sufficient). If it is -1, use a runtime test. +AC_DEFUN([gl_FUNC_LINK_FOLLOWS_SYMLINK], [dnl - AC_CACHE_CHECK([whether link(2) dereferences a symlink], -gl_ac_cv_func_link_follows_symlink, - [ -# Create a regular file. -echo conftest.file -AC_RUN_IFELSE([AC_LANG_SOURCE([[ + AC_CHECK_FUNCS_ONCE([readlink]) + dnl Mingw lacks link, although gnulib provides a good replacement. + dnl However, it also lacks symlink, so there's nothing to test in + dnl the first place, and no reason to need to distinguish between + dnl linkat variants. So, we set LINK_FOLLOWS_SYMLINKS to 0. + gl_link_follows_symlinks=0 # assume GNU behavior + if test $ac_cv_func_readlink = yes; then +AC_CACHE_CHECK([whether link(2) dereferences a symlink], + gl_cv_func_link_follows_symlink, +[ + # Create a regular file. + echo conftest.file + AC_RUN_IFELSE([AC_LANG_SOURCE([[ # include sys/types.h # include sys/stat.h # include unistd.h @@ -50,13 +63,18 @@ AC_DEFUN([gl_AC_FUNC_LINK_FOLLOWS_SYMLINK], return SAME_INODE (sb_hard, sb_file) ? 0 : 1
Re: brand new failure: ln/misc
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/4/2009 3:26 PM: I don't have time to investigate right now, so... == GNU coreutils 7.5.55-860a3: tests/test-suite.log == 1 of 350 tests failed. (36 tests were not run). Bummer. I think I see the root cause - I botched the logic in link-follow.m4 (it didn't help that a compilation failure looked the same as desired status); so I'm applying this to gnulib, plus a submodule bump in coreutils (but I'll run more testing there first). - -- 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/ iEYEARECAAYFAkqhivAACgkQ84KuGfSFAYDz8wCfYdOjGLocmVWXrT2UVKt2qk9k ksgAoMoSv9QJeG83QxVbZehV/qyDq437 =n6kH -END PGP SIGNATURE- From 8a0efd04dc9583c96b4a0d3c311ff216e07dcdcd Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Fri, 4 Sep 2009 15:40:22 -0600 Subject: [PATCH] link-follow: fix logic bug in prior patch * m4/link-follow.m4 (gl_FUNC_LINK_FOLLOWS_SYMLINK): Fix bug that reversed sense of yes and no in prior patch. Avoid confusing compilation failure with desired semantics. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog |5 + m4/link-follow.m4 | 12 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index e63d09e..193ec7f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2009-09-04 Eric Blake e...@byu.net + link-follow: fix logic bug in prior patch + * m4/link-follow.m4 (gl_FUNC_LINK_FOLLOWS_SYMLINK): Fix bug that + reversed sense of yes and no in prior patch. Avoid confusing + compilation failure with desired semantics. + link-follow: accomodate mingw and cross-compilation * m4/link-follow.m4 (gl_AC_FUNC_LINK_FOLLOWS_SYMLINK): Rename... (gl_FUNC_LINK_FOLLOWS_SYMLINK): ...to this. Change diff --git a/m4/link-follow.m4 b/m4/link-follow.m4 index eb98c3b..48885ea 100644 --- a/m4/link-follow.m4 +++ b/m4/link-follow.m4 @@ -1,4 +1,4 @@ -# serial 13 +# serial 14 dnl Run a program to determine whether link(2) follows symlinks. dnl Set LINK_FOLLOWS_SYMLINKS accordingly. @@ -60,17 +60,17 @@ AC_DEFUN([gl_FUNC_LINK_FOLLOWS_SYMLINK], /* If the dev/inode of hard and file are the same, then the link call followed the symlink. */ - return SAME_INODE (sb_hard, sb_file) ? 0 : 1; + return SAME_INODE (sb_hard, sb_file) ? 1 : 0; } ]])], - [gl_cv_func_link_follows_symlink=yes], - [gl_cv_func_link_follows_symlink=no], + [gl_cv_func_link_follows_symlink=no], dnl GNU behavior + [gl_cv_func_link_follows_symlink=yes], dnl Followed link/compile failed [gl_cv_func_link_follows_symlink=unknown] dnl We're cross compiling. ) ]) case $gl_cv_func_link_follows_symlink in - yes) ;; - no) gl_link_follows_symlinks=1 ;; + yes) gl_link_follows_symlinks=1 ;; + no) ;; # already defaulted to 0 *) gl_link_follows_symlinks=-1 ;; esac fi -- 1.6.3.3.334.g916e1
test-openat-safer failure on older kernel
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On on older machine (2.6.16.29 kernel, glibc 3.4.6), when using /proc
emulation, openat(fd,,O_RDONLY) was accidentally succeeding in opening a
copy of /proc/self/fd/n (ie. the directory pointed to by fd) instead of
failing with ENOENT. Fixed as follows. Fortunately, in a quick audit, I
didn't see any code path in fts or coreutils that seems like it would pass
an empty argument to openat, which is why my test-openat-safer was the
first thing to catch this in nearly 3 years of use.
- --
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/
iEYEARECAAYFAkqh2voACgkQ84KuGfSFAYBxOQCcD/+S3vNkOQ9VOM1bxqhwwzhV
BH4AoMwaMEkZX1Vr2IpVcQkzRlZq3yWU
=0dhK
-END PGP SIGNATURE-
From c46ae39631773a7fdae7d171cc4ef0bf2123efff Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Fri, 4 Sep 2009 21:22:21 -0600
Subject: [PATCH] openat: fail with ENOENT on empty name
* lib/openat-proc.c (openat_proc_name): Special-case the empty
buffer.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |4
lib/openat-proc.c |7 +++
2 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 193ec7f..ecbf16e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2009-09-04 Eric Blake e...@byu.net
+ openat: fail with ENOENT on empty name
+ * lib/openat-proc.c (openat_proc_name): Special-case the empty
+ buffer.
+
link-follow: fix logic bug in prior patch
* m4/link-follow.m4 (gl_FUNC_LINK_FOLLOWS_SYMLINK): Fix bug that
reversed sense of yes and no in prior patch. Avoid confusing
diff --git a/lib/openat-proc.c b/lib/openat-proc.c
index 8057033..76e1c6d 100644
--- a/lib/openat-proc.c
+++ b/lib/openat-proc.c
@@ -57,6 +57,13 @@ openat_proc_name (char buf[OPENAT_BUFFER_SIZE], int fd, char
const *file)
{
static int proc_status = 0;
+ /* Make sure the caller gets ENOENT when appropriate. */
+ if (!*file)
+{
+ buf[0] = '\0';
+ return buf;
+}
+
if (! proc_status)
{
/* Set PROC_STATUS to a positive value if /proc/self/fd is
--
1.6.3.3.334.g916e1
symlinkat (and readlinkat)
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Here's the next module in my *at series; I figured that we might as well
provide symlinkat and readlinkat in the same .o file, since they either
both work or both fail. I had to tweak at-func.c to support readlinkat's
different return type, and symlinkat's change in parameter style (rather
than duplicate lots of code, I went with a symlink_reversed shim; and I
had to use a static function rather than a macro so that CALL_FUNC would
still compile).
And while faccessat has no test (telling if AT_EACCESS works is rather
difficult without root privileges), at least symlinkat does. It passes on
at least mingw (no symlink), cygwin 1.5 (symlink but no symlinkat,
/proc/self unreliable), cygwin 1.7 (native symlinkat), and older Linux (no
symlinkat, but /proc/self works).
- --
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/
iEYEARECAAYFAkqiqnQACgkQ84KuGfSFAYCTlQCfTVM4Lr4HpH+sYNf+/CjW6lBe
eY8AoLXhqB0nXxQu9YnYZvWY8ersxg0i
=DOX+
-END PGP SIGNATURE-
From b6fa845168b3c9f501ad5ee708cf29fbf37531e2 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Sat, 5 Sep 2009 11:49:15 -0600
Subject: [PATCH] symlinkat: new module
* modules/symlinkat: New file.
* lib/symlinkat.c: Likewise.
* m4/symlinkat.m4 (gl_FUNC_SYMLINKAT): Likewise.
* m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add witnesses.
* modules/unistd (Makefile.am): Use them.
* lib/unistd.in.h (symlinkat, readlinkat): Declare them.
(faccessat) [GNULIB_POSIXCHECK]: Fix typo.
* lib/at-func.c (FUNC_RESULT): New macro, defaulting to int.
* MODULES.html.sh (File system functions): Mention module.
* doc/posix-functions/symlinkat.texi (symlinkat): Likewise.
* doc/posix-functions/readlinkat.texi (readlinkat): Likewise.
* modules/symlinkat-tests: New test.
* tests/test-symlinkat.c: Likewise.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog | 17 +-
MODULES.html.sh |1 +
doc/posix-functions/readlinkat.texi | 20 +-
doc/posix-functions/symlinkat.texi | 12 +++-
lib/at-func.c | 21 +--
lib/symlinkat.c | 106 +++
lib/unistd.in.h | 21 ++-
m4/symlinkat.m4 | 22 +++
m4/unistd_h.m4 |3 +
modules/symlinkat | 29 +
modules/symlinkat-tests | 11 +++
modules/unistd |3 +
tests/test-symlinkat.c | 120 +++
13 files changed, 370 insertions(+), 16 deletions(-)
create mode 100644 lib/symlinkat.c
create mode 100644 m4/symlinkat.m4
create mode 100644 modules/symlinkat
create mode 100644 modules/symlinkat-tests
create mode 100644 tests/test-symlinkat.c
diff --git a/ChangeLog b/ChangeLog
index 43d8983..865d807 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,20 @@
2009-09-05 Eric Blake e...@byu.net
+ symlinkat: new module
+ * modules/symlinkat: New file.
+ * lib/symlinkat.c: Likewise.
+ * m4/symlinkat.m4 (gl_FUNC_SYMLINKAT): Likewise.
+ * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add witnesses.
+ * modules/unistd (Makefile.am): Use them.
+ * lib/unistd.in.h (symlinkat, readlinkat): Declare them.
+ (faccessat) [GNULIB_POSIXCHECK]: Fix typo.
+ * lib/at-func.c (FUNC_RESULT): New macro, defaulting to int.
+ * MODULES.html.sh (File system functions): Mention module.
+ * doc/posix-functions/symlinkat.texi (symlinkat): Likewise.
+ * doc/posix-functions/readlinkat.texi (readlinkat): Likewise.
+ * modules/symlinkat-tests: New test.
+ * tests/test-symlinkat.c: Likewise.
+
test-openat-safer: add more checks
* tests/test-openat-safer.c (main): Check more code paths.
@@ -34,7 +49,7 @@
faccessat: new module
* modules/faccessat: New file.
- * lib/faccessat.m4: Likewise.
+ * lib/faccessat.c: Likewise.
* m4/faccessat.m4 (gl_FUNC_FACCESSAT): Likewise.
* m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add witness.
* modules/unistd (Makefile.am): Use it.
diff --git a/MODULES.html.sh b/MODULES.html.sh
index b60ef61..abfd93b 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -2478,6 +2478,7 @@ func_all_modules ()
func_module savewd
func_module stat-macros
func_module stat-time
+ func_module symlinkat
func_module tmpdir
func_module unlinkdir
func_module utimecmp
diff --git a/doc/posix-functions/readlinkat.texi
b/doc/posix-functions/readlinkat.texi
index 0a3f34b..b64b443 100644
--- a/doc/posix-functions/readlinkat.texi
+++ b/doc/posix-functions/readlinkat.texi
@@ -4,16 +4,28 @@ readlinkat
POSIX specification:
@url{http
Re: mingw test cleanups
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 9/6/2009 11:27 AM: $ cross_compiling=yes gl_cv_func_open_slash=no CC='gcc -mno-cygwin' \ CFLAGS='-Wall -gdwarf-2' ./gnulib-tool --with-tests --test canonicalize ... for the situation where AC_CANONICAL_HOST is mis-detected as cygwin rather than mingw (as in my above gnulib-tool command line) ... The arguments that you are passing to 'configure' here are invalid. Yes, I'm well aware of creating an independent testdir and using a proper cross-compilation environment when things really matter. But my shortcut is generally adequate enough to shave time and limp by with a single step instead of two (it is already time-consuming enough to use gnulib-tool on cygwin, due to slow forks). --- a/lib/chown.c +++ b/lib/chown.c +#else /* !HAVE_CHOWN */ + errno = EOPNOTSUPP; + return -1; +#endif } --- a/lib/lchown.c +++ b/lib/lchown.c +#else /* !HAVE_CHOWN */ + errno = EOPNOTSUPP; + return -1; +#endif } EOPNOTSUPP is an error code related to sockets. I also see it sometimes on NFS mounts. POSIX [1] describes it as Operation not supported on socket. Probably ENOSYS (Function not supported.) is a better error code. ENOSYS does make more sense (even though POSIX allows ENOSYS and EOPNOTSUPP to be equal, it does not require it). You're welcome to make that change, if I don't get to it first. --- a/modules/canonicalize-lgpl-tests +++ b/modules/canonicalize-lgpl-tests @@ -5,9 +5,12 @@ tests/test-canonicalize-lgpl.c Depends-on: configure.ac: +AC_CHECK_FUNCS_ONCE([symlink]) +HAVE_SYMLINK=ac_cv_func_symlink +AC_SUBST([HAVE_SYMLINK]) diff --git a/modules/canonicalize-tests b/modules/canonicalize-tests index 875984e..092b9ef 100644 --- a/modules/canonicalize-tests +++ b/modules/canonicalize-tests @@ -5,9 +5,12 @@ tests/test-canonicalize.c Depends-on: configure.ac: +AC_CHECK_FUNCS_ONCE([symlink]) +HAVE_SYMLINK=ac_cv_func_symlink +AC_SUBST([HAVE_SYMLINK]) Setting the value of the same variable in two different configure.ac snippets is going to break the next time someone will modify one of the two files but forget about the other one. If you cannot move these to a common .m4 file, I would use two different variable names, instead of 2x HAVE_SYMLINK. Yes, I could see the benefit of refactoring m4/readlink.m4 to make it easier to call just a detection portion from other modules without having to depend on the readlink module and compile readlink.c, as well as the creation of m4/symlink.m4 to do likewise. In fact, it probably makes sense to consolidate on a single witness - mingw doesn't support either symlink or readlink, but gnulib provides a readlink replacement, so is it better to use HAVE_SYMLINK or HAVE_READLINK as the witness that symlinks work? - -- 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/ iEYEARECAAYFAkqkDVMACgkQ84KuGfSFAYBWowCgo99U2WiZfhGqwrLSLI/a+AWc X8QAnRpSSNchmmQ+zH/3289cybnrbzeg =SAw6 -END PGP SIGNATURE-
Re: make openat declarations consistent
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 9/6/2009 2:22 PM: Eric Blake wrote: configure.ac: gl_FUNC_OPENAT +gl_MODULE_INDICATOR([openat]) This is not needed for gnulib: There is no use of GNULIB_OPENAT as a C preprocessor macro anywhere in lib/, nor in coreutils. So I propose to remove this unneeded witness. Oh, I see - GNULIB_OPENAT is used as a shell variable for substitution in three .in.h files, but not used as a C macro. So we only need the AC_SUBST (provided by gl_OPENAT_DEFAULTS), not the AC_DEFINE (provided by gl_MODULE_INDICATOR). Yes, okay to make that change. - -- 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/ iEYEARECAAYFAkqkHTYACgkQ84KuGfSFAYDYGACgjgPJU83ifwLOzFceslHu0jgS 9tAAoJtesEW2A9zCiMpTEBs7ejghhWfL =k4tv -END PGP SIGNATURE-
Re: make openat declarations consistent
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 9/6/2009 3:14 PM: No, really, all macros needed for constructing the unistd.h replacement are supposed to sit in m4/unistd_h.m4. Good argument. Another plus is that it separates the blocks for different functions in unistd.in.h and sys_stat.in.h, for legibility. And it will make it easier to support rpl_openat to support workarounds like O_CLOEXEC support. OK to apply? Yes, please. - -- 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/ iEYEARECAAYFAkqkOsAACgkQ84KuGfSFAYCvOwCfYWMUIpQCyagY6LFpxUd3yLv0 1s8AoMCxZ08CbO0aMMf2CYADD73cQZki =Clcd -END PGP SIGNATURE-
Re: make openat declarations consistent
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 9/6/2009 3:14 PM: Additionally, this patch fixes a bug: gl_FUNC_OPENAT did not AC_REQUIRE gl_OPENAT_DEFAULTS. So it could happen that gl_FUNC_OPENAT gets expanded before gl_UNISTD_H_DEFAULTS, and then the variable GNULIB_OPENAT would have the value 0 instead of 1. faccessat and symlinkat had the same bug. Pushing this followup, along with some doc fixes. - -- 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/ iEYEARECAAYFAkqkd5EACgkQ84KuGfSFAYAlJgCfTAOLWqTjXyLVAcnv4set2BXo vWoAoNA29DQqhURHJU95wVv/D4v7PaHu =8xhk -END PGP SIGNATURE- From aa686284617c4695252f39a67c20dee5a28a8408 Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Sun, 6 Sep 2009 20:53:59 -0600 Subject: [PATCH 1/2] faccessat, symlinkat: continue cleanup of previous patch * m4/symlinkat.m4 (gl_FUNC_SYMLINKAT): Ensure dependency order. * m4/faccessat.m4 (gl_FUNC_FACCESSAT): Likewise. * modules/unistd (Makefile.am): Substitute GNULIB_READLINKAT. * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Offer GNULIB_READLINKAT. * modules/symlinkat (configure.ac): Set GNULIB_READLINKAT. * lib/unistd.in.h (readlinkat): Declare if GNULIB_READLINKAT is set. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog | 11 +++ lib/unistd.in.h | 10 +++--- m4/faccessat.m4 |3 ++- m4/symlinkat.m4 |3 ++- m4/unistd_h.m4|3 ++- modules/symlinkat |1 + modules/unistd|1 + 7 files changed, 26 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index f587cc4..e5ebf06 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2009-09-06 Eric Blake e...@byu.net + + faccessat, symlinkat: continue cleanup of previous patch + * m4/symlinkat.m4 (gl_FUNC_SYMLINKAT): Ensure dependency order. + * m4/faccessat.m4 (gl_FUNC_FACCESSAT): Likewise. + * modules/unistd (Makefile.am): Substitute GNULIB_READLINKAT. + * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Offer GNULIB_READLINKAT. + * modules/symlinkat (configure.ac): Set GNULIB_READLINKAT. + * lib/unistd.in.h (readlinkat): Declare if GNULIB_READLINKAT is + set. + 2009-09-06 Bruno Haible br...@clisp.org * lib/sys_stat.in.h (fchmodat): Declare if GNULIB_FCHMODAT is set. diff --git a/lib/unistd.in.h b/lib/unistd.in.h index ed25f69..6503529 100644 --- a/lib/unistd.in.h +++ b/lib/unistd.in.h @@ -193,15 +193,19 @@ int faccessat (int fd, char const *file, int mode, int flag); # if !...@have_symlinkat@ int symlinkat (char const *contents, int fd, char const *file); # endif -# if !...@have_readlinkat@ -ssize_t readlinkat (int fd, char const *file, char *buf, size_t len); -# endif #elif defined GNULIB_POSIXCHECK # undef symlinkat # define symlinkat(c,d,n) \ (GL_LINK_WARNING (symlinkat is not portable - \ use gnulib module symlinkat for portability), \ symlinkat (c, d, n)) +#endif + +#if @GNULIB_READLINKAT@ +# if !...@have_readlinkat@ +ssize_t readlinkat (int fd, char const *file, char *buf, size_t len); +# endif +#elif defined GNULIB_POSIXCHECK # undef readlinkat # define readlinkat(d,n,b,l)\ (GL_LINK_WARNING (faccessat is not portable - \ diff --git a/m4/faccessat.m4 b/m4/faccessat.m4 index 250d97b..0aa9526 100644 --- a/m4/faccessat.m4 +++ b/m4/faccessat.m4 @@ -1,4 +1,4 @@ -# serial 2 +# serial 3 # See if we need to provide faccessat replacement. dnl Copyright (C) 2009 Free Software Foundation, Inc. @@ -12,6 +12,7 @@ AC_DEFUN([gl_FUNC_FACCESSAT], [ AC_REQUIRE([gl_FUNC_OPENAT]) AC_REQUIRE([gl_FUNC_EUIDACCESS]) + AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) AC_CHECK_FUNCS_ONCE([access]) AC_CHECK_FUNCS_ONCE([faccessat]) diff --git a/m4/symlinkat.m4 b/m4/symlinkat.m4 index 50d9954..93980d5 100644 --- a/m4/symlinkat.m4 +++ b/m4/symlinkat.m4 @@ -1,4 +1,4 @@ -# serial 1 +# serial 2 # See if we need to provide symlinkat/readlinkat replacement. dnl Copyright (C) 2009 Free Software Foundation, Inc. @@ -11,6 +11,7 @@ dnl with or without modifications, as long as this notice is preserved. AC_DEFUN([gl_FUNC_SYMLINKAT], [ AC_REQUIRE([gl_FUNC_OPENAT]) + AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) AC_CHECK_FUNCS_ONCE([symlink symlinkat readlinkat]) if test $ac_cv_func_symlinkat = no; then diff --git a/m4/unistd_h.m4 b/m4/unistd_h.m4 index d9781f7..84f0755 100644 --- a/m4/unistd_h.m4 +++ b/m4/unistd_h.m4 @@ -1,4 +1,4 @@ -# unistd_h.m4 serial 23 +# unistd_h.m4 serial 24 dnl Copyright (C) 2006-2009 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission
make lchmodat friendlier
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Jim, what do you think of this patch, so that lchmodat only fails with
ENOSYS when called on a symlink (when called on other file types, it would
now succeed)?
Also, should openat.h provide shortcuts named:
accessat() = faccessat(,0)
euidaccessat() = faccessat(,AT_EACCESS)
statat() = fstatat(,0)
lstatat() = fstatat(,AT_SYMLINK_NOFOLLOW)
to match the existing shortcuts of chownat, lchownat, etc.
- --
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/
iEYEARECAAYFAkqkflQACgkQ84KuGfSFAYCXSACguRsFKUn+gdH52N8Rvdj/Rw6Z
3PUAoKCfDxsUaCVRxgwxZdEZYtfakuf8
=EPIy
-END PGP SIGNATURE-
From d4d4d1cae33f0dcc2829d3fbf12fa0b83bc3c53e Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Sun, 6 Sep 2009 21:08:00 -0600
Subject: [PATCH] openat: with no lchmod, only fail lchmodat on symlinks
* lib/fchmodat.c (lchmod) [!HAVE_LCHMOD]: Perform lstat first, to
minimize failure cases.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |4
lib/fchmodat.c | 11 ++-
2 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 06287d4..03e91c9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2009-09-06 Eric Blake e...@byu.net
+ openat: with no lchmod, only fail lchmodat on symlinks
+ * lib/fchmodat.c (lchmod) [!HAVE_LCHMOD]: Perform lstat first, to
+ minimize failure cases.
+
renameat: new module
* modules/renameat: New file.
* lib/at-func2.c: Likewise.
diff --git a/lib/fchmodat.c b/lib/fchmodat.c
index 53cafe0..11ace83 100644
--- a/lib/fchmodat.c
+++ b/lib/fchmodat.c
@@ -30,7 +30,16 @@
system-supplied declaration. */
# undef lchmod
# define lchmod lchmod_rpl
-static int lchmod (char const *f, mode_t m) { errno = ENOSYS; return -1; }
+static int
+lchmod (char const *f, mode_t m)
+{
+ /* This is slightly racy, but better than nothing. */
+ struct stat st;
+ if (lstat (f, st) == -1 || !S_ISLNK (st.st_mode))
+return chmod (f, m);
+ errno = ENOSYS;
+ return -1;
+}
#endif
/* Solaris 10 has no function like this.
--
1.6.3.3.334.g916e1
Re: make lchmodat friendlier
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/6/2009 11:49 PM: +openat: with no lchmod, only fail lchmodat on symlinks +* lib/fchmodat.c (lchmod) [!HAVE_LCHMOD]: Perform lstat first, to +minimize failure cases. Is this useful to you on Cygwin? Fewer diagnostics? Not for cygwin, since it has lchmod. The drawback is that it makes it harder to perform a run-time test for lchmod support. The test requires a symlink. OK, I see your point. Failing with ENOSYS regardless of whether we _could_ have passed makes it obvious that there is no lchmod ability, and the user has to fall back to chmodat themselves if they know they aren't working on a symlink. Patch withdrawn (unless anyone else can speak up for a reason of why to include it). - -- 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/ iEYEARECAAYFAkqk6k8ACgkQ84KuGfSFAYDpygCePpG8//cW0CiI64q2zryCaC5R GqsAnjvcO8c2JisE4cHqpo6MlPHYHenw =zSe+ -END PGP SIGNATURE-
Re: make lchmodat friendlier
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Jim Meyering on 9/6/2009 11:49 PM:
Also, should openat.h provide shortcuts named:
accessat() = faccessat(,0)
euidaccessat() = faccessat(,AT_EACCESS)
statat() = fstatat(,0)
lstatat() = fstatat(,AT_SYMLINK_NOFOLLOW)
to match the existing shortcuts of chownat, lchownat, etc.
Good idea.
I hesitated to appropriate the names initially, but the combination
of increased readability and orthogonality make it worthwhile.
Done as follows.
Unfortunately, the naming of unlinkat prevents doing:
unlinkat() = unlinkat(,0)
so, I did not do this (even though I debated about it):
rmdirat() = unlinkat(,AT_REMOVEDIR)
Likewise, once we have support, we could do:
lutimensat() = utimensat(,AT_SYMLINK_NOFOLLOW)
but not utimensat. And I don't know of any good naming for the two
variants of linkat (there is no well-known short name that indicates
whether link will hard-link a symlink or follow it).
- --
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/
iEYEARECAAYFAkqk8AcACgkQ84KuGfSFAYBi/ACbB427B01pHBheU9GTiINZ4KlA
3KgAoJ6p1KEYdaj3n4uIeOUVKrczUNJu
=LDNW
-END PGP SIGNATURE-
From 370fec14f2e177cf533cb967e2dc04354e883c45 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Mon, 7 Sep 2009 05:28:13 -0600
Subject: [PATCH] openat: provide more convenience names
* modules/faccessat (configure.ac): Add C witness.
* lib/unistd.in.h (readlinkat): Fix typo.
* lib/openat.h (statat, lstatat, accessat, euidaccessat): New
convenience wrappers.
* top/maint.mk (sc_prohibit_openat_without_use): Allow these
wrappers in syntax checks.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog | 10 ++
lib/openat.h | 30 ++
lib/unistd.in.h |2 +-
modules/faccessat |1 +
top/maint.mk |2 +-
5 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 04e5cc1..f7ac99c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2009-09-07 Eric Blake e...@byu.net
+
+ openat: provide more convenience names
+ * modules/faccessat (configure.ac): Add C witness.
+ * lib/unistd.in.h (readlinkat): Fix typo.
+ * lib/openat.h (statat, lstatat, accessat, euidaccessat): New
+ convenience wrappers.
+ * top/maint.mk (sc_prohibit_openat_without_use): Allow these
+ wrappers in syntax checks.
+
2009-09-06 Eric Blake e...@byu.net
doc: fix comments in recent patches
diff --git a/lib/openat.h b/lib/openat.h
index 1d8596f..433b998 100644
--- a/lib/openat.h
+++ b/lib/openat.h
@@ -81,4 +81,34 @@ lchmodat (int fd, char const *file, mode_t mode)
return fchmodat (fd, file, mode, AT_SYMLINK_NOFOLLOW);
}
+static inline int
+statat (int fd, char const *name, struct stat *st)
+{
+ return fstatat (fd, name, st, 0);
+}
+
+static inline int
+lstatat (int fd, char const *name, struct stat *st)
+{
+ return fstatat (fd, name, st, AT_SYMLINK_NOFOLLOW);
+}
+
+#if GNULIB_FACCESSAT
+/* For now, there are no wrappers named laccessat or leuidaccessat,
+ since gnulib doesn't support faccessat(,AT_SYMLINK_NOFOLLOW) and
+ since access rights on symlinks are of limited utility. */
+
+static inline int
+accessat (int fd, char const *file, int mode)
+{
+ return faccessat (fd, file, mode, 0);
+}
+
+static inline int
+euidaccessat (int fd, char const *file, int mode)
+{
+ return faccessat (fd, file, mode, AT_EACCESS);
+}
+#endif
+
#endif /* _GL_HEADER_OPENAT */
diff --git a/lib/unistd.in.h b/lib/unistd.in.h
index 6503529..902b025 100644
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -208,7 +208,7 @@ ssize_t readlinkat (int fd, char const *file, char *buf,
size_t len);
#elif defined GNULIB_POSIXCHECK
# undef readlinkat
# define readlinkat(d,n,b,l)\
-(GL_LINK_WARNING (faccessat is not portable - \
+(GL_LINK_WARNING (readlinkat is not portable - \
use gnulib module symlinkat for portability), \
readlinkat (d, n, b, l))
#endif
diff --git a/modules/faccessat b/modules/faccessat
index d7737ba..92d8185 100644
--- a/modules/faccessat
+++ b/modules/faccessat
@@ -14,6 +14,7 @@ unistd
configure.ac:
gl_FUNC_FACCESSAT
+gl_MODULE_INDICATOR([faccessat])
gl_UNISTD_MODULE_INDICATOR([faccessat])
Makefile.am:
diff --git a/top/maint.mk b/top/maint.mk
index 45bc0c9..c0c3f27 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -295,7 +295,7 @@ sc_prohibit_root_dev_ino_without_use:
sc_prohibit_openat_without_use:
@h='openat.h' \
-
re='\(openat_(permissive|needs_fchdir|(save|restore)_fail)|l?ch(own|mod)at)\'
\
+
re='\(openat_(permissive|needs_fchdir|(save|restore)_fail)|l?(stat|ch(own|mod))at|(euid)?accessat)\'
\
$(_header_without_use
gnulib-tool -S --test
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 It would be nice if './gnulib-tool -S --with-tests --test xxx' created the testdir with symlinks rather than hard links, so that if a test fails, recompilation of the testdir will work even if the editor used to fix the source file breaks hard links. - -- 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/ iEYEARECAAYFAkqlA1wACgkQ84KuGfSFAYAxBwCgj+YmCEiR8QEXdCZNvmNJez6+ PHsAnAz9EI0BHFwW/Y+UkFsMrdpBB1kE =FAq+ -END PGP SIGNATURE-
merge rename and rename-dest-slash
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Any objections to deleting the rename-dest-slash module? It performs a subset of the rename module, and I'm about to: a) improve the rename module to work around a cygwin 1.5 bug (rename(a/.,b) succeeds when a is a directory, instead of failing with EINVAL), b) modernize the rename module to use stdio.in.h instead of #define rpl_rename directly in config.h c) implement the renameat module on top of rename - -- 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/ iEYEARECAAYFAkqlNTAACgkQ84KuGfSFAYC8YQCfZ0pmfwV+kF6IXBSZsdBn1kLm EqQAoLrNR3ebQ5NPM4Lp/HAqR7AAjKYo =CBmQ -END PGP SIGNATURE-
Re: merge rename and rename-dest-slash
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 9/7/2009 10:30 AM: Any objections to deleting the rename-dest-slash module? It performs a subset of the rename module, Correction. As currently written, rename.m4 checks whether: rm -rf d1 d2 mkdir d1 rename(d1/,d2) fails, but POSIX 2008 permits this case, so the test is wrong. But POSIX _does_ require rm -rf d1 d2 touch d1 rename(d1/,d2) to fail (in other words, the existing code needs to account for whether a directory is being renamed, rather than blindly rejecting trailing slash). Meanwhile, rename-dest-slash.m4 checks whether: rm -rf d1 d2 mkdir d1 rename(d1,d2/) fails, which POSIX 2008 does indeed require to fail. But note: rm -rf d1 d2 mkdir d1 d2 rename(d1,d2/) is required to pass. All of this is independent of the cygwin 1.5 bug, where POSIX requires any rename attempt targeting an explicit . or .. to fail with EINVAL. I guess it's time for me to add a unit test. - -- 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/ iEYEARECAAYFAkqlPvIACgkQ84KuGfSFAYCxZACgsXNKwV8AjPrPTZPN0dfHEeIX gfMAnAx25gaUe73an4YHzI+bi5vaOcVq =BknH -END PGP SIGNATURE-
Re: windows.h: No such file or directory
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Jim Meyering on 8/22/2009 1:58 AM:
John Taylor wrote:
I'm trying to build coreutils-7.4 with a cross-compilator (gcc-4.4.1)
that runs on i686-unknown-linux-gnu and produces code for
x86_64-unknown-linux-gnu. When building coreutils, I get the following
error message:
../../coreutils-7.4/lib/rename.c:33:21: error: windows.h: No such file
or directory
...
That is an unfortunate consequence of the fact that the rename
module must perform a so-called run test to determine whether
it must work around certain rename bugs.
When cross-compiling, it assumes the worst.
However, in your case, the m4 macro could obviously do better,
knowing the target is linux/gnu-based.
A patch would be welcome.
I'm pushing this, which improves the rename module (after first making the
stdio module easier to modify) to cross-compile to non-mingw scenarios.
If we then follow through with my proposal to remove the rename-dest-slash
module in favor of rename, then the compilation error mentioned above
should also be solved. Then there's still the matter of writing a unit
test and improving behavior for other known or discovered bugs.
- --
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/
iEYEARECAAYFAkqlQ2UACgkQ84KuGfSFAYDNrQCghIVews7NvrqUxPGppwuGmmLJ
ZyEAniFCZalzsO+dGsbx135iqlkJuOdL
=ZDOK
-END PGP SIGNATURE-
From bd24844d02c2add6a689c2963d35b9f9bb216147 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Mon, 7 Sep 2009 05:51:20 -0600
Subject: [PATCH 1/3] getcwd: minor cleanups
* lib/getcwd.c (AT_FDCWD): Delete; rely on fcntl.h instead.
(is_ENAMETOOLONG): Delete; ENAMETOOLONG is portable.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog|4
lib/getcwd.c | 16 +---
2 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index f7ac99c..771a9a9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2009-09-07 Eric Blake e...@byu.net
+ getcwd: minor cleanups
+ * lib/getcwd.c (AT_FDCWD): Delete; rely on fcntl.h instead.
+ (is_ENAMETOOLONG): Delete; ENAMETOOLONG is portable.
+
openat: provide more convenience names
* modules/faccessat (configure.ac): Add C witness.
* lib/unistd.in.h (readlinkat): Fix typo.
diff --git a/lib/getcwd.c b/lib/getcwd.c
index 2da1aee..6658ed5 100644
--- a/lib/getcwd.c
+++ b/lib/getcwd.c
@@ -59,20 +59,6 @@
#include limits.h
-/* Work around a bug in Solaris 9 and 10: AT_FDCWD is positive. Its
- value exceeds INT_MAX, so its use as an int doesn't conform to the
- C standard, and GCC and Sun C complain in some cases. */
-#if 0 AT_FDCWD AT_FDCWD == 0xffd19553
-# undef AT_FDCWD
-# define AT_FDCWD (-3041965)
-#endif
-
-#ifdef ENAMETOOLONG
-# define is_ENAMETOOLONG(x) ((x) == ENAMETOOLONG)
-#else
-# define is_ENAMETOOLONG(x) 0
-#endif
-
#ifndef MAX
# define MAX(a, b) ((a) (b) ? (b) : (a))
#endif
@@ -164,7 +150,7 @@ __getcwd (char *buf, size_t size)
# undef getcwd
dir = getcwd (buf, size);
- if (dir || (errno != ERANGE !is_ENAMETOOLONG (errno) errno != ENOENT))
+ if (dir || (errno != ERANGE errno != ENAMETOOLONG errno != ENOENT))
return dir;
#endif
--
1.6.3.3.334.g916e1
From 1132a93bd6177115d1047846c62dc96fb7facb56 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Mon, 7 Sep 2009 10:16:42 -0600
Subject: [PATCH 2/3] stdio: sort witness names
* modules/stdio (Makefile.am): Sort replacements.
* m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Likewise.
* lib/stdio.in.h: Likewise.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |5 +
lib/stdio.in.h | 631
m4/stdio_h.m4 | 104 +-
modules/stdio | 98 +-
4 files changed, 421 insertions(+), 417 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 771a9a9..e3af685 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2009-09-07 Eric Blake e...@byu.net
+ stdio: sort witness names
+ * modules/stdio (Makefile.am): Sort replacements.
+ * m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Likewise.
+ * lib/stdio.in.h: Likewise.
+
getcwd: minor cleanups
* lib/getcwd.c (AT_FDCWD): Delete; rely on fcntl.h instead.
(is_ENAMETOOLONG): Delete; ENAMETOOLONG is portable.
diff --git a/lib/stdio.in.h b/lib/stdio.in.h
index 9dfb679..495baca 100644
--- a/lib/stdio.in.h
+++ b/lib/stdio.in.h
@@ -68,154 +68,6 @@
extern C {
#endif
-
-#if @GNULIB_FPRINTF_POSIX@
-# if @REPLACE_FPRINTF@
-# define fprintf rpl_fprintf
-extern int fprintf (FILE *fp, const char *format, ...)
- __attribute__ ((__format__ (__printf__, 2, 3)));
-# endif
-#elif @GNULIB_FPRINTF@ @REPLACE_STDIO_WRITE_FUNCS
Re: merge rename and rename-dest-slash
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 9/7/2009 11:12 AM: According to Eric Blake on 9/7/2009 10:30 AM: Any objections to deleting the rename-dest-slash module? It performs a subset of the rename module, Correction. As currently written, rename.m4 checks whether: rm -rf d1 d2 mkdir d1 rename(d1/,d2) Meanwhile, rename-dest-slash.m4 checks whether: rm -rf d1 d2 mkdir d1 rename(d1,d2/) fails And POSIX 2008 is still awkward on rules for trailing slashes. I think the intent is that a directory may be specified with a trailing slash, but that a regular file must not (so, for example, the fact that Solaris 10 allows: touch a rename(a,b/) to succeed if b exists as a regular file, but fails with ENOTDIR if b does not exist, is a bug in Solaris). But I'd rather wait for the Austin group ruling on my bug report: https://www.opengroup.org/sophocles/show_mail.tpl?CALLER=index.tplsource=Llistname=austin-group-lid=12739 as well as delay the merging of these two modules until after coreutils 7.6 is released. I guess it's time for me to add a unit test. Still true, although it's hard to know what to test until we are certain what POSIX requires. - -- 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/ iEYEARECAAYFAkqle4sACgkQ84KuGfSFAYDimACffcikrffznSm+jbGVyNj3/aGA NAsAoKPuryJSyFQcmkXUqRktC8HEkFGt =Zg6u -END PGP SIGNATURE-
Re: new snapshot available: coreutils-7.5.65-61cc6
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Pádraig Brady on 9/7/2009 6:32 PM: fstatat.c, line 39: undefined symbol: AT_SYMLINK_NOFOLLOW Hmm. That should be taken care of by the gnulib fcntl.h replacement. Are we missing a #include? Can you post the full failure? gnulib 52c658e seems to have removed #include openat.h from fstatat.c but not replaced it with #include fcntl.h Yep; and only Solaris had the problem, because that is the only platform with broken fstatat (the other platforms either lack it or it works entirely). Fixed as follows; coreutils needs to update the gnulib submodule. - -- 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/ iEYEARECAAYFAkqlwIgACgkQ84KuGfSFAYCYWQCbBC7k9nNOQ7kWrZb97lGz/Tpw ADkAniQxA4kKKzEv7vognH3MDcKFRpGV =nQmf -END PGP SIGNATURE- From 2c90f1af0fd6fcf444f5c16de9ff465651a854fc Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Mon, 7 Sep 2009 20:16:00 -0600 Subject: [PATCH] fstatat: fix compilation on Solaris MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * lib/fstatat.c (includes): Add fcntl.h. Reported by Pádraig Brady. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog |6 ++ lib/fstatat.c |1 + 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0b9e3a0..7df9226 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2009-09-08 Eric Blake e...@byu.net + + fstatat: fix compilation on Solaris + * lib/fstatat.c (includes): Add fcntl.h. + Reported by Pádraig Brady. + 2009-09-07 Eric Blake e...@byu.net rename: modernize replacement diff --git a/lib/fstatat.c b/lib/fstatat.c index 2bf547e..9b0c1af 100644 --- a/lib/fstatat.c +++ b/lib/fstatat.c @@ -22,6 +22,7 @@ #include sys/stat.h #include errno.h +#include fcntl.h #include string.h #undef fstatat -- 1.6.3.3.334.g916e1
mkfifoat, renameat
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Two more *at functions. mkfifoat is rather simple; the hardest part was ensuring the test still compiles on mingw. renameat is more complex, although I intentionally made it call into a helper function, at_func2, so that I can reuse that function when I do linkat. The renameat testsuite intentionally exercises as many successful control paths of the helper function as I could manage, prior to doing semantic checks of various error conditions. There are still some pending questions to the Austin Group on what POSIX says rename _should_ do, so for now, I'm okay with commented out portions in the test suite. I verified that the unit test passed on Linux, Solaris 10, cygwin 1.5, and cygwin 1.7, although it failed miserably on mingw. Part of the problem on mingw is that the fchdir module relies on canonicalize doing the right thing, but mingw's paths (with drive letters and \, rather than leading / for absolute) throws canonicalize for a loop; and cross-compilation tries to replace getcwd, even though mingw's getcwd always works. At that point, the populated dirs[fd].name mapping have invalid entries, which hurt attempts to do fstat(dfd). It doesn't help that mingw stat() reports st_ino == 0 for every directory, rendering SAME_INODE useless. Oh well, I'll have to spend more time porting those pieces to mingw. But for now, I think renameat is far enough along to at least post for review. I suppose there might still be some pathological cases where renameat currently fails but could be made to succeed. For example, renameat(super_deep,a,root,b) will probably fail with ENAMETOOLONG when changing to super_deep and converting a to an absolute name, while changing to root and converting b to absolute would avoid that issue. But I'm not too worried about those unless someone actually reports that they encountered it in practice. - -- 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/ iEYEARECAAYFAkql0ucACgkQ84KuGfSFAYDRWwCZAfNS9a/nHigBF5Dh4FU1u+Qi ZYYAn1reDC2j3lUocZCmEI+cXmtz/wV8 =og/P -END PGP SIGNATURE- From 625504c335cdbded06262bf5607d07c1fb7675d0 Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Mon, 7 Sep 2009 05:38:18 -0600 Subject: [PATCH 1/2] mkfifoat: new module * modules/mkfifoat: New file. * lib/mkfifoat.c: Likewise. * m4/mkfifoat.m4 (gl_FUNC_MKFIFOAT): Likewise. * m4/sys_stat_h.m4 (gl_SYS_STAT_H_DEFAULTS): Add witnesses. * modules/sys_stat (Makefile.am): Use them. * lib/sys_stat.in.h (mkfifoat, mknodat): Declare them. * MODULES.html.sh (File system functions): Mention module. * doc/posix-functions/mkfifoat.texi (mkfifoat): Likewise. * doc/posix-functions/mknodat.texi (mknodat): Likewise. * modules/mkfifoat-tests: New test. * tests/test-mkfifoat.c: Likewise. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog | 13 MODULES.html.sh |1 + doc/posix-functions/mkfifoat.texi | 13 +++- doc/posix-functions/mknodat.texi | 13 +++- lib/mkfifoat.c| 101 ++ lib/sys_stat.in.h | 23 +++ m4/mkfifoat.m4| 23 +++ m4/sys_stat_h.m4 | 32 + modules/mkfifoat | 29 + modules/mkfifoat-tests| 11 +++ modules/sys_stat |9 ++- tests/test-mkfifoat.c | 124 + 12 files changed, 368 insertions(+), 24 deletions(-) create mode 100644 lib/mkfifoat.c create mode 100644 m4/mkfifoat.m4 create mode 100644 modules/mkfifoat create mode 100644 modules/mkfifoat-tests create mode 100644 tests/test-mkfifoat.c diff --git a/ChangeLog b/ChangeLog index 7df9226..c409f2f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,18 @@ 2009-09-08 Eric Blake e...@byu.net + mkfifoat: new module + * modules/mkfifoat: New file. + * lib/mkfifoat.c: Likewise. + * m4/mkfifoat.m4 (gl_FUNC_MKFIFOAT): Likewise. + * m4/sys_stat_h.m4 (gl_SYS_STAT_H_DEFAULTS): Add witnesses. + * modules/sys_stat (Makefile.am): Use them. + * lib/sys_stat.in.h (mkfifoat, mknodat): Declare them. + * MODULES.html.sh (File system functions): Mention module. + * doc/posix-functions/mkfifoat.texi (mkfifoat): Likewise. + * doc/posix-functions/mknodat.texi (mknodat): Likewise. + * modules/mkfifoat-tests: New test. + * tests/test-mkfifoat.c: Likewise. + fstatat: fix compilation on Solaris * lib/fstatat.c (includes): Add fcntl.h. Reported by Pádraig Brady. diff --git a/MODULES.html.sh b/MODULES.html.sh index abfd93b..76741b3 100755 --- a/MODULES.html.sh +++ b/MODULES.html.sh @@ -2465,6 +2465,7
Re: mkfifoat, renameat
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 9/8/2009 1:50 AM: Eric Blake wrote: --- a/modules/sys_stat +++ b/modules/sys_stat ... + -e 's|@''GNULIB_OPENAT''@|$(GNULIB_OPENAT)|g' \ Why this? The @GNULIB_OPENAT@ token is used only in fcntl.in.h, not in sys_stat.in.h. Leftovers from my tree prior to merging in your cleanup patch - I originally used GNULIB_OPENAT, you removed it, and my merge reintroduced it. Now cleaned up. - -- 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/ iEYEARECAAYFAkqmTJQACgkQ84KuGfSFAYBXGwCcCSvVz9EJ+AKEImy/vN76rJzd aX8AnA9P+Ry7YenRcy3dku/IOXOjLr4t =SoIR -END PGP SIGNATURE-
Re: relax strchrnul to LGPLv2+?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/8/2009 3:17 AM: Hi Eric, I would like to use strchrnul in libvirt, but its license is listed at the default LGPL (as glibc-derived, I guess?) and that conflicts with libvirt's requirement for LGPLv2+. What do you think about relaxing it? Done. - -- 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/ iEYEARECAAYFAkqmTLkACgkQ84KuGfSFAYBWhgCfXO2Aj11dccPfgYb5Vonv9iiZ o9oAnj4TEATqdiCKN7Ox6oQJNsEXAvuK =h/km -END PGP SIGNATURE-
Re: mkfifoat, renameat
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/8/2009 2:43 AM: I suppose there might still be some pathological cases where renameat currently fails but could be made to succeed. For example, renameat(super_deep,a,root,b) will probably fail with ENAMETOOLONG when Speaking of pathological, ;-) For some cases, in which a renamat emulator would fail to form *any* usable absolute name, it may succeed with relative names, e.g., renameat(deep1,a,deep2,b) when the deep1-relative name for deep2 or the deep2-relative name for deep1 is short enough. Of course, calculating those relative names may be rather expensive. Indeed, that's another one. We have chdir-long which deals with some pathological cases, but that only deals with one directory. I'm not sure whether it is even worth the effort to try and write a rename-long to deal with double the directories, although enough bug reports may prod us to try and do that. It seems like getting two canonical directory names, then finding the common prefix, would at least give us a start in finding a single relative location that might work. At any rate, I don't want to hold up this implementation just for the pathological cases. Very nice. Thanks especially for the thorough unit tests. Have you looked at coverage data? It'd be nice to to ensure that each of those many cases is exercised on some platform. I haven't run any profiler tool, but did step through the code by hand on older Linux (/proc/self/fd) and Cygwin 1.5 (save_cwd/fchdir), and verified that I am indeed hitting all 16 combinations of absolute vs. relative and AT_FDCWD vs. fd. The test suite was the driver in implementing this, and caught several early thinkos (which is the way it should be ;) I've pushed mkfifoat; I'm waiting to push renameat until I can finish improving the rename module to work around cygwin 1.5 and Solaris 10 deficiencies. Cygwin allowed rename(dir,file), which is an absolute no-no. And the fact that Solaris lets rename(file1,file2/) succeed if file2 is missing, but not if it is present, is just inconsistent, regardless of how the Austin group rules (but preliminary feedback on my POSIX bug report looks like others agree with me; the rename spec will probably be tweaked so that rename(dir,name/) should be okay unless name is an existing non-directory or an existing non-empty directory, and rename(file,name/) should always fail). With the full suite of portable *at functions, it should now be feasible to rewrite coreutils' copy.c to do its job via fts, and directory file descriptors, rather than via full relative names. Well, utimensat and linkat are still on my TODO list. With renameat in place, linkat should be much simpler. Should I make that testsuite also exercise all the same paths as renameat? Implementing utimensat will need some thought for how best to interact with the existing utimens module and the fact that we need rpl_utimensat on Linux to work around kernel bugs. But I will probably complete the suite this week. - -- 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/ iEYEARECAAYFAkqmTZwACgkQ84KuGfSFAYBDfwCfXVrRKx5IwfNsUSTIsHSSzazm j8AAnj2Woj9oORXB+CznMNqNk2IgiM1I =Wouf -END PGP SIGNATURE-
Re: relax strchrnul to LGPLv2+?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/8/2009 6:32 AM: What do you think about relaxing it? Done. Thanks! As I tried to use that module in libvirt, I now see that its dependent module, rawmemchr (also LGPL) needs the same treatment. So, if you would, ... Done. And this time, tested with gnulib-tool --lgpl. - -- 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/ iEYEARECAAYFAkqmUFYACgkQ84KuGfSFAYCoOACfW3Wl6gm5PFTCH/e+ccXt6l+r GLYAoMzv6PyniP+/hkIRr7ftspNiG1vQ =r4Iv -END PGP SIGNATURE-
Re: new snapshot available: coreutils-7.5.65-61cc6
Jim Meyering jim at meyering.net writes:
euidaccess.c is also broken; it refers to AT_EACCESS without using
fcntl.h.
+ euidaccess: fix compilation error
+ * lib/euidaccess.c (includes): Add fcntl.h, for AT_EACCESS.
For what system?
Cygwin 1.7 has faccessat but (currently) lacks euidaccess. And since the
former is POSIX while the latter is a GNU extension, it is conceivable that
other systems might implement *at support without extensions.
Regardless, this looks like a fine change.
The alternative would be to remove that #if HAVE_FACCESSAT block:
int
euidaccess (const char *file, int mode)
{
#if HAVE_FACCESSAT
return faccessat (AT_FDCWD, file, mode, AT_EACCESS);
#elif defined EFF_ONLY_OK
Nope; that would be reverting the intent of commit bc366ae.
--
Eric Blake
Re: lib/exclude.c calls towlower() without checking for wideline support
Bruno Haible bruno at clisp.org writes: + /* Check that the tow* functions map WEOF to WEOF. */ + ASSERT (towlower (e) == e); + ASSERT (towupper (e) == e); This trips a bug in mingw. There, WEOF is defined as 0x, but towlower (0x) returns 0x77c5. Oddly enough, towupper(0x) works as expected. -- Eric Blake
Re: mkfifoat, renameat
Eric Blake ebb9 at byu.net writes:
Part of the problem on mingw is that the fchdir module relies on
canonicalize doing the right thing, but mingw's paths (with drive letters
and \, rather than leading / for absolute) throws canonicalize for a loop;
and cross-compilation tries to replace getcwd, even though mingw's getcwd
always works. At that point, the populated dirs[fd].name mapping have
invalid entries, which hurt attempts to do fstat(dfd). It doesn't help
that mingw stat() reports st_ino == 0 for every directory, rendering
SAME_INODE useless. Oh well, I'll have to spend more time porting those
pieces to mingw.
Among other things, testing shows that mingw supports getcwd(NULL,0), so the
cross-compilation guess needs to be improved there. stat() has the annoying
property that a trailing slash changes semantics (but / and \\ are
interchangeable), and in an inconsistent manner (you can't always stat the
result of getcwd!):
getcwd reports: c:\
stat(c:,st) = fails
stat(c:/,st) = passes
getcwd reports: c:\dir
stat(c:/dir,st) = passes
stat(c:/dir/,st) = fails
getcwd reports: \\machine\share
stat(machine\\share,st) = fails
stat(machine\\share\\,st) = passes
getcwd reports: \\machine\share\dir
stat(machine\\share\\dir,st) = passes
stat(machine\\share\\dir\\,st) = fails
And it doesn't help that canonicalize is not consistently using ISSLASH macros
to munge '/' and '\\' into a canonical form. Yep, definitely some groundwork
to lay here before linkat and renameat will work on mingw.
Meanwhile, I noticed that the link module has a bug on mingw, where link
(a,b/.) created the regular file b rather than failing with ENOENT (I
noticed it because cygwin 1.5 had the same bug, and I noticed that while
enhancing the link module to work around Solaris 8 handling of trailing / in
link). I'll probably be applying this soon...
From: Eric Blake e...@byu.net
Date: Wed, 9 Sep 2009 11:06:44 -0600
Subject: [PATCH 1/2] test-link: consolidate into single C program, test more
cases
* tests/test-link.sh: Delete.
* tests/test-link.c: Test more error conditions. Exposes bugs on
at least Cygwin and Solaris.
* modules/link-tests (Files): Remove unused file.
(Depends-on): Add errno, sys_stat.
(Makefile.am): Simplify.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |8
modules/link-tests |6 ++--
tests/test-link.c | 103 +---
tests/test-link.sh | 37 ---
4 files changed, 109 insertions(+), 45 deletions(-)
delete mode 100755 tests/test-link.sh
diff --git a/ChangeLog b/ChangeLog
index 08fe19a..8372ebe 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
2009-09-09 Eric Blake e...@byu.net
+ test-link: consolidate into single C program, test more cases
+ * tests/test-link.sh: Delete.
+ * tests/test-link.c: Test more error conditions. Exposes bugs on
+ at least Cygwin and Solaris.
+ * modules/link-tests (Files): Remove unused file.
+ (Depends-on): Add errno, sys_stat.
+ (Makefile.am): Simplify.
+
dirname: add library-safe mdir_name
* lib/dirname.h (mdir_name): New prototype.
* lib/dirname.c (dir_name): Move guts...
diff --git a/modules/link-tests b/modules/link-tests
index c1ec52c..ca61deb 100644
--- a/modules/link-tests
+++ b/modules/link-tests
@@ -1,12 +1,12 @@
Files:
-tests/test-link.sh
tests/test-link.c
Depends-on:
+errno
+sys_stat
configure.ac:
Makefile.am:
-TESTS += test-link.sh
-TESTS_ENVIRONMENT += EXEEXT='@EXEEXT@'
+TESTS += test-link
check_PROGRAMS += test-link
diff --git a/tests/test-link.c b/tests/test-link.c
index 250a821..fbc794a 100644
--- a/tests/test-link.c
+++ b/tests/test-link.c
@@ -19,8 +19,12 @@
#include unistd.h
#include errno.h
+#include fcntl.h
#include stdio.h
#include stdlib.h
+#include string.h
+#include sys/stat.h
+#include unistd.h
#define ASSERT(expr) \
do \
@@ -34,30 +38,119 @@
}\
while (0)
+#define BASE test-link.t
+
int
main (int argc, char **argv)
{
+ int fd;
int ret;
- ASSERT (argc == 3);
+ /* Remove any garbage left from previous partial runs. */
+ ASSERT (system (rm -rf BASE *) == 0);
+
+ /* Create first file. */
+ fd = open (BASE a, O_CREAT | O_EXCL | O_WRONLY, 0600);
+ ASSERT (0 = fd);
+ ASSERT (write (fd, hello, 5) == 5);
+ ASSERT (close (fd) == 0);
- ret = link (argv[1], argv[2]);
- if (ret 0)
+ /* Not all file systems support link. Mingw doesn't have reliable
+ st_nlink on hard links, but our implementation does fail with
+ EPERM on poor file systems, and we can detect the inferior stat()
+ via st_ino. Cygwin 1.5.x copies rather than links files on those
+ file systems, but there, st_nlink and st_ino are reliable. */
+ ret = link (BASE a, BASE b);
+ if (!ret
getcwd on mingw
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
getcwd does not need replacement on mingw, even though it fails runtime
tests in getcwd.m4, and even though cross-compilation was pessimistic.
This fixes things, and adds a simple unit test to prove that getcwd is
behaving reasonably.
- --
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/
iEYEARECAAYFAkqobcsACgkQ84KuGfSFAYAv2QCgzG824yQId4/U2Py/Tf1H4aj9
okwAoLI0e3dKh1cFQ50FM+rH52pSCC8g
=wymK
-END PGP SIGNATURE-
From e2b8816f0465fa211eb5b49ecbeb78e9c3cc46b8 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Wed, 9 Sep 2009 20:52:26 -0600
Subject: [PATCH] getcwd: port to mingw
* m4/getcwd.m4 (gl_FUNC_GETCWD): Mingw directories are very
different from the POSIX assumptions made throughout the getcwd
module; fortunately, the mingw getcwd does not need replacement.
(gl_FUNC_GETCWD_NULL): Skip test on mingw.
* modules/getcwd-tests: New test.
* tests/test-getcwd.c: Likewise.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog|8 +
m4/getcwd.m4 | 44 ++
modules/getcwd-tests | 10 ++
tests/test-getcwd.c | 83 ++
4 files changed, 131 insertions(+), 14 deletions(-)
create mode 100644 modules/getcwd-tests
create mode 100644 tests/test-getcwd.c
diff --git a/ChangeLog b/ChangeLog
index 957a1f6..b5b4494 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
2009-09-09 Eric Blake e...@byu.net
+ getcwd: port to mingw
+ * m4/getcwd.m4 (gl_FUNC_GETCWD): Mingw directories are very
+ different from the POSIX assumptions made throughout the getcwd
+ module; fortunately, the mingw getcwd does not need replacement.
+ (gl_FUNC_GETCWD_NULL): Skip test on mingw.
+ * modules/getcwd-tests: New test.
+ * tests/test-getcwd.c: Likewise.
+
link: fix platform bugs
* m4/link.m4 (gl_FUNC_LINK): Detect Solaris and Cygwin bugs.
* lib/link.c (link): Work around them. Fix related mingw bug.
diff --git a/m4/getcwd.m4 b/m4/getcwd.m4
index 6ebe2fc..4b8c4c6 100644
--- a/m4/getcwd.m4
+++ b/m4/getcwd.m4
@@ -1,53 +1,69 @@
# getcwd.m4 - check for working getcwd that is compatible with glibc
-# Copyright (C) 2001, 2003, 2004, 2005, 2006, 2007 Free Software Foundation,
Inc.
+# Copyright (C) 2001, 2003, 2004, 2005, 2006, 2007, 2009 Free Software
+# Foundation, Inc.
# This file is free software; the Free Software Foundation
# gives unlimited permission to copy and/or distribute it,
# with or without modifications, as long as this notice is preserved.
# Written by Paul Eggert.
+# serial 2
AC_DEFUN([gl_FUNC_GETCWD_NULL],
[
AC_CACHE_CHECK([whether getcwd (NULL, 0) allocates memory for result],
[gl_cv_func_getcwd_null],
- [AC_TRY_RUN(
-[
-# include stdlib.h
+ [AC_RUN_IFELSE([AC_LANG_PROGRAM([[
# include unistd.h
# ifndef getcwd
char *getcwd ();
# endif
-int
-main ()
-{
+]], [[
+#if (defined _WIN32 || defined __WIN32__) ! defined __CYGWIN__
+/* mingw cwd does not start with '/', but getcwd does allocate. */
+#else
if (chdir (/) != 0)
-exit (1);
+return 1;
else
{
char *f = getcwd (NULL, 0);
- exit (! (f f[0] == '/' !f[1]));
+ return ! (f f[0] == '/' !f[1]);
}
-}],
+#endif
+]])],
[gl_cv_func_getcwd_null=yes],
[gl_cv_func_getcwd_null=no],
- [gl_cv_func_getcwd_null=no])])
+ [[
+ case $host_os in
+ # Guess yes on glibc systems.
+ *-gnu*) gl_cv_func_getcwd_null=guessing yes;;
+ # Guess yes on Cygwin.
+ cygwin*) gl_cv_func_getcwd_null=guessing yes;;
+ # Guess yes on mingw.
+ mingw*) gl_cv_func_getcwd_null=guessing yes;;
+ # If we don't know, assume the worst.
+ *)gl_cv_func_getcwd_null=guessing no;;
+ esac
+ ]])])
])
AC_DEFUN([gl_FUNC_GETCWD],
[
AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
AC_REQUIRE([gl_FUNC_GETCWD_NULL])
+ AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
gl_abort_bug=no
- case $gl_cv_func_getcwd_null in
- yes)
+ case $gl_cv_func_getcwd_null,$host_os in
+ *,mingw*)
+gl_cv_func_getcwd_path_max=yes;;
+ yes,*)
gl_FUNC_GETCWD_PATH_MAX
gl_FUNC_GETCWD_ABORT_BUG([gl_abort_bug=yes]);;
esac
case $gl_cv_func_getcwd_null,$gl_cv_func_getcwd_path_max,$gl_abort_bug in
- yes,yes,no) ;;
+ *yes,yes,no) ;;
*)
REPLACE_GETCWD=1
AC_LIBOBJ([getcwd])
diff --git
strndup.m4 cleanup
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I noticed this when cross-compiling to mingw: | checking for working strndup... checking for strndup... no | no The m4 macro did a no-no - it was calling AC_CHECK_FUNC([strndup]), which has side effects, in the body of an AC_CACHE_CHECK. Meanwhile, the semantics of HAVE_STRNDUP were confusing - AIX 5 has it, but it needs replacing, so the variable is better named REPLACE_STRNDUP. Cleaned up as follows: - -- 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/ iEYEARECAAYFAkqodE0ACgkQ84KuGfSFAYCZ5ACgsoQOVuMjUztgc8ezEUpiEkDg sDoAn3TTbkUmhLFoosc/vdhI7n3xZfff =4v7I -END PGP SIGNATURE- From 587f0edadfcadc708f6144de37a0653e81518484 Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Wed, 9 Sep 2009 21:34:32 -0600 Subject: [PATCH] strndup: fix improper m4 caching * m4/strndup.m4 (gl_FUNC_STRNDUP): Rework to avoid side effects inside AC_CACHE_CHECK. Use REPLACE_STRNDUP, not HAVE_STRNDUP. (gl_PREREQ_STRNDUP): Delete. * m4/string_h.m4 (gl_HEADER_STRING_H_DEFAULTS): Update default. * modules/string (Makefile.am): Substitute it. * lib/string.in.h (strndup): Modernize prototype. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog |8 lib/string.in.h |4 ++-- m4/string_h.m4 |4 ++-- m4/strndup.m4 | 46 +- modules/string |2 +- 5 files changed, 34 insertions(+), 30 deletions(-) diff --git a/ChangeLog b/ChangeLog index b5b4494..a9664bc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ 2009-09-09 Eric Blake e...@byu.net + strndup: fix improper m4 caching + * m4/strndup.m4 (gl_FUNC_STRNDUP): Rework to avoid side effects + inside AC_CACHE_CHECK. Use REPLACE_STRNDUP, not HAVE_STRNDUP. + (gl_PREREQ_STRNDUP): Delete. + * m4/string_h.m4 (gl_HEADER_STRING_H_DEFAULTS): Update default. + * modules/string (Makefile.am): Substitute it. + * lib/string.in.h (strndup): Modernize prototype. + getcwd: port to mingw * m4/getcwd.m4 (gl_FUNC_GETCWD): Mingw directories are very different from the POSIX assumptions made throughout the getcwd diff --git a/lib/string.in.h b/lib/string.in.h index 89c23c7..04deb42 100644 --- a/lib/string.in.h +++ b/lib/string.in.h @@ -203,11 +203,11 @@ extern char *strdup (char const *__s); /* Return a newly allocated copy of at most N bytes of STRING. */ #if @GNULIB_STRNDUP@ -# if ! @HAVE_STRNDUP@ +# if @REPLACE_STRNDUP@ # undef strndup # define strndup rpl_strndup # endif -# if ! @HAVE_STRNDUP@ || ! @HAVE_DECL_STRNDUP@ +# if @REPLACE_STRNDUP@ || ! @HAVE_DECL_STRNDUP@ extern char *strndup (char const *__string, size_t __n); # endif #elif defined GNULIB_POSIXCHECK diff --git a/m4/string_h.m4 b/m4/string_h.m4 index 3dbbfe1..edc5c6e 100644 --- a/m4/string_h.m4 +++ b/m4/string_h.m4 @@ -5,7 +5,7 @@ # gives unlimited permission to copy and/or distribute it, # with or without modifications, as long as this notice is preserved. -# serial 8 +# serial 9 # Written by Paul Eggert. @@ -74,7 +74,6 @@ AC_DEFUN([gl_HEADER_STRING_H_DEFAULTS], HAVE_STPNCPY=1; AC_SUBST([HAVE_STPNCPY]) HAVE_STRCHRNUL=1;AC_SUBST([HAVE_STRCHRNUL]) HAVE_DECL_STRDUP=1; AC_SUBST([HAVE_DECL_STRDUP]) - HAVE_STRNDUP=1; AC_SUBST([HAVE_STRNDUP]) HAVE_DECL_STRNDUP=1; AC_SUBST([HAVE_DECL_STRNDUP]) HAVE_DECL_STRNLEN=1; AC_SUBST([HAVE_DECL_STRNLEN]) HAVE_STRPBRK=1; AC_SUBST([HAVE_STRPBRK]) @@ -90,6 +89,7 @@ AC_DEFUN([gl_HEADER_STRING_H_DEFAULTS], REPLACE_STRSTR=0;AC_SUBST([REPLACE_STRSTR]) REPLACE_STRCASESTR=0;AC_SUBST([REPLACE_STRCASESTR]) REPLACE_STRERROR=0; AC_SUBST([REPLACE_STRERROR]) + REPLACE_STRNDUP=0; AC_SUBST([REPLACE_STRNDUP]) REPLACE_STRSIGNAL=0; AC_SUBST([REPLACE_STRSIGNAL]) REPLACE_STRTOK_R=0; AC_SUBST([REPLACE_STRTOK_R]) UNDEFINE_STRTOK_R=0; AC_SUBST([UNDEFINE_STRTOK_R]) diff --git a/m4/strndup.m4 b/m4/strndup.m4 index 4fa7d5a..437e7f6 100644 --- a/m4/strndup.m4 +++ b/m4/strndup.m4 @@ -1,4 +1,4 @@ -# strndup.m4 serial 16 +# strndup.m4 serial 17 dnl Copyright (C) 2002-2003, 2005-2009 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -9,17 +9,20 @@ AC_DEFUN([gl_FUNC_STRNDUP], dnl Persuade glibc string.h to declare strndup(). AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS]) + AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles AC_REQUIRE([gl_HEADER_STRING_H_DEFAULTS]) AC_CHECK_DECLS_ONCE([strndup]) + AC_CHECK_FUNCS_ONCE([strndup]) if test $ac_cv_have_decl_strndup
canonicalize-lgpl bug
I noticed that glibc has applied some fixes to canonicalize.c that we were
missing; and sure enough, was able to enhance the testsuite to trip these
bugs. They also applied some assert's proving that their optimization to avoid
strcpy are safe; I left the asserts out, but copied their optimization. OK to
commit?
I plan on porting the testsuite changes to test-canonicalize.c, and fixing any
fallout there as a separate commit.
From: Eric Blake e...@byu.net
Date: Thu, 10 Sep 2009 12:12:16 -0600
Subject: [PATCH] canonicalize-lgpl: reject non-directory with trailing slash
* tests/test-canonicalize-lgpl.c (main): Enhance test.
* lib/canonicalize-lgpl.c (__realpath): Synchronize with glibc.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |6 ++
lib/canonicalize-lgpl.c| 11 +++
tests/test-canonicalize-lgpl.c | 16 +++-
3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index f9d9c1b..baeaf35 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2009-09-10 Eric Blake e...@byu.net
+ canonicalize-lgpl: reject non-directory with trailing slash
+ * tests/test-canonicalize-lgpl.c (main): Enhance test.
+ * lib/canonicalize-lgpl.c (__realpath): Synchronize with glibc.
+
+2009-09-10 Eric Blake e...@byu.net
+
dirname: add library-safe mdir_name
* lib/dirname.h (mdir_name): New prototype.
* lib/dirname.c (dir_name): Move guts...
diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c
index 6b5663a..c8f313e 100644
--- a/lib/canonicalize-lgpl.c
+++ b/lib/canonicalize-lgpl.c
@@ -298,6 +298,11 @@ __realpath (const char *name, char *resolved)
while ((--dest)[-1] != '/');
}
#endif
+ else if (!S_ISDIR (st.st_mode) *end != '\0')
+ {
+ __set_errno (ENOTDIR);
+ goto error;
+ }
}
}
if (dest rpath + 1 dest[-1] == '/')
@@ -307,16 +312,14 @@ __realpath (const char *name, char *resolved)
if (extra_buf)
freea (extra_buf);
- return resolved ? memcpy (resolved, rpath, dest - rpath + 1) : rpath;
+ return rpath;
error:
{
int saved_errno = errno;
if (extra_buf)
freea (extra_buf);
-if (resolved)
- strcpy (resolved, rpath);
-else
+if (resolved == NULL)
free (rpath);
errno = saved_errno;
}
diff --git a/tests/test-canonicalize-lgpl.c b/tests/test-canonicalize-lgpl.c
index 29b919d..87e5e46 100644
--- a/tests/test-canonicalize-lgpl.c
+++ b/tests/test-canonicalize-lgpl.c
@@ -1,5 +1,5 @@
/* Test of execution of program termination handlers.
- Copyright (C) 2007-2008 Free Software Foundation, Inc.
+ Copyright (C) 2007-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
@@ -79,6 +79,20 @@ main ()
ASSERT (result == NULL);
}
+ /* Check that a non-directory with trailing slash yields NULL. */
+ {
+char *result = canonicalize_file_name (t-can-lgpl.tmp/tra/);
+ASSERT (result == NULL);
+result = canonicalize_file_name (t-can-lgpl.tmp/huk/);
+ASSERT (result == NULL);
+ }
+
+ /* Check that a missing directory yields NULL. */
+ {
+char *result = canonicalize_file_name (t-can-lgpl.tmp/ouk/..);
+ASSERT (result == NULL);
+ }
+
/* Check that a loop of symbolic links is detected. */
{
char *result = canonicalize_file_name (ise);
--
1.6.3.2
Re: canonicalize-lgpl bug
Eric Blake ebb9 at byu.net writes: I noticed that glibc has applied some fixes to canonicalize.c that we were missing; and sure enough, was able to enhance the testsuite to trip these bugs. They also applied some assert's proving that their optimization to avoid strcpy are safe; I left the asserts out, but copied their optimization. OK to commit? Hmm, on thinking about this more, there are probably existing Linux machines where glibc canonicalize_file_name is broken (fails to detect ENOTDIR on file/), and therefore we need to replace it. Additionally, it would be nice to declare canonicalize_file_name in stdlib.h. Also, now that POSIX requires realpath(name,NULL) to malloc() its results as an absolute path, it would be nice to have a realpath module that replaces broken versions (Solaris sometimes gives relative paths, and not all existing realpath implementations malloc). -- Eric Blake
Re: canonicalize-lgpl bug
Jim Meyering jim at meyering.net writes:
I noticed that glibc has applied some fixes to canonicalize.c that we were
missing; and sure enough, was able to enhance the testsuite to trip these
bugs. They also applied some assert's proving that their optimization to
avoid
strcpy are safe; I left the asserts out, but copied their optimization. OK
to
commit?
Sure.
Thanks for doing that!
Here's the current state of the series. It still doesn't fix the bug that
canonicalize-lgpl goofs on leading // when providing realpath. Nor does it fix
execution on mingw (where a good implementation need not worry about symlinks,
but should worry about drive letters and should convert '/' to '\\'). But at
least on cygwin, canonicalize-lgpl now honors // by virtue of deferring to the
working native realpath. On the other hand, cygwin realpath() behaves like all
other cygwin functions, such as stat() and open(), where garbage/.. is silently
collapsed whithout checking whether garbage exists or is a symlink.
Note that I've changed things around a bit - canonicalize now provides ONLY
canonicalize_filename_mode - the coreutils extension that allows for choosing
how strict the canonicalization needs to be (useful for implementing readlink
(1)). Meanwhile, canonicalize-lgpl is the ONLY provider of
canonicalize_file_name, and it ALSO provides a POSIX-compliant realpath, for
all systems (well, if there are any live glibc systems out there where
canonicalize_file_name(file/) still passes rather than failing with ENOENT,
then we need a subsequent patch to introduce rpl_canonicalize_file_name).
Does anyone want further changes, such as to the names of these modules? I
could reasonably see renaming canonicalize to canonicalize-mode, and
canonicalize-lgpl to either canonicalize_file_name or to realpath. I could
also try porting // handling to canonicalize-lgpl realpath (and propose the
same patch upstream to glibc), although I'd rather not do that until we find a
platform that honors // but does not have working realpath.
At any rate, I don't feel comfortable pushing this series until I have some
reviews or more feedback, and possibly a fix to cygwin to better handle '..'.
Jim, once I apply this series, coreutils will need to either: start importing
canonicalize-lgpl (right now, it avoids it), or change df.c to use
canonicalize_filename_mode(,CAN_EXISTING) (perhaps via a #define in system.h).
Eric Blake (6):
canonicalize-lgpl: reject non-directory with trailing slash
stdlib: sort witness names
canonicalize: leave canonicalize_file_name to canonicalize-lgpl
canonicalize-lgpl: use native realpath if it works
canonicalize: don't lose errno
canonicalize: honor // if distinct from /
From cfe26089b21bdddc721a619d4e7da550fc64e1fa Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Thu, 10 Sep 2009 12:12:16 -0600
Subject: [PATCH 1/6] canonicalize-lgpl: reject non-directory with trailing slash
* lib/canonicalize-lgpl.c (__realpath): Synchronize with glibc.
* tests/test-canonicalize-lgpl.c (main): Enhance test.
* tests/test-canonicalize.c (main): Likewise. Avoid collisions
with test-canonicalize-lgpl.sh run in parallel.
* tests/test-canonicalize.sh (tmpfiles): Use the name ise1, not
ise, for parallel make.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog | 10 ++
lib/canonicalize-lgpl.c| 11 +++
tests/test-canonicalize-lgpl.c | 16 +++-
tests/test-canonicalize.c | 26 +++---
tests/test-canonicalize.sh |6 +++---
5 files changed, 58 insertions(+), 11 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index c190ecc..de7b506 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2009-09-10 Eric Blake e...@byu.net
+
+ canonicalize-lgpl: reject non-directory with trailing slash
+ * lib/canonicalize-lgpl.c (__realpath): Synchronize with glibc.
+ * tests/test-canonicalize-lgpl.c (main): Enhance test.
+ * tests/test-canonicalize.c (main): Likewise. Avoid collisions
+ with test-canonicalize-lgpl.sh run in parallel.
+ * tests/test-canonicalize.sh (tmpfiles): Use the name ise1, not
+ ise, for parallel make.
+
2009-09-10 Ralf Wildenhues ralf.wildenh...@gmx.de
Remove obsolete macros from several modules.
diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c
index 6b5663a..c8f313e 100644
--- a/lib/canonicalize-lgpl.c
+++ b/lib/canonicalize-lgpl.c
@@ -298,6 +298,11 @@ __realpath (const char *name, char *resolved)
while ((--dest)[-1] != '/');
}
#endif
+ else if (!S_ISDIR (st.st_mode) *end != '\0')
+ {
+ __set_errno (ENOTDIR);
+ goto error;
+ }
}
}
if (dest rpath + 1 dest[-1] == '/')
@@ -307,16 +312,14 @@ __realpath (const char *name, char *resolved)
if (extra_buf)
freea (extra_buf
Re: canonicalize-lgpl bug
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 9/10/2009 12:21 PM: Hmm, on thinking about this more, there are probably existing Linux machines where glibc canonicalize_file_name is broken (fails to detect ENOTDIR on file/), and therefore we need to replace it. I have confirmed this. I'm working on respinning my patch series to replace canonicalize_file_name on Linux machines where it is broken. Additionally, it would be nice to declare canonicalize_file_name in stdlib.h. Also, now that POSIX requires realpath(name,NULL) to malloc() its results as an absolute path, it would be nice to have a realpath module that replaces broken versions (Solaris sometimes gives relative paths, and not all existing realpath implementations malloc). The current state of my series now does this. - -- 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/ iEYEARECAAYFAkqqOwMACgkQ84KuGfSFAYAblQCgw2t9vcweyVSPPdU9cFMO9oQ1 XxQAn30OZ/rYy2zs8xQtbc8+MBoSp7xV =MbyY -END PGP SIGNATURE-
Re: canonicalize-lgpl bug
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 9/11/2009 4:35 PM: [3/11] canonicalize: don't lose errno glibc still has a bug in realpath/c_f_n where errno could be inadvertently changed by a call to free() during an error return, but canonicalize-lgpl was immune, and now canonicalize is fixed. I guess I'll have to file a glibc bug report for the gnulib-glibc syncing (patch 9 gets the glibc-gnulib syncing) http://sources.redhat.com/bugzilla/show_bug.cgi?id=10635 [11/11] canonicalize: honor // if distinct from / Fixes c_f_mode handling of // on cygwin. This was easy to port to c_f_n as well. Updated series available here: git pull git://repo.or.cz/gnulib/ericb.git canonicalize - -- 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/ iEYEARECAAYFAkqrFMUACgkQ84KuGfSFAYAxtwCfUwKT0kv7POkRryVUg6bEmm8k PMAAn0P+hdktGhiW/iEGDkkP8ynZ5mzh =ukNo -END PGP SIGNATURE-
Re: readlink(1) behavior
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Jim Meyering on 9/10/2009 11:29 AM:
readlink -fv missing = lists /path/to/missing
readlink -fv missing/ = complains that missing is not a dir
I would like to see: 'readlink -fv missing/' list '/path/to/missing', on the
grounds that one could do 'mkdir missing' to bring the path into existence;
this matches the documentation that -f ignores a missing final element
(nowhere
does it say the final element has to be a non-file).
Yes, that makes sense.
This patch (on top of my canonicalize cleanups) does that:
- --
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/
iEYEARECAAYFAkqrur0ACgkQ84KuGfSFAYCBZgCfZQguPsWRKZJUh7WLF3Nli/tV
okIAoJOg78QYodvBq2ybYCGlaqmS/7Zq
=Yscq
-END PGP SIGNATURE-
From 424bfcda7e9dea15ae03e1332d7ca59b0d1b845d Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Sat, 12 Sep 2009 06:04:46 -0600
Subject: [PATCH] canonicalize: in CAN_ALL_BUT_LAST, allow trailing slash
* lib/canonicalize.c (canonicalize_filename_mode): Skip trailing
slashes when checking if last component is missing.
* tests/test-canonicalize.c (main): Test this.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |5 +
lib/canonicalize.c|8 ++--
tests/test-canonicalize.c | 16
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 5c03d38..c2290f3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2009-09-12 Eric Blake e...@byu.net
+ canonicalize: in CAN_ALL_BUT_LAST, allow trailing slash
+ * lib/canonicalize.c (canonicalize_filename_mode): Skip trailing
+ slashes when checking if last component is missing.
+ * tests/test-canonicalize.c (main): Test this.
+
canonicalize, canonicalize-lgpl: honor // if distinct from /
* modules/canonicalize (Files): Add double-slash-root.m4.
* modules/canonicalize-lgpl (Files): Likewise.
diff --git a/lib/canonicalize.c b/lib/canonicalize.c
index a90f1a8..2374169 100644
--- a/lib/canonicalize.c
+++ b/lib/canonicalize.c
@@ -183,8 +183,12 @@ canonicalize_filename_mode (const char *name,
canonicalize_mode_t can_mode)
saved_errno = errno;
if (can_mode == CAN_EXISTING)
goto error;
- if (can_mode == CAN_ALL_BUT_LAST *end)
- goto error;
+ if (can_mode == CAN_ALL_BUT_LAST)
+ {
+ if (end[strspn (end, /)])
+ goto error;
+ continue;
+ }
st.st_mode = 0;
}
diff --git a/tests/test-canonicalize.c b/tests/test-canonicalize.c
index f8dcc54..463297f 100644
--- a/tests/test-canonicalize.c
+++ b/tests/test-canonicalize.c
@@ -240,26 +240,42 @@ main ()
{
char *result1 = canonicalize_filename_mode (BASE /zzz, CAN_ALL_BUT_LAST);
char *result2 = canonicalize_filename_mode (BASE /zzz, CAN_MISSING);
+char *result3 = canonicalize_filename_mode (BASE /zzz/,
CAN_ALL_BUT_LAST);
+char *result4 = canonicalize_filename_mode (BASE /zzz/, CAN_MISSING);
ASSERT (result1 != NULL);
ASSERT (result2 != NULL);
+ASSERT (result3 != NULL);
+ASSERT (result4 != NULL);
ASSERT (strcmp (result1, result2) == 0);
+ASSERT (strcmp (result2, result3) == 0);
+ASSERT (strcmp (result3, result4) == 0);
ASSERT (strcmp (result1 + strlen (result1) - strlen (/ BASE /zzz),
/ BASE /zzz) == 0);
free (result1);
free (result2);
+free (result3);
+free (result4);
}
/* Check that alternate modes can resolve broken symlink basenames. */
{
char *result1 = canonicalize_filename_mode (BASE /ouk, CAN_ALL_BUT_LAST);
char *result2 = canonicalize_filename_mode (BASE /ouk, CAN_MISSING);
+char *result3 = canonicalize_filename_mode (BASE /ouk/,
CAN_ALL_BUT_LAST);
+char *result4 = canonicalize_filename_mode (BASE /ouk/, CAN_MISSING);
ASSERT (result1 != NULL);
ASSERT (result2 != NULL);
+ASSERT (result3 != NULL);
+ASSERT (result4 != NULL);
ASSERT (strcmp (result1, result2) == 0);
+ASSERT (strcmp (result2, result3) == 0);
+ASSERT (strcmp (result3, result4) == 0);
ASSERT (strcmp (result1 + strlen (result1) - strlen (/ BASE /wum),
/ BASE /wum) == 0);
free (result1);
free (result2);
+free (result3);
+free (result4);
}
/* Check that alternate modes can handle missing dirnames. */
--
1.6.3.3.334.g916e1
mingw remove bug
mingw remove() complies with C89, but not with POSIX. I'll be committing this
shortly (actually, first I probably need to commit a stat module, to make up
for the fact that both mingw rmdir(dir) and rmdir(dir/) can succeed, even
when one of stat(dir) and stat(dir/) fails, depending on which directory
was passed in).
From: Eric Blake e...@byu.net
Date: Mon, 14 Sep 2009 16:57:55 -0600
Subject: [PATCH] remove: new module
* modules/remove: New file.
* lib/remove.c: Likewise.
* m4/remove.m4 (gl_FUNC_REMOVE): Likewise.
* m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Add witnesses.
* modules/stdio (Makefile.am): Use them.
* lib/stdio.in.h (remove): Declare replacement.
* MODULES.html.sh (systems lacking POSIX:2008): Mention module.
* doc/posix-functions/remove.texi (remove): Likewise.
* modules/remove-tests: New test.
* tests/test-remove.c: Likewise.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog | 14 +
MODULES.html.sh |1 +
doc/posix-functions/remove.texi |5 ++-
lib/remove.c| 63
lib/stdio.in.h | 14 +
m4/remove.m4| 32
m4/stdio_h.m4 |4 +-
modules/remove | 26 ++
modules/remove-tests| 12 +
modules/stdio |2 +
tests/test-remove.c | 102 +++
11 files changed, 273 insertions(+), 2 deletions(-)
create mode 100644 lib/remove.c
create mode 100644 m4/remove.m4
create mode 100644 modules/remove
create mode 100644 modules/remove-tests
create mode 100644 tests/test-remove.c
diff --git a/ChangeLog b/ChangeLog
index dc32b4d..1c5b190 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2009-09-14 Eric Blake e...@byu.net
+
+ remove: new module
+ * modules/remove: New file.
+ * lib/remove.c: Likewise.
+ * m4/remove.m4 (gl_FUNC_REMOVE): Likewise.
+ * m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Add witnesses.
+ * modules/stdio (Makefile.am): Use them.
+ * lib/stdio.in.h (remove): Declare replacement.
+ * MODULES.html.sh (systems lacking POSIX:2008): Mention module.
+ * doc/posix-functions/remove.texi (remove): Likewise.
+ * modules/remove-tests: New test.
+ * tests/test-remove.c: Likewise.
+
2009-09-13 Jim Meyering meyer...@redhat.com
posixtm: don't reject a time that specify 60 as the number of seconds
diff --git a/MODULES.html.sh b/MODULES.html.sh
index 76741b3..6cfd968 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -2319,6 +2319,7 @@ func_all_modules ()
func_module realloc-posix
func_module recv
func_module recvfrom
+ func_module remove
func_module sched
func_module select
func_module send
diff --git a/doc/posix-functions/remove.texi b/doc/posix-functions/remove.texi
index 3995006..6d687d8 100644
--- a/doc/posix-functions/remove.texi
+++ b/doc/posix-functions/remove.texi
@@ -4,10 +4,13 @@ remove
POSIX specification: @url
{http://www.opengroup.org/onlinepubs/9699919799/functions/remove.html}
-Gnulib module: ---
+Gnulib module: remove
Portability problems fixed by Gnulib:
@itemize
+...@item
+This function does not remove empty directories on some platforms:
+mingw.
@end itemize
Portability problems not fixed by Gnulib:
diff --git a/lib/remove.c b/lib/remove.c
new file mode 100644
index 000..90d5b2f
--- /dev/null
+++ b/lib/remove.c
@@ -0,0 +1,63 @@
+/* Remove a file or directory.
+ Copyright (C) 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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see http://www.gnu.org/licenses/. */
+
+/* written by Eric Blake */
+
+#include config.h
+
+#include stdio.h
+
+#include errno.h
+#include string.h
+#include sys/stat.h
+#include unistd.h
+
+#undef remove
+
+/* Remove NAME from the file system. */
+int
+rpl_remove (char const *name)
+{
+ /* Mingw remove() fails with EPERM on empty directories. */
+ struct stat st;
+ size_t len = strlen (name);
+ if (!len)
+{
+ errno = ENOENT;
+ return -1;
+}
+
+ if (lstat (name, st) == 0 S_ISDIR (st.st_mode))
+{
+ /* Mingw rmdir(empty/.) mistakenly succeeds. */
+ while (ISSLASH (name[len - 1]))
+len--;
+ if (name[len - 1] == '.' (1 == len || ISSLASH (name[len - 2])))
+{
+ errno = EINVAL;
+ return -1
Re: mingw remove bug
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Ben Pfaff on 9/14/2009 5:51 PM:
+ /* Mingw remove(file/) fails with EINVAL, instead of the required
+ ENOTDIR. */
+ if (ISSLASH (name[len - 1]))
+{
+ errno = ENOTDIR;
+ return -1;
+}
I believe that this will return ENOTDIR for a file whose name
ends in '/' or '\' and on which lstat() fails (e.g. for a file
that doesn't exist).
I tested; it failed with EINVAL on mingw. And on Solaris 8,
unlink(file/) succeeds at removing file; I need to test whether
remove(file/) succeeds as well, or else enhance remove.m4 to work around
that bug too.
- --
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/
iEYEARECAAYFAkqu2p8ACgkQ84KuGfSFAYBJVgCgw/NbfFgNnJKzSUTLPlSZG19P
CBAAn1gXrc3tmQi0yadACSpTGPtGRDHp
=U16t
-END PGP SIGNATURE-
Re: fchdir on mingw
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Eric Blake on 9/1/2009 10:55 AM:
Eric Blake (5):
fchdir: fix off-by-one bug in previous patch
fchdir: simplify error handling, and support dup3
fchdir: use more consistent macro convention
This particular patch wasn't quite consistent with other replacement
macros. I'm pushing this followup:
- --
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/
iEYEARECAAYFAkqwYJsACgkQ84KuGfSFAYAOJgCeN3uOMkcSL183t6YteqTe5u0K
9nEAoNkHZTfaxabjuCryKU/HG7hEsb0U
=eKlF
-END PGP SIGNATURE-
From c9dd0005fe00602d13c3d54f0834655b1e769520 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Tue, 15 Sep 2009 15:54:43 -0600
Subject: [PATCH] fchdir: improve use of replacement functions
* m4/fchdir.m4 (gl_FUNC_FCHDIR): Set appropriate witnesses.
* m4/sys_stat_h.m4 (gl_SYS_STAT_H_DEFAULTS): Add REPLACE_FSTAT.
* m4/dirent_h.m4 (gl_DIRENT_H_DEFAULTS): Add REPLACE_OPENDIR,
REPLACE_CLOSEDIR.
* m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add REPLACE_DUP.
* modules/sys_stat (Makefile.am): Substitute correct witness.
* modules/dirent (Makefile.am): Likewise.
* modules/unistd (Makefile.am): Likewise.
* lib/dirent.in.h (opendir, closedir): Use better witnesses.
* lib/unistd.in.h (dup): Likewise.
* lib/sys_stat.in.h (fstat): Likewise.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog | 13 +
lib/dirent.in.h | 11 ++-
lib/sys_stat.in.h |2 +-
lib/unistd.in.h |8
m4/dirent_h.m4|4 +++-
m4/fchdir.m4 |8 +++-
m4/sys_stat_h.m4 |3 ++-
m4/unistd_h.m4|3 ++-
modules/dirent|3 ++-
modules/sys_stat |2 +-
modules/unistd|1 +
11 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 322aca5..e58b782 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
2009-09-15 Eric Blake e...@byu.net
+ fchdir: improve use of replacement functions
+ * m4/fchdir.m4 (gl_FUNC_FCHDIR): Set appropriate witnesses.
+ * m4/sys_stat_h.m4 (gl_SYS_STAT_H_DEFAULTS): Add REPLACE_FSTAT.
+ * m4/dirent_h.m4 (gl_DIRENT_H_DEFAULTS): Add REPLACE_OPENDIR,
+ REPLACE_CLOSEDIR.
+ * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add REPLACE_DUP.
+ * modules/sys_stat (Makefile.am): Substitute correct witness.
+ * modules/dirent (Makefile.am): Likewise.
+ * modules/unistd (Makefile.am): Likewise.
+ * lib/dirent.in.h (opendir, closedir): Use better witnesses.
+ * lib/unistd.in.h (dup): Likewise.
+ * lib/sys_stat.in.h (fstat): Likewise.
+
maint: ignore gnulib-tool temp files
* .gitignore: Ignore files created during gnulib-tool --test.
diff --git a/lib/dirent.in.h b/lib/dirent.in.h
index 8930765..6da77d9 100644
--- a/lib/dirent.in.h
+++ b/lib/dirent.in.h
@@ -35,15 +35,11 @@ extern C {
/* Declare overridden functions. */
-#if @REPLACE_FCHDIR@
-# define opendir rpl_opendir
-extern DIR * opendir (const char *);
+#if @REPLACE_CLOSEDIR@
# define closedir rpl_closedir
extern int closedir (DIR *);
#endif
-/* Declare other POSIX functions. */
-
#if @GNULIB_DIRFD@
# if !...@have_decl_dirfd@ !defined dirfd
/* Return the file descriptor associated with the given directory stream,
@@ -75,6 +71,11 @@ extern DIR *fdopendir (int fd);
fdopendir (f))
#endif
+#if @REPLACE_OPENDIR@
+# define opendir rpl_opendir
+extern DIR * opendir (const char *);
+#endif
+
#if @GNULIB_SCANDIR@
/* Scan the directory DIR, calling FILTER on each directory entry.
Entries for which FILTER returns nonzero are individually malloc'd,
diff --git a/lib/sys_stat.in.h b/lib/sys_stat.in.h
index b0ea92d..869cb0f 100644
--- a/lib/sys_stat.in.h
+++ b/lib/sys_stat.in.h
@@ -369,7 +369,7 @@ int mknodat (int fd, char const *file, mode_t mode, dev_t
dev);
mknodat (f, n, m, d))
#endif
-#if @REPLACE_FCHDIR@
+#if @REPLACE_FSTAT@
# define fstat rpl_fstat
extern int fstat (int fd, struct stat *buf);
#endif
diff --git a/lib/unistd.in.h b/lib/unistd.in.h
index f191412..553fb06 100644
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -231,6 +231,10 @@ extern int close (int);
close (f))
#endif
+#if @REPLACE_DUP@
+# define dup rpl_dup
+extern int dup (int);
+#endif
#if @GNULIB_DUP2@
# if @REPLACE_DUP2@
@@ -312,7 +316,6 @@ extern int euidaccess (const char *filename, int mode);
#if @GNULIB_FCHDIR@
# if @REPLACE_FCHDIR@
-
/* Change the process' current working directory to the directory on which
the given file descriptor is open.
Return 0 if successful, otherwise -1 and errno set.
@@ -320,9 +323,6 @@ extern int euidaccess (const char *filename, int mode);
http://www.opengroup.org/susv3xsh/fchdir.html. */
extern int fchdir (int /*fd*/);
-# define
Re: canonicalize-lgpl bug
Jim Meyering jim at meyering.net writes: [3/11] canonicalize: don't lose errno glibc still has a bug in realpath/c_f_n where errno could be inadvertently changed by a call to free() during an error return, but canonicalize-lgpl was immune, and now canonicalize is fixed. I guess I'll have to file a glibc bug report for the gnulib-glibc syncing (patch 9 gets the glibc-gnulib syncing) To ease future glibc/gnulib syncing, it'd be better not to change __set_errno (...) to errno = ... No? I guess I was a bit confusing in my description. canonicalize-lgpl and glibc are in sync (well, patched to be in sync), and in that file, I continued to keep __set_errno. canonicalize (GPL) adds a new interface, c_f_mode, and given our track record of convincing Ulrich to add new interfaces to glibc, it's doubtful that we'll be able to push any of canonicalize.c back to glibc. Besides, canonicalize.c has already made many other breaks from when it originally forked from glibc, such as renaming 'rpath' to 'rname'. So this particular __set_errno=errno cleanup is in line with gnulib owning the file. Also, if you do make a mechanical change like that, it's easier on reviewers and general maintenance to keep that in a change-set separate from any delta that makes a semantic change, like your don't lose errno fix does. OK, I'll split that patch into two. I've reached this point in reading the patches. So far they look fine. I will read the remainder, and test tomorrow. I'll try and rebase my series before then. BTW, I appreciate these commentaries. Have you considered adding something like that to each commit log? Will do as part of my rebase. -- Eric Blake
Re: mingw remove bug
Eric Blake ebb9 at byu.net writes: I'm arguing that the second program should also report No such file or directory. Ah, so for 'foo/', the code should distinguish between ENOENT and ENOTDIR, based on whether 'foo' exists. I'll update the patch and test accordingly. I'm taking a step back. Rather than my first attempt at rpl_remove, it seems like I should be able to get away with much simpler code, provided that the underlying syscalls aren't broken. It certainly has the benefit of avoiding an unnecessary [l]stat: int result = rmdir (name); if (result == -1 errno == ENOTDIR) result = unlink (name); return result; (glibc remove() does it the other way around: attempting unlink first and only calling rmdir if errno == EISDIR/EPERM. This works fine on Linux, but is too dangerous on platforms where unlink can remove a directory) So, I found two reasons that the rmdir module should no longer be obsolete - cygwin 1.5.x mistakenly removes a directory given with a trailing ./, even though POSIX requires this to fail with EINVAL; and mingw rmdir(file/) fails with EINVAL instead of ENOTDIR (which would leak EINVAL out of the above rpl_remove() logic, instead of confirming ENOTDIR failure with unlink(file/)). Meanwhile, the rmdir-errno module, in use by coreutils until today, guessed wrong for cross-compilation to Solaris (where rmdir fails with EEXIST, not ENOTEMPTY, on a populated directory); now that coreutils no longer uses the module [1], I see no reason to keep the module alive. [1] http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/18181/focus=18182 By the way, this patch series also fixes unlinkat(-1,/dir/./,AT_REMOVEDIR) on cygwin 1.5.x, since the openat module already depended on rmdir. So far, I don't know of any systems with native unlinkat but with broken rmdir, but I suppose I should add tests for unlinkat to verify the same behavior. Technically, GNU/Linux is not POSIX-compliant: rmdir(symlink-to-empty-dir/) fails with ENOTDIR, instead of succeeding at turning symlink-to-empty-dir into a dangling link. But this behavior is counter-intuitive (both rmdir(symlink- to-file/) and rmdir(symlink-to-full-dir/) are required to fail, so why is symlink-to-empty-dir special?), and GNU/Linux is at least consistent (rename (symlink-to-empty-dir/,other) also has non-POSIX behavior to prevent accidental dangling symlink creation). So I intentionally don't want to penalize GNU/Linux with a rmdir(2) wrapper that creates dangling symlinks; if we want rm(1) and rmdir(1) to do so under POSIXLY_CORRECT, then they can perform the extra checks manually. Here's what I plan on pushing later today. From: Eric Blake e...@byu.net Date: Wed, 16 Sep 2009 10:08:55 -0600 Subject: [PATCH 1/2] rmdir: work around cygwin 1.5.x and mingw bugs * m4/rmdir.m4 (gl_FUNC_RMDIR): Detect the bugs. * lib/rmdir.c (rmdir): Work around it. * modules/rmdir (Status, Notice): No longer obsolete. (Files): Add dos.m4. (Depends-on): Add unistd. (configure.ac): Set witnesses. (License): Relax to LGPLv2+. * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Set defaults. * modules/unistd (Makefile.am): Substitute witnesses. * lib/unistd.in.h (rmdir): Declare replacement. * doc/posix-functions/rmdir.texi (rmdir): Document this. * modules/rmdir-tests: New tests. * tests/test-rmdir.c: Likewise. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog | 17 ++ doc/posix-functions/rmdir.texi | 13 lib/rmdir.c| 38 ++-- lib/unistd.in.h| 15 + m4/rmdir.m4| 34 +++- m4/unistd_h.m4 |4 +- modules/rmdir | 13 ++--- modules/rmdir-tests| 11 modules/unistd |2 + tests/test-rmdir.c | 125 10 files changed, 256 insertions(+), 16 deletions(-) create mode 100644 modules/rmdir-tests create mode 100644 tests/test-rmdir.c diff --git a/ChangeLog b/ChangeLog index e58b782..c6b3e25 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2009-09-16 Eric Blake e...@byu.net + + rmdir: work around cygwin 1.5.x and mingw bugs + * m4/rmdir.m4 (gl_FUNC_RMDIR): Detect the bugs. + * lib/rmdir.c (rmdir): Work around it. + * modules/rmdir (Status, Notice): No longer obsolete. + (Files): Add dos.m4. + (Depends-on): Add unistd. + (configure.ac): Set witnesses. + (License): Relax to LGPLv2+. + * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Set defaults. + * modules/unistd (Makefile.am): Substitute witnesses. + * lib/unistd.in.h (rmdir): Declare replacement. + * doc/posix-functions/rmdir.texi (rmdir): Document this. + * modules/rmdir-tests: New tests. + * tests/test-rmdir.c: Likewise. + 2009-09-15 Eric Blake e...@byu.net fchdir: improve use of replacement functions diff --git a/doc/posix-functions/rmdir.texi b/doc
Re: mingw remove bug
Jim Meyering jim at meyering.net writes:
Meanwhile, the rmdir-errno module, in use by coreutils until today, guessed
wrong for cross-compilation to Solaris (where rmdir fails with EEXIST, not
ENOTEMPTY, on a populated directory); now that coreutils no longer uses the
module [1], I see no reason to keep the module alive.
I was thinking of marking it as obsolete,
(AFAIK, there's been no actual problem report)
but if coreutils was the only user, deleting should be ok.
OK, I'll be nice, as obsoletion gives a bit longer transition phase if anyone
else was using rmdir-errno. Also, I added a test of unlinkat, to make sure
that rmdir bugs don't reappear. Path [1/2] of the previous mail is unchanged,
but here's what replaces [2/2] and a new [3/2]:
From: Eric Blake e...@byu.net
Date: Wed, 16 Sep 2009 13:31:05 -0600
Subject: [PATCH 2/2] rmdir-errno: mark obsolete, it is unsafe for cross-
compilation
* modules/rmdir-errno (Status, Notice): Now obsolete.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |3 +++
modules/rmdir-errno |6 ++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index c6b3e25..e7f3669 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
2009-09-16 Eric Blake e...@byu.net
+ rmdir-errno: mark obsolete, it is unsafe for cross-compilation
+ * modules/rmdir-errno (Status, Notice): Now obsolete.
+
rmdir: work around cygwin 1.5.x and mingw bugs
* m4/rmdir.m4 (gl_FUNC_RMDIR): Detect the bugs.
* lib/rmdir.c (rmdir): Work around it.
diff --git a/modules/rmdir-errno b/modules/rmdir-errno
index d0b81b6..ef02a95 100644
--- a/modules/rmdir-errno
+++ b/modules/rmdir-errno
@@ -1,6 +1,12 @@
Description:
rmdir errno for nonempty directories
+Status:
+obsolete
+
+Notice:
+This module is obsolete.
+
Files:
m4/rmdir-errno.m4
--
1.6.4.2
From c433306b7db7440e27df2150decba6b89ecfc625 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Wed, 16 Sep 2009 13:21:46 -0600
Subject: [PATCH 3/2] openat-tests: ensure unlinkat behaves like rmdir
* tests/test-rmdir.c (main): Factor guts...
* tests/test-rmdir.h (test_rmdir_func): ...into new file.
* modules/rmdir-tests (Files): Ship new file.
* modules/openat-tests: New test.
* tests/test-unlinkat.c: Likewise.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog|7 +++
modules/openat-tests | 13 ++
modules/rmdir-tests |1 +
tests/test-rmdir.c | 78 +
tests/{test-rmdir.c = test-rmdir.h} | 63 +---
tests/test-unlinkat.c| 61 ++
6 files changed, 105 insertions(+), 118 deletions(-)
create mode 100644 modules/openat-tests
copy tests/{test-rmdir.c = test-rmdir.h} (63%)
create mode 100644 tests/test-unlinkat.c
diff --git a/ChangeLog b/ChangeLog
index e7f3669..94e07e6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2009-09-16 Eric Blake e...@byu.net
+ openat-tests: ensure unlinkat behaves like rmdir
+ * tests/test-rmdir.c (main): Factor guts...
+ * tests/test-rmdir.h (test_rmdir_func): ...into new file.
+ * modules/rmdir-tests (Files): Ship new file.
+ * modules/openat-tests: New test.
+ * tests/test-unlinkat.c: Likewise.
+
rmdir-errno: mark obsolete, it is unsafe for cross-compilation
* modules/rmdir-errno (Status, Notice): Now obsolete.
diff --git a/modules/openat-tests b/modules/openat-tests
new file mode 100644
index 000..a82a4f3
--- /dev/null
+++ b/modules/openat-tests
@@ -0,0 +1,13 @@
+Files:
+tests/test-rmdir.h
+tests/test-unlinkat.c
+
+Depends-on:
+
+configure.ac:
+AC_CHECK_FUNCS_ONCE([symlink])
+
+Makefile.am:
+TESTS += test-unlinkat
+check_PROGRAMS += test-unlinkat
+test_unlinkat_LDADD = $(LDADD) @LIBINTL@
diff --git a/modules/rmdir-tests b/modules/rmdir-tests
index f5b69f8..2a68120 100644
--- a/modules/rmdir-tests
+++ b/modules/rmdir-tests
@@ -1,4 +1,5 @@
Files:
+tests/test-rmdir.h
tests/test-rmdir.c
Depends-on:
diff --git a/tests/test-rmdir.c b/tests/test-rmdir.c
index 17fb0ff..6d55ea9 100644
--- a/tests/test-rmdir.c
+++ b/tests/test-rmdir.c
@@ -44,82 +44,10 @@
#define BASE test-rmdir.t
+#include test-rmdir.h
+
int
main ()
{
- /* Remove any leftovers from a previous partial run. */
- ASSERT (system (rm -rf BASE *) == 0);
-
- /* Setup. */
- ASSERT (mkdir (BASE dir, 0700) == 0);
- ASSERT (close (creat (BASE dir/file, 0600)) == 0);
-
- /* Basic error conditions. */
- errno = 0;
- ASSERT (rmdir () == -1);
- ASSERT (errno == ENOENT);
- errno = 0;
- ASSERT (rmdir (BASE nosuch) == -1);
- ASSERT (errno == ENOENT);
- errno = 0;
- ASSERT (rmdir (BASE nosuch/) == -1);
- ASSERT (errno == ENOENT);
- errno = 0;
- ASSERT (rmdir (.) == -1);
- ASSERT (errno == EINVAL || errno == EBUSY);
- /* Resulting errno after .. or / is too varied to test
improve unlink on Solaris
I'm working on a patch to fix unlink(file/) on Solaris 9. But as a prerequisite (to avoid anyone corrupting their file system if running the unit test as root, and accidentally unlinking an empty directory), I noticed that mingw failed to compile unlinkdir.c due to a missing geteuid. This works around it: From: Eric Blake e...@byu.net Date: Wed, 16 Sep 2009 17:18:28 -0600 Subject: [PATCH] unlinkdir: port to mingw * m4/unlinkdir.m4 (gl_UNLINKDIR): Add mingw to list of platforms that refuse anyone to unlink a directory. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog |6 ++ m4/unlinkdir.m4 |5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index ff11ebe..1b30e94 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2009-09-16 Eric Blake e...@byu.net + unlinkdir: port to mingw + * m4/unlinkdir.m4 (gl_UNLINKDIR): Add mingw to list of platforms + that refuse anyone to unlink a directory. + +2009-09-16 Eric Blake e...@byu.net + remove: new module * modules/remove: New file. * lib/remove.c: Likewise. diff --git a/m4/unlinkdir.m4 b/m4/unlinkdir.m4 index 0c84375..a8256d1 100644 --- a/m4/unlinkdir.m4 +++ b/m4/unlinkdir.m4 @@ -16,7 +16,7 @@ AC_DEFUN([gl_UNLINKDIR], AC_LIBOBJ([unlinkdir]) # The Hurd, the Linux kernel, the FreeBSD kernel version 2.2 and later, - # and Cygwin never let anyone (even root) unlink directories. + # Cygwin, and mingw never let anyone (even root) unlink directories. # If anyone knows of another system for which unlink can never # remove a directory, please report it to bug-coreut...@gnu.org. # Unfortunately this is difficult to test for, since it requires root access @@ -26,7 +26,8 @@ AC_DEFUN([gl_UNLINKDIR], *-*-gnu[[0-9]]* | \ *-*-linux-* | *-*-linux | \ *-*-freebsd2.2* | *-*-freebsd[[3-9]]* | *-*-freebsd[[1-9]][[0-9]]* | \ - *-cygwin) + *-cygwin | \ + *-mingw) AC_DEFINE([UNLINK_CANNOT_UNLINK_DIR], [1], [Define to 1 if unlink (dir) cannot possibly succeed.]);; esac -- 1.6.4.2
Re: canonicalize-lgpl bug
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 9/16/2009 9:11 AM: I've reached this point in reading the patches. So far they look fine. I will read the remainder, and test tomorrow. I'll try and rebase my series before then. Now rebased: git pull git://repo.or.cz/gnulib/ericb.git canonicalize I'm seeing a test failure on older Linux seeing a link error on rpl_canonicalize_file_name, so I'll have to rebase again once I solve that. - -- 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/ iEYEARECAAYFAkqxtsAACgkQ84KuGfSFAYDtsgCggxcVBicDa28f8YTFCT7Q9j70 S6MAnjdJ/ed2dqpgqMGJof/+nqtUOclX =5axz -END PGP SIGNATURE-
Re: time_t on NetBSD/i386
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Bruno Haible on 9/17/2009 1:38 AM:
Here is a proposed patch. I checked all uses of 'time_t' in getdate.y and
found only two conversions from time_t to 'long int', in the 'relunit' rule:
| tSDECIMAL_NUMBER tSEC_UNIT
{ $$ = RELATIVE_TIME_0; $$.seconds = $1.tv_sec; $$.ns = $1.tv_nsec; }
| tUDECIMAL_NUMBER tSEC_UNIT
{ $$ = RELATIVE_TIME_0; $$.seconds = $1.tv_sec; $$.ns = $1.tv_nsec; }
So this patch should handle it. Jim, Eric, opinions?
Looks reasonable to me.
- --
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/
iEYEARECAAYFAkqyHSMACgkQ84KuGfSFAYAVBACeMwqJDImtaf6ZY7XU5+L6baS+
Yl8An36E00OX17I5OveN7+bd3q5UToOE
=D4H8
-END PGP SIGNATURE-
Re: canonicalize-lgpl bug
According to Eric Blake on 9/16/2009 10:10 PM: According to Eric Blake on 9/16/2009 9:11 AM: I've reached this point in reading the patches. So far they look fine. I will read the remainder, and test tomorrow. I'll try and rebase my series before then. Now rebased: git pull git://repo.or.cz/gnulib/ericb.git canonicalize I'm seeing a test failure on older Linux seeing a link error on rpl_canonicalize_file_name, so I'll have to rebase again once I solve that. Here's what I squashed in to fix that problem; I've rebased again. diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c index 63aecb5..fbde01b 100644 --- a/lib/canonicalize-lgpl.c +++ b/lib/canonicalize-lgpl.c @@ -19,7 +19,7 @@ # include config.h #endif -#if !HAVE_CANONICALIZE_FILE_NAME || defined _LIBC +#if !HAVE_CANONICALIZE_FILE_NAME || !FUNC_REALPATH_WORKS || defined _LIBC /* Specification. */ #include stdlib.h -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net signature.asc Description: OpenPGP digital signature
module Include: section
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Some modules have: Include: #include file.h but most just have: Include: file.h Any objections to a patch that normalizes the outliers to drop #include? - -- 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/ iEYEARECAAYFAkqyMIsACgkQ84KuGfSFAYBxfACfX0PzK19BQqofK+UhXSGpvymD kRwAnjrGCrUY3VeQXPf+QKrwVxrqQw40 =afz3 -END PGP SIGNATURE-
Re: License pedantry
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Ian Beckwith on 9/17/2009 6:51 AM: Hi, I've recently adopted the Debian gnulib package. Our argument has been that gnulib is for developers, and probably should not be distributed as a package. Read gnulib-intro.texi for why we think that snapshots of gnulib are counter-productive. Documenting the copyright and licenses of all the files (a tedious but (currently) necessary process), I spotted a few extremely minor issues, and ran into a few things I'd like clarifying. The attached patch: * Replaces incorrect FSF addresses with pointers to www.gnu.org/licenses. (picked up by 'licensecheck', in the debian devscripts package). Thanks for the reminder. However, gnulib-tool already replaces the licenses with the correct text (LGPLv2, LGPLv3, GPLv2, or GPLv3) when copying from gnulib into a project. We've been intending to mass-update all gnulib files once we sort out exactly which license should be used in the repository, even though that may be different than what the end project ends up using. * Removes a stray 'dnl' in the middle of a copyright notice. Which file? This should probably be an independent patch. * Updates users.txt with pointers to the gnuit home and gitweb pages. This should likewise be a separate patch. modules/COPYING and modules/README both claim to apply to all the files in modules/, but have different copyright years and slightly different license wording. Of the licenses specified in files in modules/: When [L]GPL is specified, is that any version? version 3 or later? LGPLv2+ - version 2 or later LGPL - version 3 or later GPLv2+ - version 2 or later GPL - version 3 or later This is documented in gnulib-intro.texi, under the node Copyright. If that section is confusing, we welcome improvements. The version specified in the actual source files? The version specified in a source files is ONE of the licenses that the file can be shipped under; but gnulib-tool can be used to select another compatible license according to the project's needs. Does unmodifiable license text correspond to: Everyone is permitted to copy and distribute verbatim copies of this license document, but changing it is not allowed. Yes; this refers to files like the GPL text. Does unlimited correspond to: This file is free software; the Free Software Foundation gives unlimited permission to copy and/or distribute it, with or without modifications, as long as this notice is preserved. Yes; this refers to files like *.m4. - -- 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/ iEYEARECAAYFAkqyM6AACgkQ84KuGfSFAYDSgQCfW13lySvrllutXKjcx6Kdgd0o NysAnRQY0WOqo8wJk0CvqdzVtOuD8vOS =rfdz -END PGP SIGNATURE-
Re: module Include: section
Eric Blake ebb9 at byu.net writes: Any objections to a patch that normalizes the outliers to drop #include? As in the following? From 2655892372305598ee062e56f30b01c7d90ea178 Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Thu, 17 Sep 2009 11:49:36 -0600 Subject: [PATCH] maint: make Include sections of modules consistent Done with sed -i 's/^#include //' `git grep ^#include modules`. * modules/alloca: Use only header name; no need to list #include. * modules/alloca-opt: Likewise. * modules/arpa_inet: Likewise. * modules/canon-host: Likewise. * modules/configmake: Likewise. * modules/dirent: Likewise. * modules/eealloc: Likewise. * modules/environ: Likewise. * modules/fchdir: Likewise. * modules/fcntl: Likewise. * modules/fcntl-h: Likewise. * modules/gethrxtime: Likewise. * modules/gettime: Likewise. * modules/ignore-value: Likewise. * modules/inet_ntop: Likewise. * modules/inet_pton: Likewise. * modules/inttypes: Likewise. * modules/isnand-nolibm: Likewise. * modules/isnanf-nolibm: Likewise. * modules/mbchar: Likewise. * modules/mbfile: Likewise. * modules/mbiter: Likewise. * modules/mbuiter: Likewise. * modules/netdb: Likewise. * modules/netinet_in: Likewise. * modules/nproc: Likewise. * modules/pagealign_alloc: Likewise. * modules/poll: Likewise. * modules/printf-frexp: Likewise. * modules/pthread: Likewise. * modules/putenv: Likewise. * modules/random_r: Likewise. * modules/relocatable-prog: Likewise. * modules/search: Likewise. * modules/select: Likewise. * modules/selinux-h: Likewise. * modules/settime: Likewise. * modules/signal: Likewise. * modules/size_max: Likewise. * modules/socklen: Likewise. * modules/ssize_t: Likewise. * modules/stdarg: Likewise. * modules/stdbool: Likewise. * modules/stddef: Likewise. * modules/stdint: Likewise. * modules/stdio: Likewise. * modules/stdlib: Likewise. * modules/string: Likewise. * modules/strings: Likewise. * modules/sys_file: Likewise. * modules/sys_ioctl: Likewise. * modules/sys_select: Likewise. * modules/sys_socket: Likewise. * modules/sys_stat: Likewise. * modules/sys_time: Likewise. * modules/sys_times: Likewise. * modules/sys_utsname: Likewise. * modules/sys_wait: Likewise. * modules/sysexits: Likewise. * modules/time: Likewise. * modules/times: Likewise. * modules/tmpfile: Likewise. * modules/trim: Likewise. * modules/unistd: Likewise. * modules/wchar: Likewise. * modules/wctype: Likewise. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog| 70 ++ modules/alloca |2 +- modules/alloca-opt |2 +- modules/arpa_inet|2 +- modules/canon-host |2 +- modules/configmake |2 +- modules/dirent |2 +- modules/eealloc |2 +- modules/environ |2 +- modules/fchdir |2 +- modules/fcntl|2 +- modules/fcntl-h |2 +- modules/gethrxtime |2 +- modules/gettime |2 +- modules/ignore-value |2 +- modules/inet_ntop|2 +- modules/inet_pton|2 +- modules/inttypes |2 +- modules/isnand-nolibm|2 +- modules/isnanf-nolibm|2 +- modules/mbchar |2 +- modules/mbfile |2 +- modules/mbiter |2 +- modules/mbuiter |2 +- modules/netdb|2 +- modules/netinet_in |2 +- modules/nproc|2 +- modules/pagealign_alloc |2 +- modules/poll |2 +- modules/printf-frexp |2 +- modules/pthread |2 +- modules/putenv |2 +- modules/random_r |2 +- modules/relocatable-prog |4 +- modules/search |2 +- modules/select |2 +- modules/selinux-h|4 +- modules/settime |2 +- modules/signal |2 +- modules/size_max |2 +- modules/socklen |4 +- modules/ssize_t |2 +- modules/stdarg |2 +- modules/stdbool |2 +- modules/stddef |2 +- modules/stdint |2 +- modules/stdio|2 +- modules/stdlib |2 +- modules/string |2 +- modules/strings |2 +- modules/sys_file |2 +- modules/sys_ioctl|2 +- modules/sys_select |2 +- modules/sys_socket |2 +- modules/sys_stat |2 +- modules/sys_time |2 +- modules/sys_times|2 +- modules/sys_utsname |2 +- modules/sys_wait |2 +- modules/sysexits |2 +- modules/time |2 +- modules/times|2 +- modules/tmpfile |2 +- modules/trim |2 +- modules/unistd |2 +- modules/wchar|2 +- modules/wctype |2 +- 67 files changed, 139 insertions(+), 69 deletions(-) diff --git a/ChangeLog b/ChangeLog
Re: gnulib-tool: *** cannot find ./configure.ac
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to erobles on 9/17/2009 6:00 PM: hi! I have downloaded gnulib through git and cvs, and i tried to import getaddrinfo module, but i only got the message mentioned in the subject: $ gnulib-tool --import getaddrinfo gnulib-tool: *** cannot find ./configure.ac Are you executing inside your project's directory, where you already have a working configure.ac? It sounds like you are just in the wrong directory, since gnulib itself does not need or provide a configure.ac for itself. - -- 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/ iEYEARECAAYFAkqy2lkACgkQ84KuGfSFAYAcogCeN4PtfHMIK+X+2V2E5vLykQ5c T2EAn1vZqMDKTBzmNzKaEWOwQJ5G8GDu =9stm -END PGP SIGNATURE-
Re: readlink(1) behavior
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/18/2009 4:47 AM: Assuming ln -s /nowhere dangling-symlink, New behavior: $ ./readlink -fv dangling-symlink/ /nowhere Previous: $ readlink -fv dangling-symlink/ readlink: dangling-symlink/: No such file or directory [Exit 1] On one hand: ln -s dangling link = 0 stat dangling/ = ENOENT stat link/ = ENOENT mkdir link/ = 0 the behavior is consistent - a single mkdir can create the lone missing element in the (expanded) pathname. If we really claim that -f implies that a missing last element is acceptable if it can be created by the same name, then that explains the new behavior. On the other hand, continuing the above example, there are some Linux interfaces which distinguish between a dangling symlink with trailing slash compared to a missing file with trailing slash: rmdir link/ = ENOTDIR rmdir dangling/ rmdir dangling/ = ENOENT rmdir link/ = ENOTDIR This difference does not exist on Solaris (which follows the counter-intuitive POSIX rule that the first 'rmdir link/' remove dangling rather than fail with ENOTDIR). So, we need to decide whether I should work on changing the CAN_ALL_BUT_LAST mode to explicitly differentiate between a missing element with a trailing slash (ok) compared to an existing but dangling symlink with a trailing slash (fail, as before this change); or whether the behavior on a dangling symlink with a trailing slash is an intentional change for consistency. Once we make that decision, we either need to patch coreutils' NEWS or I need to fix the regression in gnulib canonicalize. - -- 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/ iEYEARECAAYFAkqzcGgACgkQ84KuGfSFAYCX/gCdFt1ja3jSK+lNjaxjGXsRRwqD Ly0AoKNQr+/DhFU9cTMPftrQxnlNmD3e =cQPt -END PGP SIGNATURE-
Re: canonicalize-lgpl bug
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/18/2009 1:40 AM: + mkdir subdir removed + touch regfile + ln -s regfile link1 + ln -s subdir link2 + ln -s missing link3 + ln -s subdir/missing link4 ... ++ readlink -f subdir/more + v=/sh/j/w/co/cu/tests/cu-can-f.Y2RpVABkn2/d/subdir/more + test /sh/j/w/co/cu/tests/cu-can-f.Y2RpVABkn2/d/subdir/more = /sh/j/w/co/cu/tests/cu-can-f.Y2RpVABkn2/d/subdir/more ++ readlink -f ./subdir/more/ + v=/sh/j/w/co/cu/tests/cu-can-f.Y2RpVABkn2/d/subdir/more + fail=1 Yep - back to the argument of whether we special case dangling symlinks with a trailing slash. Part of me argues that this behavior change is intentional, since: mkdir subdir/more/ will succeed. But whether we change the test depends on whether we decide that I need to fix this regression in gnulib. ++ readlink -f missing + v=/sh/j/w/co/cu/tests/cu-can-f.Y2RpVABkn2/d/missing + test /sh/j/w/co/cu/tests/cu-can-f.Y2RpVABkn2/d/missing = /sh/j/w/co/cu/tests/cu-can-f.Y2RpVABkn2/d/missing ++ readlink -f ./missing/ + v=/sh/j/w/co/cu/tests/cu-can-f.Y2RpVABkn2/d/missing + fail=1 Here, the test is just wrong; we really did intentionally change the semantics. It looks like all the other fail=1 listings in the log are variants of the above two scenarios (missing/ is an intentional change, and dangling/ is a questionable one). - -- 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/ iEYEARECAAYFAkqzcecACgkQ84KuGfSFAYCBRwCgku78Xmnjp0nol7v3kOPTrvW8 gYEAn3GXLcKkJQeZpp+szTGB5qyIgbh5 =rgUj -END PGP SIGNATURE-
Re: build failure on HP-UX 11.11
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 [adding gnulib] According to Greg Wooledge on 9/18/2009 9:52 AM: GNU m4 1.4.13 fails to build on HP-UX 11.11: make[4]: Entering directory `/usr/local/src/m4-1.4.13/lib' CC gl_avltree_oset.o In file included from sys/wait.h:91, from sys/wait.h:28, from stdlib.h:307, from stdlib.h:34, from gl_avltree_oset.c:25: sys/resource.h:106: error: field `ru_utime' has incomplete type sys/resource.h:107: error: field `ru_stime' has incomplete type Line 106 of that file is struct timeval ru_utime; plus a comment. Sounds like we're missing a header inclusion dependency. I worked around this by editing the generated ./lib/sys/time.h file. Before: # if 0 # include_next sys/time.h # endif After: # if 1 # include_next sys/time.h # endif After making that change, it was able to build, and passed the vast majority of the make check test suite. Hmm. That #if 0 line was generated from # if @have_sys_tim...@. Can you also attach config.log, and the make output, to see why HAVE_SYS_TIME_H might have been defined incorrectly? - -- 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/ iEYEARECAAYFAkqzs+0ACgkQ84KuGfSFAYA6BwCeIIn9oZQkcgH1H2+ft9GumcVQ G6sAoLpSkEp6gO2wwHbqab2Go6GXL1A8 =wMWL -END PGP SIGNATURE-
Re: ./gnulib-tool --create-testdir --dir=./gnulib_build
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Simon Josefsson on 9/19/2009 2:41 AM: Files: ... m4/canonicalize-lgpl.m4 Bruno, Ben, is this patch the right thing? I'm neither, but yes, the patch is right. Sorry for not realizing I broke that when I merged the m4 file of canonicalize/canonicalize-lgpl earlier this week. - -- 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/ iEYEARECAAYFAkq0vaAACgkQ84KuGfSFAYDx0ACfTDIp/vD7fkiQcpiSHnZagPb8 oooAoLcfdfp+YltWBZf5QrdurWtiJKXg =bY/F -END PGP SIGNATURE-
Re: build failure on HP-UX 11.11
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 9/19/2009 7:16 AM: I agree with you that it was a problem with that particular gcc, because - the build works fine with the CC=cc -Ae -D_XOPEN_SOURCE=500 setting mentioned in the INSTALL file, - as you found out, gcc's private wchar.h uses va_list before including stdarg.h. Can/should we work around this gcc bug, by ensuring that stdarg.h is always included prior to wchar.h in all configure tests? - -- 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/ iEUEARECAAYFAkq02nkACgkQ84KuGfSFAYDjlwCgpLRYqwW1PMb5TmCDGutpkzXO H3AAl2rFhz4TKDy2Bh7ZI6+7DeB4wFA= =8l5x -END PGP SIGNATURE-
Re: need opendir_safer, dirent--.h
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 9/2/2009 1:00 PM: And here's openat-safer, plus a rework of making fts safer. Subject: [PATCH 2/2] backupfile, chdir-long, fts, savedir: make safer * lib/backupfile.c (includes): Use dirent--.h, since numbered_backup can write to stderr during readdir. * lib/savedir.c (includes): Likewise. * lib/chdir-long.c (includes): Use fcntl--.h, since openat emulation can write to stderr on failure. Since openat requires chdir-long, and chdir-long requires openat-safer, but openat-safer implies that rpl_openat uses open_safer, there is _no way_ to use the openat replacement to get fd 0. This seems a bit restrictive; I think that openat-safer should be a conscious choice, not dragged in by default for just openat (however, I do admit that the use of openat without *-safer will probably be rare). It is also inconsistent - platforms with native openat behave differently than those with the replacement, and the difference can mask subtle bugs with stdin. To fix it, I'm committing this. Note that the only reason that chdir-long worried about openat-safer was that if the openat emulation could succeeds in opening fd 2, but then fail to restore the current working directory, it would then clobber fd 2 during openat_restore_fail. So, fix this obscure corner case by closing a just-created fd 2 before dying. Meanwhile, save_cwd _needs_ to be safe, but can do so manually via dup_safer rather than having a blind dependency on fcntl-safer. I've tested that, with this patch, './gnulib-tool --with-tests --test openat' no longer drags in fcntl-safer, but './gnulib-tool --with-tests - --test openat-safer' still passes the new test-openat. There's also a followup to make the use of at-func.c easier (client files should not need to know which include files were needed by at-func.c), for some upcoming patches for Solaris 9 fstatat and unlinkat. - -- 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/ iEYEARECAAYFAkq05+AACgkQ84KuGfSFAYC3UwCfQJ9Lmd1Oi9Z55GkaUD44AsYa lZ8AoI8EU/UqJyQErkuRi3ofY0NxcdBv =stXI -END PGP SIGNATURE- From 39ed9bd6ad79e753116be0242400d4586c914bf7 Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Sat, 19 Sep 2009 07:12:15 -0600 Subject: [PATCH 1/2] openat: allow return of fd 0 Partially reverts patch fc33350 from 2009-09-02. * modules/chdir-long (Depends-on): Relax openat-safer to openat. * modules/save-cwd (Depends-on): Replace fcntl-safer with unistd-safer. * lib/chdir-long.c (includes): Replace fcntl--.h with fcntl.h; this module does not leak fds. * lib/openat.c (includes): Do not use fcntl_safer; plain openat must be allowed to return 0, leaving openat_safer to add the safety. (openat_permissive): Avoid writing to just-opened fd 2 if restoring the current directory fails. * lib/openat-die.c (openat_restore_fail): Add comment. * lib/save-cwd.c (includes): Make fcntl--.h conditional. (save_cwd): Guarantee safe fd, but without use of open_safer. * tests/test-openat.c: New test. * modules/openat-tests (Files, Makefile.am): Distribute and build new file. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog| 18 +++ lib/chdir-long.c | 12 ++--- lib/openat-die.c |5 lib/openat.c | 15 +-- lib/save-cwd.c | 14 +-- modules/chdir-long |2 +- modules/openat-tests |6 +++- modules/save-cwd |4 +- tests/test-openat.c | 60 ++ 9 files changed, 116 insertions(+), 20 deletions(-) create mode 100644 tests/test-openat.c diff --git a/ChangeLog b/ChangeLog index a514b34..3e8e24b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,23 @@ 2009-09-19 Eric Blake e...@byu.net + openat: allow return of fd 0 + * modules/chdir-long (Depends-on): Relax openat-safer to openat. + * modules/save-cwd (Depends-on): Replace fcntl-safer with + unistd-safer. + * lib/chdir-long.c (includes): Replace fcntl--.h with + fcntl.h; this module does not leak fds. + * lib/openat.c (includes): Do not use fcntl_safer; plain openat + must be allowed to return 0, leaving openat_safer to add the + safety. + (openat_permissive): Avoid writing to just-opened fd 2 if + restoring the current directory fails. + * lib/openat-die.c (openat_restore_fail): Add comment. + * lib/save-cwd.c (includes): Make fcntl--.h conditional. + (save_cwd): Guarantee safe fd, but without use of open_safer. + * tests/test-openat.c: New test. + * modules/openat-tests (Files, Makefile.am): Distribute and build + new file. + relocatable-prog-wrapper: fix build * modules/relocatable-prog
Re: need opendir_safer, dirent--.h
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 9/19/2009 8:17 AM: I've tested that, with this patch, './gnulib-tool --with-tests --test openat' no longer drags in fcntl-safer, but './gnulib-tool --with-tests --test openat-safer' still passes the new test-openat. Except on Solaris 9, where openat exists but mishandles trailing slash; this failure is exposed by test-openat-safer. I've added the creation of rpl_openat onto my TODO list for fixing *at on Solaris 9. - -- 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/ iEYEARECAAYFAkq06SQACgkQ84KuGfSFAYBzuQCgxHz++zVAwDkDkSQpIyqPGjb8 5KMAoLdcePZUNyZX+MNiwYMZvf6xcN66 =KKC6 -END PGP SIGNATURE-
Re: [PATCH] posixtm: don't reject a time with 60 as the number of seconds
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/19/2009 10:46 AM: +++ b/tests/test-posixtm.c @@ -0,0 +1,175 @@ +/* Test that openat_safer leave standard fds alone. Really? This line is a bit too much copy-n-paste ;) - -- 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/ iEYEARECAAYFAkq1C98ACgkQ84KuGfSFAYAPyACffCZDHYd93Q9/q3ssFphTwozx JDwAoLn1vbL6xGrXjL/eHUeFaqJXR6p8 =sHs4 -END PGP SIGNATURE-
