From fadd1df67243af6d0d4f58b10b21755ee1f433d6 Mon Sep 17 00:00:00 2001
From: Florian Forster <octo@leeloo.lan.home.verplant.org>
Date: Mon, 7 Feb 2011 09:52:20 +0100
Subject: [PATCH] modbus plugin: Fix a small memory leak in
 mb_config_add_datagroup().

datagroup.name wasn't freed and datagroup.collect would leak if
datagroup_copy() failed.
---
 src/modbus.c | 70 +++++++++++++++++++++---------------------------------------
 1 file changed, 24 insertions(+), 46 deletions(-)

diff --git a/src/modbus.c b/src/modbus.c
index 661ab11e..2276572c 100644
--- a/src/modbus.c
+++ b/src/modbus.c
@@ -275,40 +275,6 @@ static int datagroup_append (mb_datagroup_t **dst, mb_datagroup_t *src) /* {{{ *
   return (0);
 } /* }}} int datagroup_append */
 
-/* Copy a single mb_datagroup_t and append it to another list. */
-static int datagroup_copy (mb_datagroup_t **dst, const mb_datagroup_t *src) /* {{{ */
-{
-  mb_datagroup_t *tmp;
-  int status;
-
-  if ((dst == NULL) || (src == NULL))
-    return (EINVAL);
-
-  tmp = malloc (sizeof (*tmp));
-  if (tmp == NULL)
-    return (ENOMEM);
-  memcpy (tmp, src, sizeof (*tmp));
-  tmp->name = NULL;
-  tmp->next = NULL;
-
-  tmp->name = strdup (src->name);
-  if (tmp->name == NULL)
-  {
-    sfree (tmp);
-    return (ENOMEM);
-  }
-
-  status = datagroup_append (dst, tmp);
-  if (status != 0)
-  {
-    sfree (tmp->name);
-    sfree (tmp);
-    return (status);
-  }
-
-  return (0);
-} /* }}} int datagroup_copy */
-
 /* Read functions */
 
 static int mb_submit (mb_host_t *host, mb_slave_t *slave, /* {{{ */
@@ -897,7 +863,7 @@ static int mb_config_add_slave (mb_host_t *host, oconfig_item_t *ci) /* {{{ */
         }
       }
       status = 0; /* continue after failure. */
-	}
+    }
     else
     {
       ERROR ("Modbus plugin: Unknown configuration option: %s", child->key);
@@ -1010,16 +976,19 @@ static int mb_config_add_host (oconfig_item_t *ci) /* {{{ */
 
 static int mb_config_add_datagroup (oconfig_item_t *ci) /* {{{ */
 {
-  mb_datagroup_t datagroup;
+  mb_datagroup_t *datagroup;
   int status;
   int i;
 
-  memset (&datagroup, 0, sizeof (datagroup));
-  datagroup.name = NULL;
-  datagroup.collect = NULL;
-  datagroup.next = NULL;
+  datagroup = malloc (sizeof (*datagroup));
+  if (datagroup == NULL)
+    return (ENOMEM);
+  memset (datagroup, 0, sizeof (*datagroup));
+  datagroup->name = NULL;
+  datagroup->collect = NULL;
+  datagroup->next = NULL;
 
-  status = cf_util_get_string (ci, &datagroup.name);
+  status = cf_util_get_string (ci, &datagroup->name);
   
   for (i = 0; i < ci->children_num; i++)
   {
@@ -1031,8 +1000,8 @@ static int mb_config_add_datagroup (oconfig_item_t *ci) /* {{{ */
       char buffer[1024];
       status = cf_util_get_string_buffer (child, buffer, sizeof (buffer));
       if (status == 0) {
-        data_copy_by_name (&datagroup.collect, data_definitions, buffer);
-	  }
+        data_copy_by_name (&datagroup->collect, data_definitions, buffer);
+      }
       status = 0; /* continue after failure. */
     }
     else
@@ -1045,11 +1014,20 @@ static int mb_config_add_datagroup (oconfig_item_t *ci) /* {{{ */
       break;
   } /* for (i = 0; i < ci->children_num; i++) */
 
-  if ((status == 0) && (datagroup.collect == NULL))
-    status = EINVAL;
+  if ((status == 0) && (datagroup->collect == NULL))
+    status = ENOENT;
 
   if (status == 0)
-    datagroup_copy (&data_groups, &datagroup);
+  {
+    datagroup_append (&data_groups, datagroup);
+  }
+  else /* if (status != 0) */
+  {
+    sfree (datagroup->name);
+    data_free_all (datagroup->collect);
+    assert (datagroup->next == NULL);
+    sfree (datagroup);
+  }
 
   return (status);
 } /* }}} int mb_config_add_datagroup */
-- 
2.11.0