Code

ethstat plugin: Fix the map lookup.
authorFlorian Forster <octo@collectd.org>
Fri, 11 Jan 2013 18:45:43 +0000 (19:45 +0100)
committerFlorian Forster <octo@collectd.org>
Fri, 11 Jan 2013 18:45:43 +0000 (19:45 +0100)
Previously, a pointer into the configuration structure was used to look
up the mappings. Because the configuration structure is freed or
otherwise modified, this does not work as intended (and may actually
lead to a segmentation fault when unlucky).

For completeness sake, a shutdown callback was added to free the used
memory on exit.

Fixes Github issue 224.

src/ethstat.c

index 08381a821087e275fb03922a32bc54c35ca7c3cd..ee6979c52aafd1ca542034adad6002a2a6898ce1 100644 (file)
@@ -83,6 +83,7 @@ static int ethstat_add_map (const oconfig_item_t *ci) /* {{{ */
 {
   value_map_t *map;
   int status;
+  char *key;
 
   if ((ci->values_num < 2)
       || (ci->values_num > 3)
@@ -96,9 +97,17 @@ static int ethstat_add_map (const oconfig_item_t *ci) /* {{{ */
     return (-1);
   }
 
+  key = strdup (ci->values[0].value.string);
+  if (key == NULL)
+  {
+    ERROR ("ethstat plugin: strdup(3) failed.");
+    return (ENOMEM);
+  }
+
   map = malloc (sizeof (*map));
   if (map == NULL)
   {
+    sfree (key);
     ERROR ("ethstat plugin: malloc(3) failed.");
     return (ENOMEM);
   }
@@ -115,23 +124,24 @@ static int ethstat_add_map (const oconfig_item_t *ci) /* {{{ */
     if (value_map == NULL)
     {
       sfree (map);
+      sfree (key);
       ERROR ("ethstat plugin: c_avl_create() failed.");
       return (-1);
     }
   }
 
   status = c_avl_insert (value_map,
-      /* key = */ ci->values[0].value.string,
+      /* key = */ key,
       /* value = */ map);
   if (status != 0)
   {
-    sfree (map);
     if (status > 0)
-      ERROR ("ethstat plugin: Multiple mappings for \"%s\".",
-          ci->values[0].value.string);
+      ERROR ("ethstat plugin: Multiple mappings for \"%s\".", key);
     else
-      ERROR ("ethstat plugin: c_avl_insert(\"%s\") failed.",
-          ci->values[0].value.string);
+      ERROR ("ethstat plugin: c_avl_insert(\"%s\") failed.", key);
+
+    sfree (map);
+    sfree (key);
     return (-1);
   }
 
@@ -328,10 +338,31 @@ static int ethstat_read(void)
   return 0;
 }
 
+static int ethstat_shutdown (void)
+{
+  void *key = NULL;
+  void *value = NULL;
+
+  if (value_map == NULL)
+    return (0);
+
+  while (c_avl_pick (value_map, &key, &value) == 0)
+  {
+    sfree (key);
+    sfree (value);
+  }
+
+  c_avl_destroy (value_map);
+  value_map = NULL;
+
+  return (0);
+}
+
 void module_register (void)
 {
   plugin_register_complex_config ("ethstat", ethstat_config);
   plugin_register_read ("ethstat", ethstat_read);
+  plugin_register_shutdown ("ethstat", ethstat_shutdown);
 }
 
 /* vim: set sw=2 sts=2 et fdm=marker : */