[PATCH] ctdb: calculate queue input buffer size correctly

[Swen Schillig]

Please review and push if happy.

Thanks in advance for your support.

Cheers Swen
-------------- next part --------------
From 7570afe2126110c78619eec1c893238c88cb650d Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Wed, 7 Mar 2018 13:54:45 +0100
Subject: [PATCH] ctdb: calculate queue input buffer size correctly

The queue's input buffer is calculated in an iterative way.
This can result in a few back and forth jumping and
a few memory allocations and mem-free cycles.
This is very time consuming and not required, because the required
memory size can be calculated right away.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/common/ctdb_io.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/ctdb/common/ctdb_io.c b/ctdb/common/ctdb_io.c
index c6941241883..d307d2b8bd8 100644
--- a/ctdb/common/ctdb_io.c
+++ b/ctdb/common/ctdb_io.c
@@ -43,7 +43,6 @@ struct ctdb_buffer {
 	uint8_t *data;
 	uint32_t length;
 	uint32_t size;
-	uint32_t extend;
 };
 
 struct ctdb_queue_pkt {
@@ -103,16 +102,15 @@ static void queue_process(struct ctdb_queue *queue)
 		return;
 	}
 
+	/* Did we at least read the size into the buffer */
 	pkt_size = *(uint32_t *)queue->buffer.data;
 	if (pkt_size == 0) {
 		DEBUG(DEBUG_CRIT, ("Invalid packet of length 0\n"));
 		goto failed;
 	}
 
+	/* the buffer doesn't contain the full packet, return to get the rest */
 	if (queue->buffer.length < pkt_size) {
-		if (pkt_size > queue->buffer_size) {
-			queue->buffer.extend = pkt_size;
-		}
 		return;
 	}
 
@@ -158,10 +156,10 @@ failed:
 */
 static void queue_io_read(struct ctdb_queue *queue)
 {
-	int num_ready = 0;
+	uint32_t num_ready;
+	uint32_t pkt_size;
 	ssize_t nread;
 	uint8_t *data;
-	int navail;
 
 	/* check how much data is available on the socket for immediately
 	   guaranteed nonblocking access.
@@ -185,22 +183,29 @@ static void queue_io_read(struct ctdb_queue *queue)
 			goto failed;
 		}
 		queue->buffer.size = queue->buffer_size;
-	} else if (queue->buffer.extend > 0) {
-		/* extending buffer */
-		data = talloc_realloc_size(queue, queue->buffer.data, queue->buffer.extend);
+		goto data_read;
+	}
+
+	if (queue->buffer.length < sizeof(pkt_size)) {
+		/* data read is not sufficient to gather message size */
+		goto data_read;
+	}
+
+	pkt_size = *(uint32_t *)queue->buffer.data;
+	if (pkt_size > queue->buffer.size) {
+		data = talloc_realloc_size(queue,
+					   queue->buffer.data,
+					   pkt_size);
 		if (data == NULL) {
-			DEBUG(DEBUG_ERR, ("read error realloc failed for %u\n", queue->buffer.extend));
+			DBG_ERR("read error realloc failed for %u\n", pkt_size);
 			goto failed;
 		}
 		queue->buffer.data = data;
-		queue->buffer.size = queue->buffer.extend;
-		queue->buffer.extend = 0;
+		queue->buffer.size = pkt_size;
 	}
 
-	navail = queue->buffer.size - queue->buffer.length;
-	if (num_ready > navail) {
-		num_ready = navail;
-	}
+data_read:
+	num_ready = MIN(num_ready, queue->buffer.size - queue->buffer.length);
 
 	if (num_ready > 0) {
 		nread = sys_read(queue->fd,
-- 
2.14.4