From 9461848421c468d9117bf6c57335b344db0a420e Mon Sep 17 00:00:00 2001
From: Florian Forster <octo@collectd.org>
Date: Sun, 28 Jun 2015 15:00:49 +0200
Subject: [PATCH] src/daemon/common.c: Fix handling of counter_t in
 value_to_rate().

Also fixes an off-by-one error in counter_diff() and adds a unit test for
the function.
---
 src/daemon/common.c      |  8 ++++----
 src/daemon/common_test.c | 38 ++++++++++++++++++++++++++++++++++++++
 src/testing.h            |  9 +++++++++
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/src/daemon/common.c b/src/daemon/common.c
index 9e0648ca..73dd277f 100644
--- a/src/daemon/common.c
+++ b/src/daemon/common.c
@@ -1362,10 +1362,9 @@ counter_t counter_diff (counter_t old_value, counter_t new_value)
 	if (old_value > new_value)
 	{
 		if (old_value <= 4294967295U)
-			diff = (4294967295U - old_value) + new_value;
+			diff = (4294967295U - old_value) + new_value + 1;
 		else
-			diff = (18446744073709551615ULL - old_value)
-				+ new_value;
+			diff = (18446744073709551615ULL - old_value) + new_value + 1;
 	}
 	else
 	{
@@ -1516,7 +1515,8 @@ int value_to_rate (value_t *ret_rate, derive_t value, /* {{{ */
 	}
 	else if (ds_type == DS_TYPE_COUNTER)
 	{
-		ret_rate->gauge = (((counter_t)value) - state->last_value.counter) / interval;
+		counter_t diff = counter_diff (state->last_value.counter, (counter_t) value);
+		ret_rate->gauge = ((gauge_t) diff) / ((gauge_t) interval);
 		state->last_value.counter = (counter_t) value;
 	}
 	else if (ds_type == DS_TYPE_ABSOLUTE)
diff --git a/src/daemon/common_test.c b/src/daemon/common_test.c
index f40b2b1a..b1701bcf 100644
--- a/src/daemon/common_test.c
+++ b/src/daemon/common_test.c
@@ -338,6 +338,43 @@ DEF_TEST(parse_values)
   return (0);
 }
 
+DEF_TEST(value_to_rate)
+{
+  struct {
+    time_t t0;
+    time_t t1;
+    int ds_type;
+    value_t v0;
+    value_t v1;
+    gauge_t want;
+  } cases[] = {
+    { 0, 10, DS_TYPE_DERIVE,  {.derive  =    0}, {.derive = 1000},   NAN},
+    {10, 20, DS_TYPE_DERIVE,  {.derive  = 1000}, {.derive = 2000}, 100.0},
+    {20, 30, DS_TYPE_DERIVE,  {.derive  = 2000}, {.derive = 1800}, -20.0},
+    { 0, 10, DS_TYPE_COUNTER, {.counter =    0}, {.counter = 1000},   NAN},
+    {10, 20, DS_TYPE_COUNTER, {.counter = 1000}, {.counter = 5000}, 400.0},
+    /* 32bit wrap-around. */
+    {20, 30, DS_TYPE_COUNTER, {.counter = 4294967238}, {.counter =   42}, 10.0},
+    {30, 40, DS_TYPE_COUNTER, {.counter = 18446744073709551558ULL}, {.counter =   42}, 10.0},
+  };
+  size_t i;
+
+  for (i = 0; i < STATIC_ARRAY_SIZE (cases); i++) {
+    value_to_rate_state_t state = { cases[i].v0, TIME_T_TO_CDTIME_T (cases[i].t0) };
+    value_t got;
+
+    if (cases[i].t0 == 0) {
+      OK(value_to_rate (&got, cases[i].v1.derive, &state, cases[i].ds_type, TIME_T_TO_CDTIME_T (cases[i].t1)) == EAGAIN);
+      continue;
+    }
+
+    OK(value_to_rate (&got, cases[i].v1.derive, &state, cases[i].ds_type, TIME_T_TO_CDTIME_T (cases[i].t1)) == 0);
+    DBLEQ(cases[i].want, got.gauge);
+  }
+
+  return 0;
+}
+
 int main (void)
 {
   RUN_TEST(sstrncpy);
@@ -349,6 +386,7 @@ int main (void)
   RUN_TEST(escape_string);
   RUN_TEST(strunescape);
   RUN_TEST(parse_values);
+  RUN_TEST(value_to_rate);
 
   END_TEST;
 }
diff --git a/src/testing.h b/src/testing.h
index 5df1b83a..4056311f 100644
--- a/src/testing.h
+++ b/src/testing.h
@@ -54,6 +54,15 @@ static int check_count__ = 0;
   printf ("ok %i - %s evaluates to \"%s\"\n", ++check_count__, #actual, expect); \
 } while (0)
 
+#define DBLEQ(expect, actual) do { \
+  if ((isnan (expect) && !isnan (actual)) || ((expect) != (actual))) {\
+    printf ("not ok %i - %s incorrect: expected %.15g, got %.15g\n", \
+        ++check_count__, #actual, expect, actual); \
+    return (-1); \
+  } \
+  printf ("ok %i - %s evaluates to %.15g\n", ++check_count__, #actual, expect); \
+} while (0)
+
 #define CHECK_NOT_NULL(expr) do { \
   void *ptr_; \
   ptr_ = (expr); \
-- 
2.11.0