bug-coreutils
[Top][All Lists]
Advanced

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

Re: coreutils-8.2 misc/ls-time test failure


From: Eric Blake
Subject: Re: coreutils-8.2 misc/ls-time test failure
Date: Fri, 18 Dec 2009 00:16:50 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

Jim Meyering <jim <at> meyering.net> writes:

> > Sounds quite hairy.  Any ideas for improvements?
> 
> Thanks for investigating and scoping out the solution.
> I agree that it sounds hairy, but it also sounds like the required approach.

So, here we go in three steps; maybe they are worth squashing into one when I 
finally apply (I'm still running tests on more platforms, first, in case any 
other gotchas pop up).  I also still need to report this to lkml.

Eric Blake (3):
      [1/3] utimens: check for ctime update
Expose the bug.  The testsuite additions should fail miserably on any Linux 
kernel newer than 2.6.18, but pass on cygwin 1.7 (native utimensat), as well as 
on most other platforms that lack utimensat (older kernels, cygwin 1.5, 
BSD, ...).  mingw doesn't really have the bug, but that is because they disobey 
POSIX by using st_ctime to track the birthtime (on NTFS) or modification time 
(on FAT), rather than tracking a true change time, but I did ensure the 
testsuite additions pass there, too.

      [2/3] utimens: detect Linux bug
Avoid the bug in our all-in-one wrapper.  Pretty much the hairy code I proposed.

      [3/3] futimens, utimensat: detect Linux bug
Repeat the fix for the actual syscall replacements, and beef up the m4 tests.


>From 2f9e401067786829993acfb62c23a047fde553cd Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 17 Dec 2009 12:30:47 -0700
Subject: [PATCH 1/3] utimens: check for ctime update

futimens/utimensat on Linux bumps ctime if both times are
given, but mistakenly left it alone if UTIME_OMIT was used.

* tests/test-utimens-common.h (check_ctime): Define.
* tests/test-utimens.h (test_utimens): Expose the Linux bug.
* tests/test-futimens.h (test_futimens): Likewise.
* tests/test-lutimens.h (test_lutimens): Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                   |    8 +++++++
 tests/test-futimens.h       |   33 +++++++++++++++++++++++++++---
 tests/test-lutimens.h       |   46 ++++++++++++++++++++++++++++++++++++------
 tests/test-utimens-common.h |   27 ++++++++++++++++--------
 tests/test-utimens.h        |   35 ++++++++++++++++++++++++++++----
 5 files changed, 124 insertions(+), 25 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3291499..ebcd33d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2009-12-17  Eric Blake  <address@hidden>
+
+       utimens: check for ctime update
+       * tests/test-utimens-common.h (check_ctime): Define.
+       * tests/test-utimens.h (test_utimens): Expose the Linux bug.
+       * tests/test-futimens.h (test_futimens): Likewise.
+       * tests/test-lutimens.h (test_lutimens): Likewise.
+
 2009-12-16  Eric Blake  <address@hidden>

        fcntl: use to simplify other modules
diff --git a/tests/test-futimens.h b/tests/test-futimens.h
index 7c05bbf..e39c8f7 100644
--- a/tests/test-futimens.h
+++ b/tests/test-futimens.h
@@ -53,6 +53,10 @@ test_futimens (int (*func) (int, struct timespec const *),
      UTIMECMP_TRUNCATE_SOURCE to compensate, with st1 as the
      source.  */
   ASSERT (0 <= utimecmp (BASE "file", &st2, &st1, UTIMECMP_TRUNCATE_SOURCE));
+  if (check_ctime)
+    ASSERT (st1.st_ctime < st2.st_ctime
+            || (st1.st_ctime == st2.st_ctime
+                && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));
   {
     /* On some NFS systems, the 'now' timestamp of creat or a NULL
        timespec is determined by the server, but the 'now' timestamp
@@ -101,17 +105,38 @@ test_futimens (int (*func) (int, struct timespec const *),
     ASSERT (st2.st_mtime == Y2K);
     ASSERT (0 <= get_stat_mtime_ns (&st2));
     ASSERT (get_stat_mtime_ns (&st2) < BILLION);
+    if (check_ctime)
+      ASSERT (st1.st_ctime < st2.st_ctime
+              || (st1.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));
   }

   /* Play with UTIME_OMIT, UTIME_NOW.  */
   {
+    struct stat st3;
     struct timespec ts[2] = { { BILLION, UTIME_OMIT }, { 0, UTIME_NOW } };
+    nap ();
+    ASSERT (func (fd, ts) == 0);
+    ASSERT (fstat (fd, &st3) == 0);
+    ASSERT (st3.st_atime == Y2K);
+    ASSERT (0 <= get_stat_atime_ns (&st3));
+    ASSERT (get_stat_atime_ns (&st3) <= BILLION / 2);
+    ASSERT (utimecmp (BASE "file", &st1, &st3, 0) <= 0);
+    if (check_ctime)
+      ASSERT (st2.st_ctime < st3.st_ctime
+              || (st2.st_ctime == st3.st_ctime
+                  && get_stat_ctime_ns (&st2) < get_stat_ctime_ns (&st3)));
+    nap ();
+    ts[0].tv_nsec = 0;
     ASSERT (func (fd, ts) == 0);
     ASSERT (fstat (fd, &st2) == 0);
-    ASSERT (st2.st_atime == Y2K);
-    ASSERT (0 <= get_stat_atime_ns (&st2));
-    ASSERT (get_stat_atime_ns (&st2) <= BILLION / 2);
-    ASSERT (utimecmp (BASE "file", &st1, &st2, 0) <= 0);
+    ASSERT (st2.st_atime == BILLION);
+    ASSERT (get_stat_atime_ns (&st2) == 0);
+    ASSERT (0 <= utimecmp (BASE "file", &st2, &st3, UTIMECMP_TRUNCATE_SOURCE));
+    if (check_ctime)
+      ASSERT (st3.st_ctime < st2.st_ctime
+              || (st3.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st3) < get_stat_ctime_ns (&st2)));
   }

   /* Cleanup.  */
diff --git a/tests/test-lutimens.h b/tests/test-lutimens.h
index c9302c8..ef28af3 100644
--- a/tests/test-lutimens.h
+++ b/tests/test-lutimens.h
@@ -54,11 +54,16 @@ test_lutimens (int (*func) (char const *, struct timespec 
const *), bool print)
   }
   {
     struct timespec ts[2] = { { Y2K, 0 }, { Y2K, 0 } };
+    nap ();
     ASSERT (func (BASE "file", ts) == 0);
   }
-  ASSERT (stat (BASE "file", &st1) == 0);
-  ASSERT (st1.st_atime == Y2K);
-  ASSERT (st1.st_mtime == Y2K);
+  ASSERT (stat (BASE "file", &st2) == 0);
+  ASSERT (st2.st_atime == Y2K);
+  ASSERT (st2.st_mtime == Y2K);
+  if (check_ctime)
+    ASSERT (st1.st_ctime < st2.st_ctime
+            || (st1.st_ctime == st2.st_ctime
+                && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));

   /* Play with symlink timestamps.  */
   if (symlink (BASE "file", BASE "link"))
@@ -98,6 +103,8 @@ test_lutimens (int (*func) (char const *, struct timespec 
const *), bool print)
   if (st1.st_atime != st2.st_atime
       || get_stat_atime_ns (&st1) != get_stat_atime_ns (&st2))
     atime_supported = false;
+  ASSERT (st1.st_ctime == st2.st_ctime);
+  ASSERT (get_stat_ctime_ns (&st1) == get_stat_ctime_ns (&st2));

   /* Invalid arguments.  */
   {
@@ -123,6 +130,7 @@ test_lutimens (int (*func) (char const *, struct timespec 
const *), bool print)
   /* Set both times.  */
   {
     struct timespec ts[2] = { { Y2K, BILLION / 2 - 1 }, { Y2K, BILLION - 1 } };
+    nap ();
     ASSERT (func (BASE "link", ts) == 0);
     ASSERT (lstat (BASE "link", &st2) == 0);
     if (atime_supported)
@@ -134,20 +142,44 @@ test_lutimens (int (*func) (char const *, struct timespec 
const *), bool print)
     ASSERT (st2.st_mtime == Y2K);
     ASSERT (0 <= get_stat_mtime_ns (&st2));
     ASSERT (get_stat_mtime_ns (&st2) < BILLION);
+    if (check_ctime)
+      ASSERT (st1.st_ctime < st2.st_ctime
+              || (st1.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));
   }

   /* Play with UTIME_OMIT, UTIME_NOW.  */
   {
+    struct stat st3;
     struct timespec ts[2] = { { BILLION, UTIME_OMIT }, { 0, UTIME_NOW } };
+    nap ();
+    ASSERT (func (BASE "link", ts) == 0);
+    ASSERT (lstat (BASE "link", &st3) == 0);
+    if (atime_supported)
+      {
+        ASSERT (st3.st_atime == Y2K);
+        ASSERT (0 <= get_stat_atime_ns (&st3));
+        ASSERT (get_stat_atime_ns (&st3) < BILLION / 2);
+      }
+    ASSERT (utimecmp (BASE "link", &st1, &st3, 0) <= 0);
+    if (check_ctime)
+      ASSERT (st2.st_ctime < st3.st_ctime
+              || (st2.st_ctime == st3.st_ctime
+                  && get_stat_ctime_ns (&st2) < get_stat_ctime_ns (&st3)));
+    nap ();
+    ts[0].tv_nsec = 0;
     ASSERT (func (BASE "link", ts) == 0);
     ASSERT (lstat (BASE "link", &st2) == 0);
     if (atime_supported)
       {
-        ASSERT (st2.st_atime == Y2K);
-        ASSERT (0 <= get_stat_atime_ns (&st2));
-        ASSERT (get_stat_atime_ns (&st2) < BILLION / 2);
+        ASSERT (st2.st_atime == BILLION);
+        ASSERT (get_stat_atime_ns (&st2) == 0);
       }
-    ASSERT (utimecmp (BASE "link", &st1, &st2, 0) <= 0);
+    ASSERT (0 <= utimecmp (BASE "link", &st2, &st3, UTIMECMP_TRUNCATE_SOURCE));
+    if (check_ctime)
+      ASSERT (st3.st_ctime < st2.st_ctime
+              || (st3.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st3) < get_stat_ctime_ns (&st2)));
   }

   /* Symlink to directory.  */
diff --git a/tests/test-utimens-common.h b/tests/test-utimens-common.h
index 6c404cc..707971a 100644
--- a/tests/test-utimens-common.h
+++ b/tests/test-utimens-common.h
@@ -19,14 +19,14 @@
 #ifndef GL_TEST_UTIMENS_COMMON
 # define GL_TEST_UTIMENS_COMMON

-#include <fcntl.h>
-#include <errno.h>
-#include <string.h>
-#include <unistd.h>
+# include <fcntl.h>
+# include <errno.h>
+# include <string.h>
+# include <unistd.h>

-#include "stat-time.h"
-#include "timespec.h"
-#include "utimecmp.h"
+# include "stat-time.h"
+# include "timespec.h"
+# include "utimecmp.h"

 enum {
   BILLION = 1000 * 1000 * 1000,
@@ -62,9 +62,18 @@ nap (void)
      a quantization boundary equal to the resolution.  Our usage of
      utimecmp allows equality, so no need to waste 980 milliseconds
      if the replacement usleep rounds to 1 second.  */
-#if HAVE_USLEEP
+# if HAVE_USLEEP
   usleep (20 * 1000); /* 20 milliseconds.  */
-#endif
+# endif
 }

+# if (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__
+/* Skip ctime tests on native Windows, since it is either a copy of
+   mtime or birth time (depending on the file system), rather than a
+   properly tracked change time.  */
+#  define check_ctime 0
+# else
+#  define check_ctime 1
+# endif
+
 #endif /* GL_TEST_UTIMENS_COMMON */
diff --git a/tests/test-utimens.h b/tests/test-utimens.h
index 710741a..472ff57 100644
--- a/tests/test-utimens.h
+++ b/tests/test-utimens.h
@@ -37,6 +37,10 @@ test_utimens (int (*func) (char const *, struct timespec 
const *), bool print)
   ASSERT (func (BASE "file", NULL) == 0);
   ASSERT (stat (BASE "file", &st2) == 0);
   ASSERT (0 <= utimecmp (BASE "file", &st2, &st1, UTIMECMP_TRUNCATE_SOURCE));
+  if (check_ctime)
+    ASSERT (st1.st_ctime < st2.st_ctime
+            || (st1.st_ctime == st2.st_ctime
+                && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));
   {
     /* On some NFS systems, the 'now' timestamp of creat or a NULL
        timespec is determined by the server, but the 'now' timestamp
@@ -97,18 +101,39 @@ test_utimens (int (*func) (char const *, struct timespec 
const *), bool print)
     ASSERT (st2.st_mtime == Y2K);
     ASSERT (0 <= get_stat_mtime_ns (&st2));
     ASSERT (get_stat_mtime_ns (&st2) < BILLION);
+    if (check_ctime)
+      ASSERT (st1.st_ctime < st2.st_ctime
+              || (st1.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));
   }

   /* Play with UTIME_OMIT, UTIME_NOW.  */
   {
+    struct stat st3;
     struct timespec ts[2] = { { BILLION, UTIME_OMIT }, { 0, UTIME_NOW } };
+    nap ();
     ASSERT (func (BASE "file", ts) == 0);
-    ASSERT (stat (BASE "file", &st2) == 0);
-    ASSERT (st2.st_atime == Y2K);
-    ASSERT (0 <= get_stat_atime_ns (&st2));
-    ASSERT (get_stat_atime_ns (&st2) < BILLION / 2);
+    ASSERT (stat (BASE "file", &st3) == 0);
+    ASSERT (st3.st_atime == Y2K);
+    ASSERT (0 <= get_stat_atime_ns (&st3));
+    ASSERT (get_stat_atime_ns (&st3) < BILLION / 2);
     /* See comment above about this utimecmp call.  */
-    ASSERT (0 <= utimecmp (BASE "file", &st2, &st1, UTIMECMP_TRUNCATE_SOURCE));
+    ASSERT (0 <= utimecmp (BASE "file", &st3, &st1, UTIMECMP_TRUNCATE_SOURCE));
+    if (check_ctime)
+      ASSERT (st2.st_ctime < st3.st_ctime
+              || (st2.st_ctime == st3.st_ctime
+                  && get_stat_ctime_ns (&st2) < get_stat_ctime_ns (&st3)));
+    nap ();
+    ts[0].tv_nsec = 0;
+    ASSERT (func (BASE "file", ts) == 0);
+    ASSERT (stat (BASE "file", &st2) == 0);
+    ASSERT (st2.st_atime == BILLION);
+    ASSERT (get_stat_atime_ns (&st2) == 0);
+    ASSERT (0 <= utimecmp (BASE "file", &st2, &st3, UTIMECMP_TRUNCATE_SOURCE));
+    if (check_ctime)
+      ASSERT (st3.st_ctime < st2.st_ctime
+              || (st3.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st3) < get_stat_ctime_ns (&st2)));
   }

   /* Make sure this dereferences symlinks.  */
-- 
1.6.4.2


>From 61531bdee16ad6dc6ca385eeed0e4381d255f31c Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 17 Dec 2009 16:57:37 -0700
Subject: [PATCH 2/3] utimens: detect Linux bug

utimensat with UTIME_OMIT fails to bump ctime.  Work around it,
and use a cache to minimize syscalls once presence or absence
of bug can be ascertained.

* lib/utimens.c (detect_ctime_bug): New helper function.
(fdutimens, lutimens): Use it to work around bug.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog     |    4 ++
 lib/utimens.c |  128 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 120 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index ebcd33d..e26282f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2009-12-17  Eric Blake  <address@hidden>

+       utimens: detect Linux bug
+       * lib/utimens.c (detect_ctime_bug): New helper function.
+       (fdutimens, lutimens): Use it to work around bug.
+
        utimens: check for ctime update
        * tests/test-utimens-common.h (check_ctime): Define.
        * tests/test-utimens.h (test_utimens): Expose the Linux bug.
diff --git a/lib/utimens.c b/lib/utimens.c
index d8e2f32..a28c664 100644
--- a/lib/utimens.c
+++ b/lib/utimens.c
@@ -53,21 +53,65 @@ struct utimbuf
 #undef futimens
 #undef utimensat

-#if HAVE_UTIMENSAT || HAVE_FUTIMENS
-/* Cache variables for whether the utimensat syscall works; used to
-   avoid calling the syscall if we know it will just fail with ENOSYS.
-   There are some Linux kernel versions where a flag of 0 passes, but
-   not AT_SYMLINK_NOFOLLOW.  0 = unknown, 1 = yes, -1 = no.  */
-static int utimensat_works_really;
-static int lutimensat_works_really;
-#endif /* HAVE_UTIMENSAT || HAVE_UTIMENSAT */
-
 /* Solaris 9 mistakenly succeeds when given a non-directory with a
    trailing slash.  Force the use of rpl_stat for a fix.  */
 #ifndef REPLACE_FUNC_STAT_FILE
 # define REPLACE_FUNC_STAT_FILE 0
 #endif

+#if HAVE_UTIMENSAT || HAVE_FUTIMENS
+/* Cache variables for whether the utimensat syscall works; used to
+   avoid calling the syscall if we know it will just fail with ENOSYS,
+   and to avoid unnecessary work in massaging timestamps if the
+   syscall will work.  Multiple variables are needed, to distinguish
+   between the following scenarios on Linux:
+   utimensat doesn't exist, or is in glibc but kernel 2.6.18 fails with ENOSYS
+   utimensat handles files, but kernel 2.6.22 fails on symlinks
+   utimensat handles symlinks, but kernel 2.6.25 is too strict for UTIME_OMIT
+   utimensat handles UTIME_OMIT, but kernel 2.6.32 fails to always set ctime
+   utimensat completely works
+   For each cache variable: 0 = unknown, 1 = yes, -1 = no.  */
+static int utimensat_works_really;
+static int lutimensat_works_really;
+static int utimensat_ctime_really;
+
+/* Determine whether the kernel has a ctime bug.  ST1 and ST2
+   correspond to stat data before and after a successful time change.
+   TIMES contains the timestamps that were used during the time change
+   (at least one of times used UTIME_OMIT or UTIME_NOW).  Update the
+   cache variable if there is conclusive evidence of the kernel
+   working or being buggy.  Return true if TIMES has been updated and
+   another kernel call is needed, whether or not the kernel is known
+   to have the bug.  */
+static bool
+detect_ctime_bug (struct stat *st1, struct stat *st2, struct timespec times[2])
+{
+  struct timespec now;
+  if (st1->st_ctime != st2->st_ctime
+      || get_stat_ctime_ns (st1) != get_stat_ctime_ns (st2))
+    {
+      utimensat_ctime_really = 1;
+      return false;
+    }
+  if (times[0].tv_nsec == UTIME_NOW)
+    now = get_stat_atime (st2);
+  else if (times[1].tv_nsec == UTIME_NOW)
+    now = get_stat_mtime (st2);
+  else
+    gettime (&now);
+  if (now.tv_sec < st2->st_ctime
+      || 2 < now.tv_sec - st2->st_ctime
+      || (get_stat_ctime_ns (st2)
+          && now.tv_sec - st2->st_ctime < 2
+          && (20000000 < (1000000000 * (now.tv_sec - st2->st_ctime)
+                          + now.tv_nsec - get_stat_ctime_ns (st2)))))
+    utimensat_ctime_really = -1;
+  times[0] = get_stat_atime (st2);
+  times[1] = get_stat_mtime (st2);
+  return true;
+}
+#endif /* HAVE_UTIMENSAT || HAVE_FUTIMENS */
+
 /* Validate the requested timestamps.  Return 0 if the resulting
    timespec can be used for utimensat (after possibly modifying it to
    work around bugs in utimensat).  Return 1 if the timespec needs
@@ -205,10 +249,26 @@ fdutimens (char const *file, int fd, struct timespec 
const timespec[2])
 #if HAVE_UTIMENSAT || HAVE_FUTIMENS
   if (0 <= utimensat_works_really)
     {
+      int result;
+      struct stat st1;
+      struct stat st2;
+      /* Linux kernel 2.6.32 has a bug where it fails to bump ctime if
+         UTIME_OMIT was used.  It correctly bumps ctime if both times
+         are specified, but it costs time to do the extra [f]stat and
+         gettime(), so we avoid it if the function already works.  */
+      if (utimensat_ctime_really <= 0 && adjustment_needed)
+        {
+          if (fd < 0 ? stat (file, &st1) : fstat (fd, &st1))
+            return -1;
+          if (ts[0].tv_nsec == UTIME_OMIT && ts[1].tv_nsec == UTIME_OMIT)
+            return 0;
+          if (utimensat_ctime_really < 0)
+            update_timespec (&st1, &ts);
+        }
 # if HAVE_UTIMENSAT
       if (fd < 0)
         {
-          int result = utimensat (AT_FDCWD, file, ts, 0);
+          result = utimensat (AT_FDCWD, file, ts, 0);
 #  ifdef __linux__
           /* Work around a kernel bug:
              http://bugzilla.redhat.com/442352
@@ -223,13 +283,23 @@ fdutimens (char const *file, int fd, struct timespec 
const timespec[2])
           if (result == 0 || errno != ENOSYS)
             {
               utimensat_works_really = 1;
+              if (result == 0 && utimensat_ctime_really == 0
+                  && adjustment_needed)
+                {
+                  /* Perform a followup stat to see if the kernel has
+                     a ctime bug.  */
+                  if (stat (file, &st2))
+                    return -1;
+                  if (detect_ctime_bug (&st1, &st2, ts))
+                    result = utimensat (AT_FDCWD, file, ts, 0);
+                }
               return result;
             }
         }
 # endif /* HAVE_UTIMENSAT */
 # if HAVE_FUTIMENS
       {
-        int result = futimens (fd, ts);
+        result = futimens (fd, ts);
 #  ifdef __linux__
         /* Work around the same bug as above.  */
         if (0 < result)
@@ -238,6 +308,15 @@ fdutimens (char const *file, int fd, struct timespec const 
timespec[2])
         if (result == 0 || errno != ENOSYS)
           {
             utimensat_works_really = 1;
+            /* Work around the same bug as above.  */
+            if (result == 0 && utimensat_ctime_really == 0
+                && adjustment_needed)
+              {
+                if (fstat (fd, &st2))
+                  return -1;
+                if (detect_ctime_bug (&st1, &st2, ts))
+                  result = futimens (fd, ts);
+              }
             return result;
           }
       }
@@ -386,7 +465,23 @@ lutimens (char const *file, struct timespec const timespec
[2])
 #if HAVE_UTIMENSAT
   if (0 <= lutimensat_works_really)
     {
-      int result = utimensat (AT_FDCWD, file, ts, AT_SYMLINK_NOFOLLOW);
+      int result;
+      struct stat st1;
+      struct stat st2;
+      /* Linux kernel 2.6.32 has a bug where it fails to bump ctime if
+         UTIME_OMIT was used.  It correctly bumps ctime if both times
+         are specified, but it costs time to do the extra lstat and
+         gettime(), so we avoid it if the function already works.  */
+      if (utimensat_ctime_really <= 0 && adjustment_needed)
+        {
+          if (lstat (file, &st1))
+            return -1;
+          if (ts[0].tv_nsec == UTIME_OMIT && ts[1].tv_nsec == UTIME_OMIT)
+            return 0;
+          if (utimensat_ctime_really < 0)
+            update_timespec (&st1, &ts);
+        }
+      result = utimensat (AT_FDCWD, file, ts, AT_SYMLINK_NOFOLLOW);
 # ifdef __linux__
       /* Work around a kernel bug:
          http://bugzilla.redhat.com/442352
@@ -402,6 +497,15 @@ lutimens (char const *file, struct timespec const timespec
[2])
         {
           utimensat_works_really = 1;
           lutimensat_works_really = 1;
+          if (result == 0 && utimensat_ctime_really == 0
+              && adjustment_needed)
+            {
+              /* Check for the ctime bug.  */
+              if (lstat (file, &st2))
+                return -1;
+              if (detect_ctime_bug (&st1, &st2, ts))
+                result = utimensat (AT_FDCWD, file, ts, AT_SYMLINK_NOFOLLOW);
+            }
           return result;
         }
     }
-- 
1.6.4.2


>From 1e385ba7e75a4b08acf85222e1305fc26f3996c6 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 17 Dec 2009 07:32:00 -0700
Subject: [PATCH 3/3] futimens, utimensat: detect Linux bug

futimens is trivial - let utimens do the work.
utimensat is tougher: we don't want to call into utimens,
because that can cause unnecessary chdir.  So we have to
repeat the logic.  It ends up looking rather hairy.

* m4/futimens.m4 (gl_FUNC_FUTIMENS): Detect ctime bug.
* m4/utimensat.m4 (gl_FUNC_UTIMENSAT): Likewise.
* lib/utimensat.c (rpl_utimensat): Work around it.
* doc/posix-functions/futimens.texi (futimens): Document the bug.
* doc/posix-functions/utimensat.texi (utimensat): Likewise.
---
 ChangeLog                          |    7 +++
 doc/posix-functions/futimens.texi  |    4 ++
 doc/posix-functions/utimensat.texi |    4 ++
 lib/futimens.c                     |    2 +-
 lib/utimensat.c                    |   97 ++++++++++++++++++++++++++++++++++--
 m4/futimens.m4                     |   33 +++++++++---
 m4/utimensat.m4                    |   28 ++++++++---
 7 files changed, 153 insertions(+), 22 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e26282f..4420582 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2009-12-17  Eric Blake  <address@hidden>

+       futimens, utimensat: detect Linux bug
+       * m4/futimens.m4 (gl_FUNC_FUTIMENS): Detect ctime bug.
+       * m4/utimensat.m4 (gl_FUNC_UTIMENSAT): Likewise.
+       * lib/utimensat.c (rpl_utimensat): Work around it.
+       * doc/posix-functions/futimens.texi (futimens): Document the bug.
+       * doc/posix-functions/utimensat.texi (utimensat): Likewise.
+
        utimens: detect Linux bug
        * lib/utimens.c (detect_ctime_bug): New helper function.
        (fdutimens, lutimens): Use it to work around bug.
diff --git a/doc/posix-functions/futimens.texi b/doc/posix-
functions/futimens.texi
index fee3d08..66f5142 100644
--- a/doc/posix-functions/futimens.texi
+++ b/doc/posix-functions/futimens.texi
@@ -24,6 +24,10 @@ futimens
 the @code{tv_sec} argument to be 0, and don't necessarily handle all
 file permissions in the manner required by POSIX:
 Linux kernel 2.6.25.
address@hidden
+When using @code{UTIME_OMIT}, some systems fail to modify the change
+time of the file:
+Linux kernel 2.6.32.
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/doc/posix-functions/utimensat.texi b/doc/posix-
functions/utimensat.texi
index 67f5078..415ef26 100644
--- a/doc/posix-functions/utimensat.texi
+++ b/doc/posix-functions/utimensat.texi
@@ -26,6 +26,10 @@ utimensat
 the @code{tv_sec} argument to be 0, and don't necessarily handle all
 file permissions in the manner required by POSIX:
 Linux kernel 2.6.25.
address@hidden
+When using @code{UTIME_OMIT}, some systems fail to modify the change
+time of the file:
+Linux kernel 2.6.32.
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/lib/futimens.c b/lib/futimens.c
index 031a464..e93ffc4 100644
--- a/lib/futimens.c
+++ b/lib/futimens.c
@@ -32,6 +32,6 @@ futimens (int fd, struct timespec const times[2])
 {
   /* fdutimens also works around bugs in native futimens, when running
      with glibc compiled against newer headers but on a Linux kernel
-     older than 2.6.26.  */
+     older than 2.6.32.  */
   return fdutimens (NULL, fd, times);
 }
diff --git a/lib/utimensat.c b/lib/utimensat.c
index 3808439..690f50f 100644
--- a/lib/utimensat.c
+++ b/lib/utimensat.c
@@ -23,6 +23,8 @@
 #include <errno.h>
 #include <fcntl.h>

+#include "stat-time.h"
+#include "timespec.h"
 #include "utimens.h"

 #if HAVE_UTIMENSAT
@@ -31,10 +33,11 @@

 /* If we have a native utimensat, but are compiling this file, then
    utimensat was defined to rpl_utimensat by our replacement
-   sys/stat.h.  We assume the native version might fail with ENOSYS
-   (as is the case when using newer glibc but older Linux kernel).  In
-   this scenario, rpl_utimensat checks whether the native version is
-   usable, and local_utimensat provides the fallback manipulation.  */
+   sys/stat.h.  We assume the native version might fail with ENOSYS,
+   or succeed without properly affecting ctime (as is the case when
+   using newer glibc but older Linux kernel).  In this scenario,
+   rpl_utimensat checks whether the native version is usable, and
+   local_utimensat provides the fallback manipulation.  */

 static int local_utimensat (int, char const *, struct timespec const[2], int);
 # define AT_FUNC_NAME local_utimensat
@@ -46,9 +49,49 @@ rpl_utimensat (int fd, char const *file, struct timespec 
const times[2],
                int flag)
 {
   static int utimensat_works_really; /* 0 = unknown, 1 = yes, -1 = no.  */
+  static int utimensat_ctime_really; /* 0 = unknown, 1 = yes, -1 = no.  */
   if (0 <= utimensat_works_really)
     {
-      int result = utimensat (fd, file, times, flag);
+      int result;
+      struct stat st1;
+      struct stat st2;
+      struct timespec ts[2];
+      /* Linux kernel 2.6.32 has a bug where it fails to bump ctime if
+         UTIME_OMIT was used.  It correctly bumps ctime if both times
+         are specified, but it costs time to do the extra [l]stat and
+         gettime(), so we avoid it if the function already works.  */
+      if (utimensat_ctime_really <= 0 && times
+          && (times[0].tv_nsec == UTIME_NOW
+              || times[0].tv_nsec == UTIME_OMIT
+              || times[1].tv_nsec == UTIME_NOW
+              || times[1].tv_nsec == UTIME_OMIT))
+        {
+          if (fstatat (fd, file, &st1, flag))
+            return -1;
+          if (times[0].tv_nsec == UTIME_OMIT
+              && times[1].tv_nsec == UTIME_OMIT)
+            return 0;
+          if (times[0].tv_nsec == UTIME_NOW
+              && times[1].tv_nsec == UTIME_NOW)
+            times = NULL;
+          else if (utimensat_ctime_really < 0)
+            {
+              if (times[0].tv_nsec == UTIME_OMIT)
+                ts[0] = get_stat_atime (&st1);
+              else if (times[0].tv_nsec == UTIME_NOW)
+                gettime (&ts[0]);
+              else
+                ts[0] = times[0];
+              if (times[1].tv_nsec == UTIME_OMIT)
+                ts[1] = get_stat_mtime (&st1);
+              else if (times[1].tv_nsec == UTIME_NOW)
+                gettime (&ts[1]);
+              else
+                ts[1] = times[1];
+              times = ts;
+            }
+        }
+      result = utimensat (fd, file, times, flag);
       /* Linux kernel 2.6.25 has a bug where it returns EINVAL for
          UTIME_NOW or UTIME_OMIT with non-zero tv_sec, which
          local_utimensat works around.  Meanwhile, EINVAL for a bad
@@ -59,6 +102,50 @@ rpl_utimensat (int fd, char const *file, struct timespec 
const times[2],
       if (result == 0 || (errno != ENOSYS && errno != EINVAL))
         {
           utimensat_works_really = 1;
+          if (result == 0 && utimensat_ctime_really == 0 && times
+              && (times[0].tv_nsec == UTIME_NOW
+                  || times[0].tv_nsec == UTIME_OMIT
+                  || times[1].tv_nsec == UTIME_NOW
+                  || times[1].tv_nsec == UTIME_OMIT))
+            {
+              /* Perform a followup [l]stat.  If ctime changed, then
+                 the kernel does not have a bug.  Otherwise, if ctime
+                 is close to now (20 milliseconds is large enough for
+                 the 10ms quantization typical of file systems with
+                 sub-second resolution, 2 seconds is required for
+                 FAT), the results are inconclusive.  Whether or not
+                 the results are conclusive about a kernel bug, we
+                 re-call utimensat with specified times to guarantee a
+                 correct ctime.  Avoid calling gettime() more than
+                 once (that is, if the original call used UTIME_NOW,
+                 then the stat results are close enough to now).  */
+              struct timespec now;
+              if (fstatat (fd, file, &st2, flag))
+                return -1;
+              if (st1.st_ctime != st2.st_ctime
+                  || get_stat_ctime_ns (&st1) != get_stat_ctime_ns (&st2))
+                {
+                  utimensat_ctime_really = 1;
+                  return result;
+                }
+              if (times[0].tv_nsec == UTIME_NOW)
+                now = get_stat_atime (&st2);
+              else if (times[1].tv_nsec == UTIME_NOW)
+                now = get_stat_mtime (&st2);
+              else
+                gettime (&now);
+              if (now.tv_sec < st2.st_ctime
+                  || 2 < now.tv_sec - st2.st_ctime
+                  || (get_stat_ctime_ns (&st2)
+                      && now.tv_sec - st2.st_ctime < 2
+                      && (20000000 < (1000000000 * (now.tv_sec - st2.st_ctime)
+                                      + now.tv_nsec
+                                      - get_stat_ctime_ns (&st2)))))
+                utimensat_ctime_really = -1;
+              ts[0] = get_stat_atime (&st2);
+              ts[1] = get_stat_mtime (&st2);
+              result = utimensat (fd, file, ts, flag);
+            }
           return result;
         }
     }
diff --git a/m4/futimens.m4 b/m4/futimens.m4
index 0547e7a..ba5d6b6 100644
--- a/m4/futimens.m4
+++ b/m4/futimens.m4
@@ -1,4 +1,4 @@
-# serial 1
+# serial 2
 # See if we need to provide futimens replacement.

 dnl Copyright (C) 2009 Free Software Foundation, Inc.
@@ -22,17 +22,32 @@ AC_DEFUN([gl_FUNC_FUTIMENS],
       [AC_RUN_IFELSE([AC_LANG_PROGRAM([[
 #include <fcntl.h>
 #include <sys/stat.h>
+#include <unistd.h>
+]], [[struct timespec ts[2] = { { 1, UTIME_OMIT }, { 1, UTIME_NOW } };
+      int fd = creat ("conftest.file", 0600);
+      struct stat st;
+      if (fd < 0) return 1;
+      if (futimens (AT_FDCWD, NULL)) return 2;
+      if (futimens (fd, ts)) return 3;
+      sleep (1);
+      ts[0].tv_nsec = UTIME_NOW;
+      ts[1].tv_nsec = UTIME_OMIT;
+      if (futimens (fd, ts)) return 4;
+      if (fstat (fd, &st)) return 5;
+      if (st.st_ctime < st.st_atime) return 6;
+      ]])],
+         [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
 #ifdef __linux__
-/* The Linux kernel added futimens in 2.6.22, but it had bugs until 2.6.26.
-   Always replace futimens to support older kernels.  */
+/* The Linux kernel added futimens in 2.6.22, but has bugs with UTIME_OMIT
+   in 2.6.32.  Always replace futimens to support older kernels.  */
 choke me
 #endif
-]], [[struct timespec ts[2] = { { 1, UTIME_OMIT }, { 1, UTIME_OMIT } };
-      if (futimens (AT_FDCWD, NULL)) return 1;
-      return futimens (open (".", O_RDONLY), ts);]])],
-         [gl_cv_func_futimens_works=yes],
-         [gl_cv_func_futimens_works="needs runtime check"],
-         [gl_cv_func_futimens_works="guessing no"])])
+      ]])],
+           [gl_cv_func_futimens_works=yes],
+           [gl_cv_func_futimens_works="needs runtime check"])],
+         [gl_cv_func_futimens_works=no],
+         [gl_cv_func_futimens_works="guessing no"])
+      rm -f conftest.file])
     if test "$gl_cv_func_futimens_works" != yes; then
       REPLACE_FUTIMENS=1
       AC_LIBOBJ([futimens])
diff --git a/m4/utimensat.m4 b/m4/utimensat.m4
index 2a6033f..21e1ea5 100644
--- a/m4/utimensat.m4
+++ b/m4/utimensat.m4
@@ -1,4 +1,4 @@
-# serial 1
+# serial 2
 # See if we need to provide utimensat replacement.

 dnl Copyright (C) 2009 Free Software Foundation, Inc.
@@ -22,15 +22,29 @@ AC_DEFUN([gl_FUNC_UTIMENSAT],
       [AC_RUN_IFELSE([AC_LANG_PROGRAM([[
 #include <fcntl.h>
 #include <sys/stat.h>
+#include <unistd.h>
+]], [[struct timespec ts[2] = { { 1, UTIME_OMIT }, { 1, UTIME_NOW } };
+      struct stat st;
+      const char *f = "conftest.file";
+      if (close (creat (f, 0600))) return 1;
+      if (utimensat (AT_FDCWD, f, NULL, AT_SYMLINK_NOFOLLOW)) return 2;
+      if (utimensat (AT_FDCWD, f, ts, 0)) return 3;
+      sleep (1);
+      ts[0].tv_nsec = UTIME_NOW;
+      ts[1].tv_nsec = UTIME_OMIT;
+      if (utimensat (AT_FDCWD, f, ts, 0)) return 4;
+      if (stat (f, &st)) return 5;
+      if (st.st_ctime < st.st_atime) return 6;]])],
+         [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
 #ifdef __linux__
-/* The Linux kernel added utimensat in 2.6.22, but it had bugs until 2.6.26.
-   Always replace utimensat to support older kernels.  */
+/* The Linux kernel added utimensat in 2.6.22, but has bugs with UTIME_OMIT
+   in 2.6.32.  Always replace utimensat to support older kernels.  */
 choke me
 #endif
-]], [[struct timespec ts[2] = { { 1, UTIME_OMIT }, { 1, UTIME_OMIT } };
-      return utimensat (AT_FDCWD, ".", NULL, AT_SYMLINK_NOFOLLOW);]])],
-         [gl_cv_func_utimensat_works=yes],
-         [gl_cv_func_utimensat_works="needs runtime check"],
+      ]])],
+           [gl_cv_func_utimensat_works=yes],
+           [gl_cv_func_utimensat_works="needs runtime check"])],
+         [gl_cv_func_utimensat_works=no],
          [gl_cv_func_utimensat_works="guessing no"])])
     if test "$gl_cv_func_utimensat_works" != yes; then
       REPLACE_UTIMENSAT=1
-- 
1.6.4.2








reply via email to

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