[PATCH] ctdb-server: Update ctdb_start_revoke_ro_record mem handling

[Swen Schillig]

...and again, please review.

Thanks.

Cheers Swen
-------------- next part --------------
From e820a888ab59aae6cb73715eac13fed45a87792e Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Wed, 31 Jan 2018 13:06:30 +0100
Subject: [PATCH] ctdb-server: Update ctdb_start_revoke_ro_record mem handling

Use talloc_object_pool for repeating consecutive memory allocations.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/server/ctdb_call.c | 95 +++++++++++++++++++++++--------------------------
 1 file changed, 44 insertions(+), 51 deletions(-)

diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index 97f9850a726..76c6c3889d7 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -32,6 +32,7 @@
 #include "lib/util/samba_util.h"
 #include "lib/util/sys_rw.h"
 #include "lib/util/util_process.h"
+#include "lib/tevent/tevent_internal.h"
 
 #include "ctdb_private.h"
 #include "ctdb_client.h"
@@ -1819,97 +1820,89 @@ static int ctdb_revoke_all_delegations(struct ctdb_context *ctdb, struct ctdb_db
 }
 
 
-int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb, struct ctdb_db_context *ctdb_db, TDB_DATA key, struct ctdb_ltdb_header *header, TDB_DATA data)
+int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb,
+				struct ctdb_db_context *ctdb_db, TDB_DATA key,
+				struct ctdb_ltdb_header *header, TDB_DATA data)
 {
+	struct revokechild_handle *rev_hdl;
 	TDB_DATA tdata;
-	struct revokechild_handle *rc;
 	pid_t parent = getpid();
-	int ret;
 
-	header->flags &= ~(CTDB_REC_RO_REVOKING_READONLY|CTDB_REC_RO_HAVE_DELEGATIONS|CTDB_REC_RO_HAVE_READONLY);
+	header->flags &= ~(CTDB_REC_RO_REVOKING_READONLY |
+			   CTDB_REC_RO_HAVE_DELEGATIONS |
+			   CTDB_REC_RO_HAVE_READONLY);
 	header->flags |= CTDB_REC_FLAG_MIGRATED_WITH_DATA;
 	header->rsn   -= 1;
 
-	if ((rc = talloc_zero(ctdb_db, struct revokechild_handle)) == NULL) {
+	rev_hdl = talloc_pooled_object(ctdb_db, struct revokechild_handle, 2,
+				       sizeof(struct tevent_fd) + key.dsize);
+	if (rev_hdl == NULL) {
 		DEBUG(DEBUG_ERR,("Failed to allocate revokechild_handle\n"));
 		return -1;
 	}
 
 	tdata = tdb_fetch(ctdb_db->rottdb, key);
-	if (tdata.dsize > 0) {
-		uint8_t *tmp;
+	talloc_steal(rev_hdl, tdata.dptr);
 
-		tmp = tdata.dptr;
-		tdata.dptr = talloc_memdup(rc, tdata.dptr, tdata.dsize);
-		free(tmp);
-	}
+	rev_hdl->status  = 0;
+	rev_hdl->ctdb    = ctdb;
+	rev_hdl->ctdb_db = ctdb_db;
+	rev_hdl->fd[0]   = -1;
+	rev_hdl->fd[1]   = -1;
 
-	rc->status    = 0;
-	rc->ctdb      = ctdb;
-	rc->ctdb_db   = ctdb_db;
-	rc->fd[0]     = -1;
-	rc->fd[1]     = -1;
+	rev_hdl->key.dsize = key.dsize;
+	rev_hdl->key.dptr  = talloc_memdup(rev_hdl, key.dptr, key.dsize);
 
-	talloc_set_destructor(rc, revokechild_destructor);
-
-	rc->key.dsize = key.dsize;
-	rc->key.dptr  = talloc_memdup(rc, key.dptr, key.dsize);
-	if (rc->key.dptr == NULL) {
+	if ((rev_hdl->key.dptr == NULL) || (pipe(rev_hdl->fd) != 0)) {
 		DEBUG(DEBUG_ERR,("Failed to allocate key for revokechild_handle\n"));
-		talloc_free(rc);
-		return -1;
+		goto err_out;
 	}
 
-	ret = pipe(rc->fd);
-	if (ret != 0) {
-		DEBUG(DEBUG_ERR,("Failed to allocate key for revokechild_handle\n"));
-		talloc_free(rc);
-		return -1;
-	}
-
-
-	rc->child = ctdb_fork(ctdb);
-	if (rc->child == (pid_t)-1) {
+	rev_hdl->child = ctdb_fork(ctdb);
+	if (rev_hdl->child == (pid_t)-1) {
 		DEBUG(DEBUG_ERR,("Failed to fork child for revokechild\n"));
-		talloc_free(rc);
-		return -1;
+		goto err_out;
 	}
 
-	if (rc->child == 0) {
+	if (rev_hdl->child == 0) {
 		char c = 0;
-		close(rc->fd[0]);
+		close(rev_hdl->fd[0]);
 
 		prctl_set_comment("ctdb_revokechild");
 		if (switch_from_server_to_client(ctdb) != 0) {
 			DEBUG(DEBUG_ERR,("Failed to switch from server to client for revokechild process\n"));
 			c = 1;
-			goto child_finished;
+		} else {
+			c = ctdb_revoke_all_delegations(ctdb, ctdb_db, tdata,
+							key, header, data);
 		}
 
-		c = ctdb_revoke_all_delegations(ctdb, ctdb_db, tdata, key, header, data);
-
-child_finished:
-		sys_write(rc->fd[1], &c, 1);
+		sys_write(rev_hdl->fd[1], &c, 1);
 		ctdb_wait_for_process_to_exit(parent);
 		_exit(0);
 	}
 
-	close(rc->fd[1]);
-	rc->fd[1] = -1;
-	set_close_on_exec(rc->fd[0]);
+	close(rev_hdl->fd[1]);
+	rev_hdl->fd[1] = -1;
+	set_close_on_exec(rev_hdl->fd[0]);
 
 	/* This is an active revokechild child process */
-	DLIST_ADD_END(ctdb_db->revokechild_active, rc);
+	DLIST_ADD_END(ctdb_db->revokechild_active, rev_hdl);
+	talloc_set_destructor(rev_hdl, revokechild_destructor);
 
-	rc->fde = tevent_add_fd(ctdb->ev, rc, rc->fd[0], TEVENT_FD_READ,
-				revokechild_handler, (void *)rc);
-	if (rc->fde == NULL) {
+	rev_hdl->fde = tevent_add_fd(ctdb->ev, rev_hdl, rev_hdl->fd[0],
+				     TEVENT_FD_READ, revokechild_handler,
+				     (void *)rev_hdl);
+	if (rev_hdl->fde == NULL) {
 		DEBUG(DEBUG_ERR,("Failed to set up fd event for revokechild process\n"));
-		talloc_free(rc);
+		talloc_free(rev_hdl);
 	}
-	tevent_fd_set_auto_close(rc->fde);
+	tevent_fd_set_auto_close(rev_hdl->fde);
 
 	return 0;
+err_out:
+	talloc_free(rev_hdl);
+	return -1;
 }
 
 int ctdb_add_revoke_deferred_call(struct ctdb_context *ctdb, struct ctdb_db_context *ctdb_db, TDB_DATA key, struct ctdb_req_header *hdr, deferred_requeue_fn fn, void *call_context)
-- 
2.14.3