[PATCH] Fix wrong talloc context, remove queue_destructor and minor updates.
Swen Schillig
swen at vnet.ibm.com
Tue Dec 12 10:18:30 UTC 2017
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
More information about the samba-technical
mailing list