src/daemon/utils_time.h: Improve precision of conversion macros.

This patch also adds test cases for most conversions. It is inspired by
collectd/go-collectd#5 and uses some of the same test cases.

diff --git a/src/daemon/Makefile.am b/src/daemon/Makefile.am
index 1634713b97393a8ecd97d0d31b0f87494aafcb38..cd53a7bf205d48333f97d272c7f2e58d20db0628 100644 (file)
--- a/src/daemon/Makefile.am
+++ b/src/daemon/Makefile.am
@@ -93,8 +93,8 @@ else
 collectd_LDADD += -loconfig
 endif
 
-check_PROGRAMS = test_common test_meta_data test_utils_avltree test_utils_heap test_utils_subst
-TESTS          = test_common test_meta_data test_utils_avltree test_utils_heap test_utils_subst
+check_PROGRAMS = test_common test_meta_data test_utils_avltree test_utils_heap test_utils_time test_utils_subst
+TESTS          = test_common test_meta_data test_utils_avltree test_utils_heap test_utils_time test_utils_subst
 
 test_common_SOURCES = common_test.c ../testing.h
 test_common_LDADD = libcommon.la libplugin_mock.la
@@ -108,6 +108,8 @@ test_utils_avltree_LDADD = libavltree.la $(COMMON_LIBS)
 test_utils_heap_SOURCES = utils_heap_test.c ../testing.h
 test_utils_heap_LDADD = libheap.la $(COMMON_LIBS)
 
+test_utils_time_SOURCES = utils_time_test.c ../testing.h
+
 test_utils_subst_SOURCES = utils_subst_test.c ../testing.h \
                           utils_subst.c utils_subst.h
 test_utils_subst_LDADD = libcommon.la libplugin_mock.la

diff --git a/src/daemon/utils_time.h b/src/daemon/utils_time.h
index 9b08e8e4de68f011d24b4dc947c9d23922506544..f2d474a089871867150bb534df024cc9afde7447 100644 (file)
--- a/src/daemon/utils_time.h
+++ b/src/daemon/utils_time.h
@@ -42,32 +42,37 @@
 /* typedef uint64_t cdtime_t; */
 
 /* 2^30 = 1073741824 */
-#define TIME_T_TO_CDTIME_T(t) (((cdtime_t) (t)) * 1073741824)
-#define CDTIME_T_TO_TIME_T(t) ((time_t) ((t) / 1073741824))
+#define TIME_T_TO_CDTIME_T(t) (((cdtime_t) (t)) << 30)
+
+#define MS_TO_CDTIME_T(ms) (((((cdtime_t) (ms)) / 1000) << 30) | \
+    ((((((cdtime_t) (ms)) % 1000) << 30) + 500) / 1000))
+#define US_TO_CDTIME_T(us) (((((cdtime_t) (us)) / 1000000) << 30) | \
+    ((((((cdtime_t) (us)) % 1000000) << 30) + 500000) / 1000000))
+#define NS_TO_CDTIME_T(ns) (((((cdtime_t) (ns)) / 1000000000) << 30) | \
+    ((((((cdtime_t) (ns)) % 1000000000) << 30) + 500000000) / 1000000000))
+
+#define CDTIME_T_TO_TIME_T(t) ((((time_t) (t)) + (1 << 29)) >> 30)
+#define CDTIME_T_TO_MS(t)  (((((long) (t)) >> 30) * 1000L) + \
+  (((((long) (t)) & 0x3fffffff) * 1000L + (1 << 29)) >> 30))
+#define CDTIME_T_TO_US(t)  (((((suseconds_t) (t)) >> 30) * 1000000L) + \
+  (((((suseconds_t) (t)) & 0x3fffffff) * 1000000L + (1 << 29)) >> 30))
+#define CDTIME_T_TO_NS(t)  (((((long) (t)) >> 30) * 1000000000L) + \
+  (((((long) (t)) & 0x3fffffff) * 1000000000L + (1 << 29)) >> 30))
 
 #define CDTIME_T_TO_DOUBLE(t) (((double) (t)) / 1073741824.0)
 #define DOUBLE_TO_CDTIME_T(d) ((cdtime_t) ((d) * 1073741824.0))
 
-#define MS_TO_CDTIME_T(ms) ((cdtime_t)    (((double) (ms)) * 1073741.824))
-#define CDTIME_T_TO_MS(t)  ((long)        (((double) (t))  / 1073741.824))
-#define US_TO_CDTIME_T(us) ((cdtime_t)    (((double) (us)) * 1073.741824))
-#define CDTIME_T_TO_US(t)  ((suseconds_t) (((double) (t))  / 1073.741824))
-#define NS_TO_CDTIME_T(ns) ((cdtime_t)    (((double) (ns)) * 1.073741824))
-#define CDTIME_T_TO_NS(t)  ((long)        (((double) (t))  / 1.073741824))
-
 #define CDTIME_T_TO_TIMEVAL(cdt,tvp) do {                                    \
         (tvp)->tv_sec = CDTIME_T_TO_TIME_T (cdt);                            \
-        (tvp)->tv_usec = CDTIME_T_TO_US ((cdt) % 1073741824);                \
+        (tvp)->tv_usec = CDTIME_T_TO_US ((cdt) & 0x3fffffff);                \
 } while (0)
-#define TIMEVAL_TO_CDTIME_T(tv) (TIME_T_TO_CDTIME_T ((tv)->tv_sec)           \
-    + US_TO_CDTIME_T ((tv)->tv_usec))
+#define TIMEVAL_TO_CDTIME_T(tv) US_TO_CDTIME_T(1000000 * (tv)->tv_sec + (tv)->tv_usec)
 
 #define CDTIME_T_TO_TIMESPEC(cdt,tsp) do {                                   \
   (tsp)->tv_sec = CDTIME_T_TO_TIME_T (cdt);                                  \
-  (tsp)->tv_nsec = CDTIME_T_TO_NS ((cdt) % 1073741824);                      \
+  (tsp)->tv_nsec = CDTIME_T_TO_NS ((cdt) & 0x3fffffff);                      \
 } while (0)
-#define TIMESPEC_TO_CDTIME_T(ts) (TIME_T_TO_CDTIME_T ((ts)->tv_sec)           \
-    + NS_TO_CDTIME_T ((ts)->tv_nsec))
+#define TIMESPEC_TO_CDTIME_T(ts) NS_TO_CDTIME_T(1000000000 * (ts)->tv_sec + (ts)->tv_nsec)
 
 cdtime_t cdtime (void);
 

diff --git a/src/daemon/utils_time_test.c b/src/daemon/utils_time_test.c
new file mode 100644 (file)
index 0000000..1f13556
--- /dev/null
+++ b/src/daemon/utils_time_test.c
@@ -0,0 +1,124 @@
+/**
+ * collectd - src/daemon/utils_time_test.c
+ * Copyright (C) 2015       Florian octo Forster
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *   Florian octo Forster <octo at collectd.org>
+ */
+
+#define DBL_PRECISION 1e-3
+
+#include "testing.h"
+#include "collectd.h"
+#include "utils_time.h"
+
+DEF_TEST(conversion)
+{
+  struct {
+    cdtime_t t;
+    double d;
+    time_t tt;
+    struct timeval tv;
+    struct timespec ts;
+  } cases[] = {
+  /*              cdtime          double      time_t               timeval                 timespec */
+    {                  0,          0.0  ,          0, {         0,      0}, {         0,         0}},
+    {        10737418240,         10.0  ,         10, {        10,      0}, {        10,         0}},
+    {1542908534771941376, 1436945549.0  , 1436945549, {1436945549,      0}, {1436945549,         0}},
+    {1542908535540740522, 1436945549.716, 1436945550, {1436945549, 716000}, {1436945549, 716000000}},
+    // 1426076671.123 * 2^30 = 1531238166015458148.352
+    {1531238166015458148, 1426076671.123, 1426076671, {1426076671, 123000}, {1426076671, 123000000}},
+    // 1426076681.234 * 2^30 = 1531238176872061730.816
+    {1531238176872061731, 1426076681.234, 1426076681, {1426076681, 234000}, {1426076681, 234000000}},
+    // 1426083986.314 * 2^30 = 1531246020641985396.736
+    {1531246020641985397, 1426083986.314, 1426083986, {1426083986, 314000}, {1426083986, 314000000}},
+    // 1426083986.494142531 * 2^30 = 1531246020835411966.5
+    {1531246020835411967, 1426083986.494, 1426083986, {1426083986, 494143}, {1426083986, 494142531}},
+    // 1426083986.987410814 * 2^30 = 1531246021365054752.4
+    {1531246021365054752, 1426083986.987, 1426083987, {1426083986, 987411}, {1426083986, 987410814}},
+
+    /* These cases test the cdtime_t -> ns conversion rounds correctly. */
+    // 1546167635576736987 / 2^30 = 1439980823.1524536265...
+    {1546167635576736987, 1439980823.152, 1439980823, {1439980823, 152454}, {1439980823, 152453627}},
+    // 1546167831554815222 / 2^30 = 1439981005.6712620165...
+    {1546167831554815222, 1439981005.671, 1439981006, {1439981005, 671262}, {1439981005, 671262017}},
+    // 1546167986577716567 / 2^30 = 1439981150.0475896215...
+    {1546167986577716567, 1439981150.048, 1439981150, {1439981150,  47590}, {1439981005,  47589622}},
+  };
+  size_t i;
+
+  for (i = 0; i < (sizeof (cases) / sizeof (cases[0])); i++) {
+    struct timeval tv;
+    struct timespec ts;
+    time_t tt;
+
+    // cdtime -> ns
+    CDTIME_T_TO_TIMESPEC (cases[i].t, &ts);
+    EXPECT_EQ_UINT64 ((uint64_t) cases[i].ts.tv_nsec, (uint64_t) ts.tv_nsec);
+
+    // cdtime -> us
+    CDTIME_T_TO_TIMEVAL (cases[i].t, &tv);
+    EXPECT_EQ_UINT64 ((uint64_t) cases[i].tv.tv_usec, (uint64_t) tv.tv_usec);
+
+    // cdtime -> s
+    tt = CDTIME_T_TO_TIME_T (cases[i].t);
+    EXPECT_EQ_UINT64 ((uint64_t) cases[i].tt, (uint64_t) tt);
+
+    // cdtime -> double
+    DBLEQ (cases[i].d, CDTIME_T_TO_DOUBLE (cases[i].t));
+  }
+
+  return 0;
+}
+
+/* These cases test the ns -> cdtime_t conversion rounds correctly. */
+DEF_TEST(ns_to_cdtime)
+{
+  struct {
+    long ns;
+    cdtime_t want;
+  } cases[] = {
+    // 1439981652801860766 * 2^30 / 10^9 = 1546168526406004689.4
+    {1439981652801860766, 1546168526406004689},
+    // 1439981836985281914 * 2^30 / 10^9 = 1546168724171447263.4
+    {1439981836985281914, 1546168724171447263},
+    // 1439981880053705608 * 2^30 / 10^9 = 1546168770415815077.4
+    {1439981880053705608, 1546168770415815077},
+  };
+  size_t i;
+
+  for (i = 0; i < (sizeof (cases) / sizeof (cases[0])); i++) {
+    cdtime_t got = NS_TO_CDTIME_T (cases[i].ns);
+    EXPECT_EQ_UINT64 ((uint64_t) cases[i].want, (uint64_t) got);
+  }
+
+  return 0;
+}
+
+int main (void)
+{
+  RUN_TEST(conversion);
+  RUN_TEST(ns_to_cdtime);
+
+  END_TEST;
+}
+
+/* vim: set sw=2 sts=2 et : */

diff --git a/src/testing.h b/src/testing.h
index 84a1242953284309fe015df0ee6c43d95e6ba434..1bbe800913315d798530fb9d0390541443f35c7c 100644 (file)
--- a/src/testing.h
+++ b/src/testing.h
@@ -24,6 +24,8 @@
  *   Florian octo Forster <octo at collectd.org>
  */
 
+#include <inttypes.h>
+
 static int fail_count__ = 0;
 static int check_count__ = 0;
 
@@ -59,6 +61,15 @@ static int check_count__ = 0;
   printf ("ok %i - %s evaluates to \"%s\"\n", ++check_count__, #actual, expect); \
 } while (0)
 
+#define EXPECT_EQ(expect, actual, format) do { \
+  if ((expect) != (actual)) {\
+    printf ("not ok %i - %s incorrect: expected " format ", got " format "\n", \
+        ++check_count__, #actual, expect, actual); \
+    return (-1); \
+  } \
+  printf ("ok %i - %s evaluates to " format "\n", ++check_count__, #actual, expect); \
+} while (0)
+
 #define EXPECT_INTEQ(expect, actual) do { \
   if ((expect) != (actual)) {\
     printf ("not ok %i - %s incorrect: expected %d, got %d\n", \
@@ -68,6 +79,8 @@ static int check_count__ = 0;
   printf ("ok %i - %s evaluates to %d\n", ++check_count__, #actual, expect); \
 } while (0)
 
+#define EXPECT_EQ_UINT64(expect, actual) EXPECT_EQ((expect), (actual), "%"PRIu64)
+
 #define DBLEQ(expect, actual) do { \
   double e = (expect); double a = (actual); \
   if (isnan (e) && !isnan (a)) { \