Re: I: coreutils-20041123: src/touch.c regression
Jim Meyering [EMAIL PROTECTED] writes: The only other question is whether EROFS is always defined. This code now contains the first use of that symbol in coreutils. I checked a few packages and found that the uses in binutils and cvs are both protected with #if directives. But rsync and openssh use it unconditionally, so it's probably ok, these days. Yes, I think so. EROFS was in Unix Version 7, so the only problem would be with (non-POSIX) systems that omitted it for some reason, which I think is unlikely for realistic coreutils targets. Binutils is a bit special, since it wants to (or, at least, wanted to) be portable to the old Mac MPW, which lacks all sorts of errno values that coreutils assumes (EACCES and ENOENT, for example). I wouldn't worry about it much as a precedent here. CVS 1.12.11 uses EROFS unconditionally; perhaps you were looking at an older version of CVS? Other than binutils, the uses of #ifdef EROFS that I've seen seem to stem from paranoia rather than real need. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: I: coreutils-20041123: src/touch.c regression
Dmitry V. Levin [EMAIL PROTECTED] writes:
Your change (at 2004-11-23) to src/touch.c introduces regression:
On GNU/Linux without futimes syscall and without /proc mounted, futimes()
function from glibc returns ENOENT, futimens() from gnulib also returns
ENOENT, and touch utility fails.
I suggest to modify gnulib's futimens() to fall back to utimes()
when futimes() fails this way.
Thanks for reporting that; I installed this patch. Could you please
give it a try? My GNU/Linux distribution doesn't have this problem.
Thanks.
2005-01-03 Paul Eggert [EMAIL PROTECTED]
* utimens.c (futimens) [HAVE_FUTIMES]: Fall back on utimes if
futimes fails with errno == ENOENT. Problem reported by
Dmitry V. Levin.
--- utimens.c 23 Nov 2004 20:42:13 - 1.3
+++ utimens.c 3 Jan 2005 08:51:59 - 1.4
@@ -1,4 +1,4 @@
-/* Copyright (C) 2003, 2004 Free Software Foundation, Inc.
+/* Copyright (C) 2003, 2004, 2005 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
@@ -24,6 +24,8 @@
#include utimens.h
+#include errno.h
+
#if HAVE_UTIME_H
# include utime.h
#endif
@@ -74,7 +76,16 @@ futimens (int fd ATTRIBUTE_UNUSED,
t = NULL;
# if HAVE_FUTIMES
if (0 = fd)
-return futimes (fd, t);
+{
+ if (futimes (fd, t) == 0)
+ return 0;
+
+ /* On GNU/Linux without the futimes syscall and without /proc
+mounted, glibc futimes fails with errno == ENOENT. Fall back
+on utimes in this case. */
+ if (errno != ENOENT)
+ return -1;
+}
# endif
return utimes (file, t);
___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: I: coreutils-20041123: src/touch.c regression
Hi, On Mon, Jan 03, 2005 at 12:53:58AM -0800, Paul Eggert wrote: Your change (at 2004-11-23) to src/touch.c introduces regression: On GNU/Linux without futimes syscall and without /proc mounted, futimes() function from glibc returns ENOENT, futimens() from gnulib also returns ENOENT, and touch utility fails. I suggest to modify gnulib's futimens() to fall back to utimes() when futimes() fails this way. Thanks for reporting that; I installed this patch. Could you please give it a try? My GNU/Linux distribution doesn't have this problem. Yes, this patch surely fixes the problem: open(f, O_WRONLY|O_NONBLOCK|O_CREAT|O_NOCTTY|O_LARGEFILE, 0666) = 3 utimes(/proc/self/fd/3, NULL) = -1 ENOSYS (Function not implemented) utime(/proc/self/fd/3, NULL) = -1 ENOENT (No such file or directory) utimes(f, NULL) = -1 ENOSYS (Function not implemented) utime(f, NULL)= 0 close(3)= 0 According to documentation, futimes() may also fail with ENOSYS; in this case, falling back on utimes() also looks reasonable: + if (errno != ENOENT) + if (errno != ENOSYS errno != ENOENT) -- ldv pgpZVUr2WHom7.pgp Description: PGP signature ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: I: coreutils-20041123: src/touch.c regression
Dmitry V. Levin [EMAIL PROTECTED] wrote: ... According to documentation, futimes() may also fail with ENOSYS; in this case, falling back on utimes() also looks reasonable: + if (errno != ENOENT) + if (errno != ENOSYS errno != ENOENT) Thanks. I've made that change. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: I: coreutils-20041123: src/touch.c regression
Jim Meyering [EMAIL PROTECTED] writes:
Dmitry V. Levin [EMAIL PROTECTED] wrote:
+ if (errno != ENOSYS errno != ENOENT)
Thanks.
I've made that change.
On further thought, this looks like a classic example where we should
be testing for known valid error numbers rather than adding bogus
error numbers as reports come in from the field. So I installed this:
2005-01-03 Paul Eggert [EMAIL PROTECTED]
* utimens.c (futimens): Robustify the previous patch, by checking
for known valid error numbers rather than observed invalid ones.
--- utimens.c 3 Jan 2005 19:23:09 - 1.5
+++ utimens.c 3 Jan 2005 19:54:54 - 1.6
@@ -81,10 +81,16 @@ futimens (int fd ATTRIBUTE_UNUSED,
return 0;
/* On GNU/Linux without the futimes syscall and without /proc
-mounted, glibc futimes fails with errno == ENOENT or ENOSYS.
-Fall back on utimes in this case. */
- if (errno != ENOENT errno != ENOSYS)
- return -1;
+mounted, glibc futimes fails with errno == ENOENT. Fall back
+on utimes if we get a weird error number like that. */
+ switch (errno)
+ {
+ case EACCES:
+ case EIO:
+ case EPERM:
+ case EROFS:
+ return -1;
+ }
}
# endif
return utimes (file, t);
___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: I: coreutils-20041123: src/touch.c regression
Paul Eggert [EMAIL PROTECTED] wrote:
Jim Meyering [EMAIL PROTECTED] writes:
Dmitry V. Levin [EMAIL PROTECTED] wrote:
+ if (errno != ENOSYS errno != ENOENT)
Thanks.
I've made that change.
On further thought, this looks like a classic example where we should
be testing for known valid error numbers rather than adding bogus
error numbers as reports come in from the field. So I installed this:
Either way is fine with me, as long as we get it right :-)
I have a slight preference for the approach that uses the smaller,
set of errno values, on the assumption that it's easier to
verify completeness. Also, I suspect it is more likely that the
larger set will grow, in this case.
Your switch statement should probably handle EBADF, too,
to avoid an unnecessary utimes call after such a failure.
2005-01-03 Paul Eggert [EMAIL PROTECTED]
* utimens.c (futimens): Robustify the previous patch, by checking
for known valid error numbers rather than observed invalid ones.
...
+ switch (errno)
+ {
+ case EACCES:
+ case EIO:
+ case EPERM:
+ case EROFS:
+ return -1;
+ }
}
# endif
return utimes (file, t);
___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: I: coreutils-20041123: src/touch.c regression
Jim Meyering [EMAIL PROTECTED] writes: Your switch statement should probably handle EBADF, too, to avoid an unnecessary utimes call after such a failure. EBADF shouldn't happen as it violates the futimens documentation, which says FD must be either negative -- in which case it is ignored -- or a file descriptor that is open on FILE., and the code in question is reachable only if FD is nonnegative. I omitted a test for EBADF because I figured that there's no point optimizing cases that shouldn't happen. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
