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 08381a8..ee6979c 100644 (file)
@@ -83,6 +83,7 @@ static int ethstat_add_map (const oconfig_item_t *ci) /* {{{ */
 {
   value_map_t *map;
   int status;
 {
   value_map_t *map;
   int status;
+  char *key;
 
   if ((ci->values_num < 2)
       || (ci->values_num > 3)
 
   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);
   }
 
     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)
   {
   map = malloc (sizeof (*map));
   if (map == NULL)
   {
+    sfree (key);
     ERROR ("ethstat plugin: malloc(3) failed.");
     return (ENOMEM);
   }
     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);
     if (value_map == NULL)
     {
       sfree (map);
+      sfree (key);
       ERROR ("ethstat plugin: c_avl_create() failed.");
       return (-1);
     }
   }
 
   status = c_avl_insert (value_map,
       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)
   {
       /* value = */ map);
   if (status != 0)
   {
-    sfree (map);
     if (status > 0)
     if (status > 0)
-      ERROR ("ethstat plugin: Multiple mappings for \"%s\".",
-          ci->values[0].value.string);
+      ERROR ("ethstat plugin: Multiple mappings for \"%s\".", key);
     else
     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);
   }
 
     return (-1);
   }
 
@@ -328,10 +338,31 @@ static int ethstat_read(void)
   return 0;
 }
 
   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);
 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 : */
 }
 
 /* vim: set sw=2 sts=2 et fdm=marker : */