From e41add16b3f19e89cd746a76c66cd28670037d7d Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Mon, 29 Jun 2015 12:50:51 +0200 Subject: [PATCH] src/daemon/common.[ch]: Improve signature of value_to_rate(). Passing in "value" as a derive_t and returning the rate as a value_t is just inconvenient and wrong. Instead, pass "value" as a value_t and return the rate in a gauge_t. While at it, move the state pointer to the end of the argument list. --- src/cpu.c | 10 +++--- src/daemon/common.c | 67 ++++++++++++++++------------------------ src/daemon/common.h | 4 +-- src/daemon/common_test.c | 8 ++--- 4 files changed, 38 insertions(+), 51 deletions(-) diff --git a/src/cpu.c b/src/cpu.c index 2a2a4a97..bcaea38c 100644 --- a/src/cpu.c +++ b/src/cpu.c @@ -526,11 +526,12 @@ static void cpu_commit (void) /* {{{ */ /* Adds a derive value to the internal state. This should be used by each read * function for each state. At the end of the iteration, the read function * should call cpu_commit(). */ -static int cpu_stage (size_t cpu_num, size_t state, derive_t value, cdtime_t now) /* {{{ */ +static int cpu_stage (size_t cpu_num, size_t state, derive_t d, cdtime_t now) /* {{{ */ { int status; cpu_state_t *s; - value_t v; + gauge_t rate = NAN; + value_t val = {.derive = d}; if (state >= COLLECTD_CPU_STATE_ACTIVE) return (EINVAL); @@ -544,12 +545,11 @@ static int cpu_stage (size_t cpu_num, size_t state, derive_t value, cdtime_t now s = get_cpu_state (cpu_num, state); - v.gauge = NAN; - status = value_to_rate (&v, value, &s->conv, DS_TYPE_DERIVE, now); + status = value_to_rate (&rate, val, DS_TYPE_DERIVE, now, &s->conv); if (status != 0) return (status); - s->rate = v.gauge; + s->rate = rate; s->has_value = 1; return (0); } /* }}} int cpu_stage */ diff --git a/src/daemon/common.c b/src/daemon/common.c index 73dd277f..92950ded 100644 --- a/src/daemon/common.c +++ b/src/daemon/common.c @@ -1469,11 +1469,10 @@ int rate_to_value (value_t *ret_value, gauge_t rate, /* {{{ */ return (0); } /* }}} value_t rate_to_value */ -int value_to_rate (value_t *ret_rate, derive_t value, /* {{{ */ - value_to_rate_state_t *state, - int ds_type, cdtime_t t) +int value_to_rate (gauge_t *ret_rate, /* {{{ */ + value_t value, int ds_type, cdtime_t t, value_to_rate_state_t *state) { - double interval; + gauge_t interval; /* Another invalid state: The time is not increasing. */ if (t <= state->last_time) @@ -1485,51 +1484,39 @@ int value_to_rate (value_t *ret_rate, derive_t value, /* {{{ */ interval = CDTIME_T_TO_DOUBLE(t - state->last_time); /* Previous value is invalid. */ - if (state->last_time == 0) /* {{{ */ + if (state->last_time == 0) { - if (ds_type == DS_TYPE_DERIVE) - { - state->last_value.derive = value; - } - else if (ds_type == DS_TYPE_COUNTER) - { - state->last_value.counter = (counter_t) value; - } - else if (ds_type == DS_TYPE_ABSOLUTE) - { - state->last_value.absolute = (absolute_t) value; - } - else - { - assert (23 == 42); - } - + state->last_value = value; state->last_time = t; return (EAGAIN); - } /* }}} */ + } - if (ds_type == DS_TYPE_DERIVE) - { - ret_rate->gauge = (value - state->last_value.derive) / interval; - state->last_value.derive = value; + switch (ds_type) { + case DS_TYPE_DERIVE: { + derive_t diff = value.derive - state->last_value.derive; + *ret_rate = ((gauge_t) diff) / ((gauge_t) interval); + break; } - else if (ds_type == DS_TYPE_COUNTER) - { - 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; + case DS_TYPE_GAUGE: { + *ret_rate = value.gauge; + break; } - else if (ds_type == DS_TYPE_ABSOLUTE) - { - ret_rate->gauge = (((absolute_t)value) - state->last_value.absolute) / interval; - state->last_value.absolute = (absolute_t) value; + case DS_TYPE_COUNTER: { + counter_t diff = counter_diff (state->last_value.counter, value.counter); + *ret_rate = ((gauge_t) diff) / ((gauge_t) interval); + break; } - else - { - assert (23 == 42); + case DS_TYPE_ABSOLUTE: { + absolute_t diff = value.absolute; + *ret_rate = ((gauge_t) diff) / ((gauge_t) interval); + break; + } + default: + return EINVAL; } - state->last_time = t; + state->last_value = value; + state->last_time = t; return (0); } /* }}} value_t rate_to_value */ diff --git a/src/daemon/common.h b/src/daemon/common.h index da21cad9..c3f7f548 100644 --- a/src/daemon/common.h +++ b/src/daemon/common.h @@ -357,8 +357,8 @@ counter_t counter_diff (counter_t old_value, counter_t new_value); int rate_to_value (value_t *ret_value, gauge_t rate, rate_to_value_state_t *state, int ds_type, cdtime_t t); -int value_to_rate (value_t *ret_rate, derive_t value, - value_to_rate_state_t *state, int ds_type, cdtime_t t); +int value_to_rate (gauge_t *ret_rate, value_t value, int ds_type, cdtime_t t, + value_to_rate_state_t *state); /* Converts a service name (a string) to a port number * (in the range [1-65535]). Returns less than zero on error. */ diff --git a/src/daemon/common_test.c b/src/daemon/common_test.c index d34e4112..21602d55 100644 --- a/src/daemon/common_test.c +++ b/src/daemon/common_test.c @@ -362,15 +362,15 @@ DEF_TEST(value_to_rate) 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; + gauge_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); + OK(value_to_rate (&got, cases[i].v1, cases[i].ds_type, TIME_T_TO_CDTIME_T(cases[i].t1), &state) == 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); + OK(value_to_rate (&got, cases[i].v1, cases[i].ds_type, TIME_T_TO_CDTIME_T(cases[i].t1), &state) == 0); + DBLEQ(cases[i].want, got); } return 0; -- 2.30.2