[PATCH] Address another obvious memory leak in loadparm.

[Volker Lendecke]

On Wed, Feb 17, 2016 at 12:18:09AM +0000, Hemanth Thummala wrote:
 
> I have attached the patch which has the fix for this
> problem. Please let me know if the patch looks good.

Yes, the patch looks very good, thanks! I haven't run a private autobuild
with that yet. Does that survive?

Attached find two cosmetic changes. Feel free to squash them.

Also, your change triggers the question -- do we have a proper talloc
hierarchy for "struct loadparm_service"? If so, couldn't we avoid the
special handling in free_service()?

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 8e2203a5d2b1cc2b854ccd88e2519ac3d472a495 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 17 Feb 2016 08:43:52 +0100
Subject: [PATCH 1/2] loadparm: Respect 80-char line length

---
 source3/param/loadparm.c |   15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 476aa10..bb3592a 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -93,9 +93,11 @@ static struct smbconf_csn conf_last_csn;
 static int config_backend = CONFIG_BACKEND_FILE;
 
 /* some helpful bits */
-#define LP_SNUM_OK(i) (((i) >= 0) && ((i) < iNumServices) && (ServicePtrs != NULL) && \
-				(ServicePtrs[(i)] != NULL) && ServicePtrs[(i)]->valid)
-#define VALID(i) (ServicePtrs != NULL && (ServicePtrs[i]!= NULL) && ServicePtrs[i]->valid)
+#define LP_SNUM_OK(i) (((i) >= 0) && ((i) < iNumServices) && \
+                       (ServicePtrs != NULL) && \
+		       (ServicePtrs[(i)] != NULL) && ServicePtrs[(i)]->valid)
+#define VALID(i) ((ServicePtrs != NULL) && (ServicePtrs[i]!= NULL) && \
+                  ServicePtrs[i]->valid)
 
 #define USERSHARE_VALID 1
 #define USERSHARE_PENDING_DELETE 2
@@ -1392,9 +1394,12 @@ static int add_a_service(const struct loadparm_service *pservice, const char *na
 	if (!found) {
 		/* if not, then create one */
 		i = iNumServices;
-		tsp = talloc_realloc(NULL, ServicePtrs, struct loadparm_service *, num_to_alloc);
+		tsp = talloc_realloc(NULL, ServicePtrs,
+				     struct loadparm_service *,
+				     num_to_alloc);
 		if (tsp == NULL) {
-			DEBUG(0,("add_a_service: failed to enlarge ServicePtrs!\n"));
+			DEBUG(0, ("add_a_service: failed to enlarge "
+				  "ServicePtrs!\n"));
 			return (-1);
 		}
 		ServicePtrs = tsp;
-- 
1.7.9.5


From b024e98e0c05e0883b9af07d7105c8c44811300b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 17 Feb 2016 08:44:33 +0100
Subject: [PATCH 2/2] loadparm: Avoid a bool variable

---
 source3/param/loadparm.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index bb3592a..50b29e3 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -1374,7 +1374,6 @@ static int add_a_service(const struct loadparm_service *pservice, const char *na
 	int i;
 	int num_to_alloc = iNumServices + 1;
 	struct loadparm_service **tsp = NULL;
-	bool found = false;
 
 	/* it might already exist */
 	if (name) {
@@ -1386,14 +1385,12 @@ static int add_a_service(const struct loadparm_service *pservice, const char *na
 
 	/* Re use empty slots if any before allocating new one.*/
 	for (i=0; i < iNumServices; i++) {
-		if (!ServicePtrs[i]) {
-			found = true;
+		if (ServicePtrs[i] == NULL) {
 			break;
 		}
 	}
-	if (!found) {
+	if (i == iNumServices) {
 		/* if not, then create one */
-		i = iNumServices;
 		tsp = talloc_realloc(NULL, ServicePtrs,
 				     struct loadparm_service *,
 				     num_to_alloc);
-- 
1.7.9.5