CTDB: Small patch set fixing memory leak and other things for ctdb_daemon_read_cb

[Swen Schillig]

The attached file contains 3 small patches fixing a memory leak,
removes not require checks and is cleaning up the code for column
length, function call formatting and using the right logging macros.

Please review and push if OK.

Thanks in advance.

Cheers Swen.
-------------- next part --------------
From 258b69321f262a67c931ac80fb43612e34d415c3 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Tue, 13 Mar 2018 08:57:40 +0100
Subject: [PATCH 1/3] ctdb: Remove double sanity checks from
 ctdb_daemon_read_cb

Within ctdb_daemon_read_cb the provided data is checked for sanity,
e.g. correct size and content. This is not required because it was
done already by the caller.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/server/ctdb_daemon.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c
index 35c1ab639b5..d67f47ac152 100644
--- a/ctdb/server/ctdb_daemon.c
+++ b/ctdb/server/ctdb_daemon.c
@@ -889,17 +889,7 @@ static void ctdb_daemon_read_cb(uint8_t *data, size_t cnt, void *args)
 
 	CTDB_INCREMENT_STAT(client->ctdb, client_packets_recv);
 
-	if (cnt < sizeof(*hdr)) {
-		ctdb_set_error(client->ctdb, "Bad packet length %u in daemon\n", 
-			       (unsigned)cnt);
-		return;
-	}
 	hdr = (struct ctdb_req_header *)data;
-	if (cnt != hdr->length) {
-		ctdb_set_error(client->ctdb, "Bad header length %u expected %u\n in daemon", 
-			       (unsigned)hdr->length, (unsigned)cnt);
-		return;
-	}
 
 	if (hdr->ctdb_magic != CTDB_MAGIC) {
 		ctdb_set_error(client->ctdb, "Non CTDB packet rejected\n");
-- 
2.14.3


From 6efe4a08953893fa6dad3e62b685e1f985141d00 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Tue, 13 Mar 2018 09:03:18 +0100
Subject: [PATCH 2/3] ctdb: Minor cleanup of ctdb_daemon_read_cb

Minor code cleanup of ctdb_daemon_read_cb adjusting for column length,
function call alignment and logging macros.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/server/ctdb_daemon.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c
index d67f47ac152..f1b76350acb 100644
--- a/ctdb/server/ctdb_daemon.c
+++ b/ctdb/server/ctdb_daemon.c
@@ -897,13 +897,18 @@ static void ctdb_daemon_read_cb(uint8_t *data, size_t cnt, void *args)
 	}
 
 	if (hdr->ctdb_version != CTDB_PROTOCOL) {
-		ctdb_set_error(client->ctdb, "Bad CTDB version 0x%x rejected in daemon\n", hdr->ctdb_version);
+		ctdb_set_error(client->ctdb,
+			       "Bad CTDB version 0x%x rejected in daemon\n",
+			       hdr->ctdb_version);
 		return;
 	}
 
-	DEBUG(DEBUG_DEBUG,(__location__ " client request %u of type %u length %u from "
-		 "node %u to %u\n", hdr->reqid, hdr->operation, hdr->length,
-		 hdr->srcnode, hdr->destnode));
+	D_DEBUG("client request %u of type %u length %u from node %u to %u\n",
+		hdr->reqid,
+		hdr->operation,
+		hdr->length,
+		hdr->srcnode,
+		hdr->destnode);
 
 	/* it is the responsibility of the incoming packet function to free 'data' */
 	daemon_incoming_packet(client, hdr);
-- 
2.14.3


From 6f2bc249e546ac55fc13bc5a8a0ba46d7e565b27 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Tue, 13 Mar 2018 09:06:45 +0100
Subject: [PATCH 3/3] ctdb: Fixing possible memory leak in ctdb_daemon_read_cb

In case of an error condition the further processing of the data is cancelled
and the callback returns. In such a scenario the data has to be free'd.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/server/ctdb_daemon.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c
index f1b76350acb..b42e2e35f8b 100644
--- a/ctdb/server/ctdb_daemon.c
+++ b/ctdb/server/ctdb_daemon.c
@@ -893,14 +893,14 @@ static void ctdb_daemon_read_cb(uint8_t *data, size_t cnt, void *args)
 
 	if (hdr->ctdb_magic != CTDB_MAGIC) {
 		ctdb_set_error(client->ctdb, "Non CTDB packet rejected\n");
-		return;
+		goto err_out;
 	}
 
 	if (hdr->ctdb_version != CTDB_PROTOCOL) {
 		ctdb_set_error(client->ctdb,
 			       "Bad CTDB version 0x%x rejected in daemon\n",
 			       hdr->ctdb_version);
-		return;
+		goto err_out;
 	}
 
 	D_DEBUG("client request %u of type %u length %u from node %u to %u\n",
@@ -912,6 +912,10 @@ static void ctdb_daemon_read_cb(uint8_t *data, size_t cnt, void *args)
 
 	/* it is the responsibility of the incoming packet function to free 'data' */
 	daemon_incoming_packet(client, hdr);
+	return;
+
+err_out:
+	TALLOC_FREE(data);
 }
 
 
-- 
2.14.3