From 2ede5e7fd0f7e75edd1fba531cd870bb16092e71 Mon Sep 17 00:00:00 2001
From: Florian Forster <octo@collectd.org>
Date: Wed, 26 Oct 2016 09:31:47 +0200
Subject: [PATCH] src/daemon/utils_cache.c: Refactor uc_check_timeout().

---
 src/daemon/utils_cache.c | 113 ++++++++++++++++-------------------------------
 1 file changed, 38 insertions(+), 75 deletions(-)

diff --git a/src/daemon/utils_cache.c b/src/daemon/utils_cache.c
index 7c1fa41e..09298725 100644
--- a/src/daemon/utils_cache.c
+++ b/src/daemon/utils_cache.c
@@ -239,81 +239,54 @@ int uc_init (void)
 
 int uc_check_timeout (void)
 {
-  cdtime_t now;
-  cache_entry_t *ce;
-
-  char **keys = NULL;
-  cdtime_t *keys_time = NULL;
-  cdtime_t *keys_interval = NULL;
-  int keys_len = 0;
-
-  char *key;
-  c_avl_iterator_t *iter;
+  cdtime_t now = cdtime();
 
-  int status;
+  struct {
+    char *key;
+    cdtime_t time;
+    cdtime_t interval;
+  } *expired = NULL;
+  size_t expired_num = 0;
 
   pthread_mutex_lock (&cache_lock);
 
-  now = cdtime ();
-
   /* Build a list of entries to be flushed */
-  iter = c_avl_get_iterator (cache_tree);
+  c_avl_iterator_t *iter = c_avl_get_iterator (cache_tree);
+  char *key = NULL;
+  cache_entry_t *ce = NULL;
   while (c_avl_iterator_next (iter, (void *) &key, (void *) &ce) == 0)
   {
-    char **tmp;
-    cdtime_t *tmp_time;
-
     /* If the entry is fresh enough, continue. */
     if ((now - ce->last_update) < (ce->interval * timeout_g))
       continue;
 
-    /* If entry has not been updated, add to `keys' array */
-    tmp = realloc ((void *) keys,
-	(keys_len + 1) * sizeof (char *));
+    void *tmp = realloc (expired, (expired_num + 1) * sizeof (*expired));
     if (tmp == NULL)
     {
       ERROR ("uc_check_timeout: realloc failed.");
       continue;
     }
-    keys = tmp;
-
-    tmp_time = realloc (keys_time, (keys_len + 1) * sizeof (*keys_time));
-    if (tmp_time == NULL)
-    {
-      ERROR ("uc_check_timeout: realloc failed.");
-      continue;
-    }
-    keys_time = tmp_time;
+    expired = tmp;
 
-    tmp_time = realloc (keys_interval, (keys_len + 1) * sizeof (*keys_interval));
-    if (tmp_time == NULL)
-    {
-      ERROR ("uc_check_timeout: realloc failed.");
-      continue;
-    }
-    keys_interval = tmp_time;
+    expired[expired_num].key = strdup (key);
+    expired[expired_num].time = ce->last_time;
+    expired[expired_num].interval = ce->interval;
 
-    keys[keys_len] = strdup (key);
-    if (keys[keys_len] == NULL)
+    if (expired[expired_num].key == NULL)
     {
       ERROR ("uc_check_timeout: strdup failed.");
       continue;
     }
-    keys_time[keys_len] = ce->last_time;
-    keys_interval[keys_len] = ce->interval;
 
-    keys_len++;
+    expired_num++;
   } /* while (c_avl_iterator_next) */
 
   c_avl_iterator_destroy (iter);
   pthread_mutex_unlock (&cache_lock);
 
-  if (keys_len == 0)
+  if (expired_num == 0)
   {
-    /* realloc() may have been called for these. */
-    sfree (keys);
-    sfree (keys_time);
-    sfree (keys_interval);
+    sfree (expired);
     return (0);
   }
 
@@ -322,55 +295,45 @@ int uc_check_timeout (void)
    * including plugin specific meta data, rates, history, …. This must be done
    * without holding the lock, otherwise we will run into a deadlock if a
    * plugin calls the cache interface. */
-  for (int i = 0; i < keys_len; i++)
+  for (size_t i = 0; i < expired_num; i++)
   {
-    value_list_t vl = VALUE_LIST_INIT;
-
-    vl.values = NULL;
-    vl.values_len = 0;
-    vl.meta = NULL;
+    value_list_t vl = {
+      .time = expired[i].time,
+      .interval = expired[i].interval,
+    };
 
-    status = parse_identifier_vl (keys[i], &vl);
-    if (status != 0)
+    if (parse_identifier_vl (expired[i].key, &vl) != 0)
     {
-      ERROR ("uc_check_timeout: parse_identifier_vl (\"%s\") failed.", keys[i]);
+      ERROR ("uc_check_timeout: parse_identifier_vl (\"%s\") failed.", expired[i].key);
       continue;
     }
 
-    vl.time = keys_time[i];
-    vl.interval = keys_interval[i];
-
     plugin_dispatch_missing (&vl);
-  } /* for (i = 0; i < keys_len; i++) */
+  } /* for (i = 0; i < expired_num; i++) */
 
   /* Now actually remove all the values from the cache. We don't re-evaluate
    * the timestamp again, so in theory it is possible we remove a value after
    * it is updated here. */
   pthread_mutex_lock (&cache_lock);
-  for (int i = 0; i < keys_len; i++)
+  for (size_t i = 0; i < expired_num; i++)
   {
-    key = NULL;
-    ce = NULL;
+    char *key = NULL;
+    cache_entry_t *value = NULL;
 
-    status = c_avl_remove (cache_tree, keys[i],
-	(void *) &key, (void *) &ce);
-    if (status != 0)
+    if (c_avl_remove (cache_tree, expired[i].key, (void *) &key, (void *) &value) != 0)
     {
-      ERROR ("uc_check_timeout: c_avl_remove (\"%s\") failed.", keys[i]);
-      sfree (keys[i]);
+      ERROR ("uc_check_timeout: c_avl_remove (\"%s\") failed.", expired[i].key);
+      sfree (expired[i].key);
       continue;
     }
-
-    sfree (keys[i]);
     sfree (key);
-    cache_free (ce);
-  } /* for (i = 0; i < keys_len; i++) */
-  pthread_mutex_unlock (&cache_lock);
+    cache_free (value);
 
-  sfree (keys);
-  sfree (keys_time);
-  sfree (keys_interval);
+    sfree (expired[i].key);
+  } /* for (i = 0; i < expired_num; i++) */
+  pthread_mutex_unlock (&cache_lock);
 
+  sfree (expired);
   return (0);
 } /* int uc_check_timeout */
 
-- 
2.11.0