[PATCH] Fix wrong talloc context, remove queue_destructor and minor updates.

[Swen Schillig]

Hi Martin
On Tue, 2017-12-12 at 15:37 +1100, Martin Schwenke wrote:
> Hi Swen,
> 
Thanks a lot for quick reply and the provided guidance.
> Comments:
> 
> * I don't think removing the talloc context argument is valid.  There
>   is no guarantee that private_data points to a talloc context, so 
you're right,
even though we might end up with a lot of other problems then.
Anyway, I removed that change.
> 
> * In the commit message for the destructor removal you could point to
>   the commit that contains the removal of the only use of the
>   "destroyed" structure member, which was really the only reason for
>   the existence of the destructor.
Hmm, if I understand it correctly, then it was wrong even back then
when there was a consumer of "destroyed". Because right after calling
the destructor that attribute would even have been free'd as well,
right ?

I've split it all up and hopefully explained my intention.

I would really appreciate if you (and everybody else of course) could
have a look again.

Cheers Swen
-------------- next part --------------
From d188d0319190439b71c84d26d35af0b214d3aa96 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Tue, 12 Dec 2017 09:59:11 +0100
Subject: [PATCH 1/3] Fix wrong talloc context.

Fixing the parent context of queue->name to queue.

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

diff --git a/ctdb/common/ctdb_io.c b/ctdb/common/ctdb_io.c
index 3e732e8..757cd1f 100644
--- a/ctdb/common/ctdb_io.c
+++ b/ctdb/common/ctdb_io.c
@@ -436,8 +436,9 @@ struct ctdb_queue *ctdb_queue_setup(struct ctdb_context *ctdb,
 
 	queue = talloc_zero(mem_ctx, struct ctdb_queue);
 	CTDB_NO_MEMORY_NULL(ctdb, queue);
+
 	va_start(ap, fmt);
-	queue->name = talloc_vasprintf(mem_ctx, fmt, ap);
+	queue->name = talloc_vasprintf(queue, fmt, ap);
 	va_end(ap);
 	CTDB_NO_MEMORY_NULL(ctdb, queue->name);
 
-- 
2.9.5


From 207dd0e143f4ccf47a0a5c4e82b9d7e7bb03d2fe Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Tue, 12 Dec 2017 10:35:17 +0100
Subject: [PATCH 2/3] Removed queue_destructor.

Remove queue_destructor() as it is not required to free "child-memory"
or set queue attributes just before the queue structure is free'd.

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

diff --git a/ctdb/common/ctdb_io.c b/ctdb/common/ctdb_io.c
index 757cd1f..a7ec8db 100644
--- a/ctdb/common/ctdb_io.c
+++ b/ctdb/common/ctdb_io.c
@@ -412,17 +412,6 @@ int ctdb_queue_set_fd(struct ctdb_queue *queue, int fd)
 	return 0;
 }
 
-/* If someone sets up this pointer, they want to know if the queue is freed */
-static int queue_destructor(struct ctdb_queue *queue)
-{
-	TALLOC_FREE(queue->buffer.data);
-	queue->buffer.length = 0;
-	queue->buffer.size = 0;
-	if (queue->destroyed != NULL)
-		*queue->destroyed = true;
-	return 0;
-}
-
 /*
   setup a packet queue on a socket
  */
@@ -456,7 +445,6 @@ struct ctdb_queue *ctdb_queue_setup(struct ctdb_context *ctdb,
 			return NULL;
 		}
 	}
-	talloc_set_destructor(queue, queue_destructor);
 
 	queue->buffer_size = ctdb->tunable.queue_buffer_size;
 	/* In client code, ctdb->tunable is not initialized.
-- 
2.9.5


From 08366244f4913a9957c3917311d2e540b63a2173 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Tue, 12 Dec 2017 11:01:59 +0100
Subject: [PATCH 3/3] Remove double check and double assignments.

There's no need to perform assignments which are performed again
in the almost always called function ctdb_queue_set_fd().

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

diff --git a/ctdb/common/ctdb_io.c b/ctdb/common/ctdb_io.c
index a7ec8db..2b43287 100644
--- a/ctdb/common/ctdb_io.c
+++ b/ctdb/common/ctdb_io.c
@@ -435,15 +435,13 @@ struct ctdb_queue *ctdb_queue_setup(struct ctdb_context *ctdb,
 	CTDB_NO_MEMORY_NULL(ctdb, queue->im);
 
 	queue->ctdb = ctdb;
-	queue->fd = fd;
 	queue->alignment = alignment;
 	queue->private_data = private_data;
 	queue->callback = callback;
-	if (fd != -1) {
-		if (ctdb_queue_set_fd(queue, fd) != 0) {
-			talloc_free(queue);
-			return NULL;
-		}
+
+	if (ctdb_queue_set_fd(queue, fd) != 0) {
+		talloc_free(queue);
+		return NULL;
 	}
 
 	queue->buffer_size = ctdb->tunable.queue_buffer_size;
-- 
2.9.5