[PATCH] Fix TimeValue POSIX representation

[Dmitri Gribenko]

Hi chandlerc,

Currently, TimeValue has a toPosixTime() function.  The semantics of this function are not clear.  This patch choses a particular interpretation of what is "POSIX time", and changes TimeValue to follow it consistently.

Here are the point of confusion that the patch tries to resolve:
* The toPosixTime() function has a confusing comment saying that POSIX zero time is at *100 ns* past 12:00:00a Jan 1, 1970.  I tried to find this in relevant standards, but there is nothing like this.
* In fact, toPosixTime() function does not perform this 100 ns adjustment.
* NANOSECONDS_PER_POSIX_TICK was defined as 100, but the comment says that the tick is 10ms.
* Even more, toPosixTime() was completely wrong when it was adding seconds and 100 ns-intervals.  Seems like this function was never used with a TimeValue that had non-zero nanoseconds.

This patch changes a POSIX tick as 10ms (to match the most common 100Hz clock), and fixes up the toPosixTime() function to return time in 10ms ticks.

Another possibility would be, of course, to set 1 tick = 100 ns.  What does everyone think?

Please review.

http://llvm-reviews.chandlerc.com/D2720

Files:
  include/llvm/Support/TimeValue.h
  lib/Support/Unix/Path.inc
  unittests/Support/TimeValueTest.cpp

Index: include/llvm/Support/TimeValue.h
===================================================================
--- include/llvm/Support/TimeValue.h
+++ include/llvm/Support/TimeValue.h
@@ -74,8 +74,8 @@
       MILLISECONDS_PER_SECOND = 1000,       ///< One Thousand
       NANOSECONDS_PER_MICROSECOND = 1000,   ///< One Thousand
       NANOSECONDS_PER_MILLISECOND = 1000000,///< One Million
-      NANOSECONDS_PER_POSIX_TICK = 100,     ///< Posix tick is 100 Hz (10ms)
-      NANOSECONDS_PER_WIN32_TICK = 100      ///< Win32 tick is 100 Hz (10ms)
+      NANOSECONDS_PER_POSIX_TICK = 10000000,///< Posix tick is 100 Hz (10ms)
+      NANOSECONDS_PER_WIN32_TICK = 100      ///< Win32 tick is 10^7 Hz (10ns)
     };
 
   /// @}
@@ -238,9 +238,10 @@
 
     /// Converts the TimeValue into the corresponding number of "ticks" for
     /// Posix, correcting for the difference in Posix zero time.
-    /// @brief Convert to unix time (100 nanoseconds since 12:00:00a Jan 1,1970)
+    /// @brief Convert to POSIX time (10ms intervals since 12:00:00a Jan 1,1970)
     uint64_t toPosixTime() const {
       uint64_t result = seconds_ - PosixZeroTimeSeconds;
+      result *= NANOSECONDS_PER_SECOND / NANOSECONDS_PER_POSIX_TICK;
       result += nanos_ / NANOSECONDS_PER_POSIX_TICK;
       return result;
     }
Index: lib/Support/Unix/Path.inc
===================================================================
--- lib/Support/Unix/Path.inc
+++ lib/Support/Unix/Path.inc
@@ -527,7 +527,7 @@
 error_code setLastModificationAndAccessTime(int FD, TimeValue Time) {
 #if defined(HAVE_FUTIMENS)
   timespec Times[2];
-  Times[0].tv_sec = Time.toPosixTime();
+  Times[0].tv_sec = Time.toEpochTime();
   Times[0].tv_nsec = 0;
   Times[1] = Times[0];
   if (::futimens(FD, Times))
@@ -535,7 +535,7 @@
   return error_code::success();
 #elif defined(HAVE_FUTIMES)
   timeval Times[2];
-  Times[0].tv_sec = Time.toPosixTime();
+  Times[0].tv_sec = Time.toEpochTime();
   Times[0].tv_usec = 0;
   Times[1] = Times[0];
   if (::futimes(FD, Times))
Index: unittests/Support/TimeValueTest.cpp
===================================================================
--- unittests/Support/TimeValueTest.cpp
+++ unittests/Support/TimeValueTest.cpp
@@ -30,7 +30,8 @@
   epoch.fromWin32Time(ft1970);
 
   // The "seconds" part in Posix time may be expected as zero.
-  EXPECT_EQ(ns / 100, epoch.toPosixTime());
+  EXPECT_EQ(ns / 10000000, epoch.toPosixTime());
+  EXPECT_EQ(ns, static_cast<uint32_t>(epoch.nanoseconds()));
 
   // Confirm it reversible.
   EXPECT_EQ(ft1970, epoch.toWin32Time());
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2720.1.patch
Type: text/x-patch
Size: 2562 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140207/eda130dd/attachment.bin>