bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] utimens: remove invalid futimesat call


From: Eric Blake
Subject: Re: [PATCH] utimens: remove invalid futimesat call
Date: Wed, 18 Nov 2009 07:13:40 -0700
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.23) Gecko/20090812 Thunderbird/2.0.0.23 Mnenhy/0.7.6.666

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Jim Meyering on 11/8/2009 7:21 AM:
> This fixes coreutils' final FreeBSD-8.0-rc2 test failure:

And breaks the Solaris 10 futimens workaround.  Since Solaris lacks
futimens and futimes, the only way to change fd timestamps is via
futimesat with a NULL argument.

> Subject: [PATCH] utimens: remove invalid futimesat call
> 
> * lib/utimens.c (fdutimens) [HAVE_FUTIMESAT]: Remove invalid futimesat
> call.  It used the file descriptor of the target file as the DIR_FD
> parameter and NULL as the file name.  That caused failure with
> errno == EFAULT on FreeBSD-8.0-rc2.

That just means that FreeBSD goofed in trying to copy Solaris semantics
when they implemented futimesat (and why didn't they go all the way and
implement the standardized futimens?).  By the way, cygwin 1.7's original
implementation of futimesat also got it wrong, but it was fixed shortly
after to copy the special-casing semantics of Solaris.  Are you in a
position to report this to the FreeBSD developers?  Here's what I'm
pushing; tested on Solaris 10, but I don't have access to FreeBSD 8.0-rc2
to test there.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----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/

iEYEARECAAYFAksEARQACgkQ84KuGfSFAYDb/ACgxBUpBBihmfltKG/BcofplIFn
d8IAn2vz4tXlnHEoAlHuqukoS0At31+l
=NVfX
-----END PGP SIGNATURE-----
>From 0883405cc751858a633bebc56acb75381a6e50c8 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 18 Nov 2009 06:59:44 -0700
Subject: [PATCH] utimens: fix regression on Solaris

Revert commit 26c5fd742f.  Solaris lacks futimens and futimes, so
futimesat is the only way to change fd timestamps.  But since
FreeBSD futimesat can't change fd timestamps, we need a configure
check to avoid the crash there.

* m4/utimens.m4 (gl_UTIMENS): Check for BSD bug.
* lib/utimens.c (fdutimens): Revert 2009-11-08 change; Solaris 10
can only change fd timestamps via futimesat.  Instead, use an
additional witness macro to avoid BSD bug.
Reported by Jim Meyering.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog     |    9 +++++++++
 lib/utimens.c |   14 +++++++++-----
 m4/utimens.m4 |   26 +++++++++++++++++++++++++-
 3 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 62b5423..649834b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2009-11-18  Eric Blake  <address@hidden>
+
+       utimens: fix regression on Solaris
+       * m4/utimens.m4 (gl_UTIMENS): Check for BSD bug.
+       * lib/utimens.c (fdutimens): Revert 2009-11-08 change; Solaris 10
+       can only change fd timestamps via futimesat.  Instead, use an
+       additional witness macro to avoid BSD bug.
+       Reported by Jim Meyering.
+
 2009-11-17  Eric Blake  <address@hidden>

        usleep: use it to simplify tests
diff --git a/lib/utimens.c b/lib/utimens.c
index bd482d7..eb63487 100644
--- a/lib/utimens.c
+++ b/lib/utimens.c
@@ -280,9 +280,9 @@ fdutimens (char const *file, int fd, struct timespec const 
timespec[2])
       }
     else
       {
-        /* If futimesat (above) or futimes fails here, don't try to speed
-           things up by returning right away.  glibc can incorrectly fail
-           with errno == ENOENT if /proc isn't mounted.  Also, Mandrake 10.0
+        /* If futimesat or futimes fails here, don't try to speed things
+           up by returning right away.  glibc can incorrectly fail with
+           errno == ENOENT if /proc isn't mounted.  Also, Mandrake 10.0
            in high security mode doesn't allow ordinary users to read
            /proc/self, so glibc incorrectly fails with errno == EACCES.
            If errno == EIO, EPERM, or EROFS, it's probably safe to fail
@@ -290,7 +290,10 @@ fdutimens (char const *file, int fd, struct timespec const 
timespec[2])
            worth optimizing, and who knows what other messed-up systems
            are out there?  So play it safe and fall back on the code
            below.  */
-# if HAVE_FUTIMES
+# if HAVE_FUTIMESAT && !FUTIMESAT_NULL_BUG
+        if (futimesat (fd, NULL, t) == 0)
+          return 0;
+# elif HAVE_FUTIMES
         if (futimes (fd, t) == 0)
           return 0;
 # endif
@@ -299,7 +302,8 @@ fdutimens (char const *file, int fd, struct timespec const 
timespec[2])

     if (!file)
       {
-#if ! (HAVE_FUTIMESAT || (HAVE_WORKING_UTIMES && HAVE_FUTIMES))
+#if ! ((HAVE_FUTIMESAT && !FUTIMESAT_NULL_BUG)          \
+        || (HAVE_WORKING_UTIMES && HAVE_FUTIMES))
         errno = ENOSYS;
 #endif
         return -1;
diff --git a/m4/utimens.m4 b/m4/utimens.m4
index 381d087..35ef949 100644
--- a/m4/utimens.m4
+++ b/m4/utimens.m4
@@ -4,7 +4,7 @@ dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
 dnl with or without modifications, as long as this notice is preserved.

-dnl serial 3
+dnl serial 4

 AC_DEFUN([gl_UTIMENS],
 [
@@ -15,4 +15,28 @@ AC_DEFUN([gl_UTIMENS],
   AC_REQUIRE([gl_CHECK_TYPE_STRUCT_TIMESPEC])
   AC_REQUIRE([gl_CHECK_TYPE_STRUCT_UTIMBUF])
   AC_CHECK_FUNCS_ONCE([futimes futimesat futimens utimensat lutimes])
+
+  if test $ac_cv_func_futimens = no && test $ac_cv_func_futimesat = yes; then
+    dnl FreeBSD 8.0-rc2 mishandles futimesat(fd,NULL,time).  It is not
+    dnl standardized, but Solaris implemented it first and uses it as
+    dnl its only means to set fd time.
+    AC_CACHE_CHECK([whether futimesat handles NULL file],
+      [gl_cv_func_futimesat_works],
+      [touch conftest.file
+       AC_RUN_IFELSE([AC_LANG_PROGRAM([[
+#include <stddef.h>
+#include <sys/times.h>
+]], [[    int fd = open ("conftest.file", O_RDWR);
+          if (fd < 0) return 1;
+          if (futimesat (fd, NULL, NULL)) return 2;
+        ]])],
+        [gl_cv_func_futimesat_works=yes],
+        [gl_cv_func_futimesat_works=no],
+        [gl_cv_func_futimesat_works="guessing no"])
+      rm -f conftest.file])
+    if test "$gl_cv_func_futimesat_works" != yes; then
+      AC_DEFINE([FUTIMESAT_NULL_BUG], [1],
+        [Define to 1 if futimesat mishandles a NULL file name.])
+    fi
+  fi
 ])
-- 
1.6.5.rc1


reply via email to

[Prev in Thread] Current Thread [Next in Thread]