Attachment 13099 Details for Bug 12705 – initial patch as sent the the mailing list (needs bug ids and other fixes)

[patch] initial patch as sent the the mailing list (needs bug ids and other fixes)

auth-log-messaging.patch (text/plain), 29.33 KB, created by Andrew Bartlett on 2017-03-20 22:59:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2017-03-20 22:59:27 UTC

Size: 29.33 KB

patch

obsolete

>From 48ec03952ab0663257a846838272ba213683ee6a Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Wed, 8 Mar 2017 14:53:26 +1300
>Subject: [PATCH 1/7] pymessaging: Add support for irpc_add_name
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Pair-Programmed-by: Gary Lockyer <gary@catalyst.net.nz>
>Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
>---
> python/samba/tests/messaging.py     |  4 ++++
> source4/lib/messaging/pymessaging.c | 23 ++++++++++++++++++++++-
> 2 files changed, 26 insertions(+), 1 deletion(-)
>
>diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py
>index 5d32d60..1c5dfe5 100644
>--- a/python/samba/tests/messaging.py
>+++ b/python/samba/tests/messaging.py
>@@ -49,6 +49,10 @@ class MessagingTests(TestCase):
>         x = self.get_context()
>         self.assertTrue(isinstance(x.server_id, server_id))
> 
>+    def test_add_name(self):
>+        x = self.get_context()
>+        x.irpc_add_name("samba.messaging test")
>+
>     def test_ping_speed(self):
>         server_ctx = self.get_context((0, 1))
>         def ping_callback(src, data):
>diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c
>index f62354b..9d0997f 100644
>--- a/source4/lib/messaging/pymessaging.c
>+++ b/source4/lib/messaging/pymessaging.c
>@@ -241,6 +241,25 @@ static PyObject *py_imessaging_deregister(PyObject *self, PyObject *args, PyObje
> 	Py_RETURN_NONE;
> }
> 
>+static PyObject *py_irpc_add_name(PyObject *self, PyObject *args, PyObject *kwargs)
>+{
>+	imessaging_Object *iface = (imessaging_Object *)self;
>+	char *server_name;
>+	NTSTATUS status;
>+
>+	if (!PyArg_ParseTuple(args, "s", &server_name)) {
>+		return NULL;
>+	}
>+
>+	status = irpc_add_name(iface->msg_ctx, server_name);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		PyErr_SetNTSTATUS(status);
>+		return NULL;
>+	}
>+
>+	Py_RETURN_NONE;
>+}
>+
> static PyObject *py_irpc_servers_byname(PyObject *self, PyObject *args, PyObject *kwargs)
> {
> 	imessaging_Object *iface = (imessaging_Object *)self;
>@@ -341,10 +360,12 @@ static PyMethodDef py_imessaging_methods[] = {
> 		"S.register(callback, msg_type=None) -> msg_type\nRegister a message handler" },
> 	{ "deregister", (PyCFunction)py_imessaging_deregister, METH_VARARGS|METH_KEYWORDS,
> 		"S.deregister(callback, msg_type) -> None\nDeregister a message handler" },
>+	{ "irpc_add_name", (PyCFunction)py_irpc_add_name, METH_VARARGS,
>+		"S.irpc_add_name(name) -> None\nAdd this context to the list of server_id values that are registered for a particular name" },
> 	{ "irpc_servers_byname", (PyCFunction)py_irpc_servers_byname, METH_VARARGS,
> 		"S.irpc_servers_byname(name) -> list\nGet list of server_id values that are registered for a particular name" },
> 	{ "irpc_all_servers", (PyCFunction)py_irpc_all_servers, METH_NOARGS,
>-		"S.irpc_servers_byname() -> list\nGet list of all registered names and the associated server_id values" },
>+		"S.irpc_all_servers() -> list\nGet list of all registered names and the associated server_id values" },
> 	{ NULL, NULL, 0, NULL }
> };
> 
>-- 
>2.9.3
>
>
>From 461fc025fba11d3ff6b723b8aa421c7d94083ce7 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 14 Mar 2017 13:39:00 +1300
>Subject: [PATCH 2/7] pymessaging: Add irpc_remove_name
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> python/samba/tests/messaging.py     |  8 ++++++++
> source4/lib/messaging/pymessaging.c | 16 ++++++++++++++++
> 2 files changed, 24 insertions(+)
>
>diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py
>index 1c5dfe5..3eeab52 100644
>--- a/python/samba/tests/messaging.py
>+++ b/python/samba/tests/messaging.py
>@@ -22,6 +22,7 @@ import samba
> from samba.messaging import Messaging
> from samba.tests import TestCase
> from samba.dcerpc.server_id import server_id
>+from samba.ndr import ndr_print
> 
> class MessagingTests(TestCase):
> 
>@@ -52,6 +53,13 @@ class MessagingTests(TestCase):
>     def test_add_name(self):
>         x = self.get_context()
>         x.irpc_add_name("samba.messaging test")
>+        name_list = x.irpc_servers_byname("samba.messaging test")
>+        self.assertEqual(len(name_list), 1)
>+        self.assertEqual(ndr_print(x.server_id),
>+                         ndr_print(name_list[0]))
>+        x.irpc_remove_name("samba.messaging test")
>+        self.assertEqual([],
>+                         x.irpc_servers_byname("samba.messaging test"))
> 
>     def test_ping_speed(self):
>         server_ctx = self.get_context((0, 1))
>diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c
>index 9d0997f..b317955 100644
>--- a/source4/lib/messaging/pymessaging.c
>+++ b/source4/lib/messaging/pymessaging.c
>@@ -260,6 +260,20 @@ static PyObject *py_irpc_add_name(PyObject *self, PyObject *args, PyObject *kwar
> 	Py_RETURN_NONE;
> }
> 
>+static PyObject *py_irpc_remove_name(PyObject *self, PyObject *args, PyObject *kwargs)
>+{
>+	imessaging_Object *iface = (imessaging_Object *)self;
>+	char *server_name;
>+
>+	if (!PyArg_ParseTuple(args, "s", &server_name)) {
>+		return NULL;
>+	}
>+
>+	irpc_remove_name(iface->msg_ctx, server_name);
>+
>+	Py_RETURN_NONE;
>+}
>+
> static PyObject *py_irpc_servers_byname(PyObject *self, PyObject *args, PyObject *kwargs)
> {
> 	imessaging_Object *iface = (imessaging_Object *)self;
>@@ -362,6 +376,8 @@ static PyMethodDef py_imessaging_methods[] = {
> 		"S.deregister(callback, msg_type) -> None\nDeregister a message handler" },
> 	{ "irpc_add_name", (PyCFunction)py_irpc_add_name, METH_VARARGS,
> 		"S.irpc_add_name(name) -> None\nAdd this context to the list of server_id values that are registered for a particular name" },
>+	{ "irpc_remove_name", (PyCFunction)py_irpc_remove_name, METH_VARARGS,
>+		"S.irpc_remove_name(name) -> None\nAdd this context to the list of server_id values that are registered for a particular name" },
> 	{ "irpc_servers_byname", (PyCFunction)py_irpc_servers_byname, METH_VARARGS,
> 		"S.irpc_servers_byname(name) -> list\nGet list of server_id values that are registered for a particular name" },
> 	{ "irpc_all_servers", (PyCFunction)py_irpc_all_servers, METH_NOARGS,
>-- 
>2.9.3
>
>
>From c540d344b987f688c8fbdcbef87c85b0c8cd525f Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 14 Mar 2017 16:07:46 +1300
>Subject: [PATCH 3/7] selftest: Test server_id database add and removal
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> python/samba/tests/messaging.py | 19 +++++++++++++------
> selftest/knownfail              |  1 +
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
>diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py
>index 3eeab52..a70be96 100644
>--- a/python/samba/tests/messaging.py
>+++ b/python/samba/tests/messaging.py
>@@ -23,6 +23,7 @@ from samba.messaging import Messaging
> from samba.tests import TestCase
> from samba.dcerpc.server_id import server_id
> from samba.ndr import ndr_print
>+import random
> 
> class MessagingTests(TestCase):
> 
>@@ -46,20 +47,26 @@ class MessagingTests(TestCase):
>         for name in x.irpc_all_servers():
>             self.assertTrue(isinstance(x.irpc_servers_byname(name.name), list))
> 
>+    def test_unknown_name(self):
>+        x = self.get_context()
>+        self.assertRaises(KeyError,
>+                          x.irpc_servers_byname, "samba.messaging test NONEXISTING")
>+
>     def test_assign_server_id(self):
>         x = self.get_context()
>         self.assertTrue(isinstance(x.server_id, server_id))
> 
>-    def test_add_name(self):
>+    def test_add_remove_name(self):
>         x = self.get_context()
>-        x.irpc_add_name("samba.messaging test")
>-        name_list = x.irpc_servers_byname("samba.messaging test")
>+        name = "samba.messaging test-%d" % random.randint(1, 1000000)
>+        x.irpc_add_name(name)
>+        name_list = x.irpc_servers_byname(name)
>         self.assertEqual(len(name_list), 1)
>         self.assertEqual(ndr_print(x.server_id),
>                          ndr_print(name_list[0]))
>-        x.irpc_remove_name("samba.messaging test")
>-        self.assertEqual([],
>-                         x.irpc_servers_byname("samba.messaging test"))
>+        x.irpc_remove_name(name)
>+        self.assertRaises(KeyError,
>+                          x.irpc_servers_byname, name)
> 
>     def test_ping_speed(self):
>         server_ctx = self.get_context((0, 1))
>diff --git a/selftest/knownfail b/selftest/knownfail
>index cfd4b35..2f3b22b 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -317,3 +317,4 @@
> ^samba3.smb2.credits.skipped_mid.*
> ^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_dbcheck
> ^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_check_missing
>+^samba.tests.messaging.samba.tests.messaging.MessagingTests.test_add_remove_name
>\ No newline at end of file
>-- 
>2.9.3
>
>
>From eef04949817bab65aa3c3268cf8690e7753e174e Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 14 Mar 2017 15:22:01 +1300
>Subject: [PATCH 4/7] lib/util: Do not return an unterminated pointer in
> tdb_fetch_talloc()
>
>Otherwise, if a TDB entry is truncated to 0 length, this will return
>uninitialised memory!
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/util/server_id_db.c | 42 +++++++++++++++++++++++++++++++++---------
> lib/util/util_tdb.c     |  9 +++++----
> lib/util/util_tdb.h     |  2 +-
> selftest/knownfail      |  1 -
> 4 files changed, 39 insertions(+), 15 deletions(-)
>
>diff --git a/lib/util/server_id_db.c b/lib/util/server_id_db.c
>index e0b8476..937de89 100644
>--- a/lib/util/server_id_db.c
>+++ b/lib/util/server_id_db.c
>@@ -137,7 +137,8 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name,
> 	size_t idbuf_len = server_id_str_buf_unique(server, NULL, 0);
> 	char idbuf[idbuf_len];
> 	TDB_DATA key;
>-	uint8_t *data;
>+	TDB_DATA data;
>+	TDB_DATA data_store;
> 	char *ids, *id;
> 	int ret;
> 
>@@ -156,18 +157,32 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name,
> 		return ret;
> 	}
> 
>-	ids = (char *)data;
>+	if (data.dsize == 0) {
>+		tdb_chainunlock(tdb, key);
>+		TALLOC_FREE(data.dptr);
>+		return ENOENT;
>+	}
>+
>+	/* We assert that the DB contains a NULL-terminated string */
>+	ids = (char *)data.dptr;
> 
> 	id = strv_find(ids, idbuf);
> 	if (id == NULL) {
> 		tdb_chainunlock(tdb, key);
>-		TALLOC_FREE(data);
>+		TALLOC_FREE(data.dptr);
> 		return ENOENT;
> 	}
> 
> 	strv_delete(&ids, id);
>-	ret = tdb_store(tdb, key, talloc_tdb_data(ids), TDB_MODIFY);
>-	TALLOC_FREE(data);
>+
>+	data_store = talloc_tdb_data(ids);
>+
>+	if (data_store.dsize == 0) {
>+		ret = tdb_delete(tdb, key);
>+	} else {
>+		ret = tdb_store(tdb, key, data_store, TDB_MODIFY);
>+	}
>+	TALLOC_FREE(data.dptr);
> 
> 	tdb_chainunlock(tdb, key);
> 
>@@ -199,7 +214,7 @@ int server_id_db_lookup(struct server_id_db *db, const char *name,
> {
> 	struct tdb_context *tdb = db->tdb->tdb;
> 	TDB_DATA key;
>-	uint8_t *data;
>+	TDB_DATA data;
> 	char *ids, *id;
> 	unsigned num_servers;
> 	struct server_id *servers;
>@@ -212,12 +227,17 @@ int server_id_db_lookup(struct server_id_db *db, const char *name,
> 		return ret;
> 	}
> 
>-	ids = (char *)data;
>+	if (data.dsize == 0) {
>+		return ENOENT;
>+	}
>+
>+	/* We assert that the DB contains a NULL-terminated string */
>+	ids = (char *)data.dptr;
> 	num_servers = strv_count(ids);
> 
> 	servers = talloc_array(mem_ctx, struct server_id, num_servers);
> 	if (servers == NULL) {
>-		TALLOC_FREE(data);
>+		TALLOC_FREE(data.dptr);
> 		return ENOMEM;
> 	}
> 
>@@ -227,7 +247,7 @@ int server_id_db_lookup(struct server_id_db *db, const char *name,
> 		servers[i++] = server_id_from_string(NONCLUSTER_VNN, id);
> 	}
> 
>-	TALLOC_FREE(data);
>+	TALLOC_FREE(data.dptr);
> 
> 	*pnum_servers = num_servers;
> 	*pservers = servers;
>@@ -283,6 +303,10 @@ static int server_id_db_traverse_fn(struct tdb_context *tdb,
> 	}
> 	name = (const char *)key.dptr;
> 
>+	if (data.dsize == 0) {
>+		return 0;
>+	}
>+
> 	ids = (char *)talloc_memdup(state->mem_ctx, data.dptr, data.dsize);
> 	if (ids == NULL) {
> 		return 0;
>diff --git a/lib/util/util_tdb.c b/lib/util/util_tdb.c
>index 9bf18dc..8ff89ff 100644
>--- a/lib/util/util_tdb.c
>+++ b/lib/util/util_tdb.c
>@@ -486,19 +486,20 @@ int map_unix_error_from_tdb(enum TDB_ERROR err)
> 
> struct tdb_fetch_talloc_state {
> 	TALLOC_CTX *mem_ctx;
>-	uint8_t *buf;
>+	TDB_DATA buf;
> };
> 
> static int tdb_fetch_talloc_parser(TDB_DATA key, TDB_DATA data,
>                                    void *private_data)
> {
> 	struct tdb_fetch_talloc_state *state = private_data;
>-	state->buf = talloc_memdup(state->mem_ctx, data.dptr, data.dsize);
>+	state->buf.dptr = talloc_memdup(state->mem_ctx, data.dptr, data.dsize);
>+	state->buf.dsize = data.dsize;
> 	return 0;
> }
> 
> int tdb_fetch_talloc(struct tdb_context *tdb, TDB_DATA key,
>-		     TALLOC_CTX *mem_ctx, uint8_t **buf)
>+		     TALLOC_CTX *mem_ctx, TDB_DATA *buf)
> {
> 	struct tdb_fetch_talloc_state state = { .mem_ctx = mem_ctx };
> 	int ret;
>@@ -509,7 +510,7 @@ int tdb_fetch_talloc(struct tdb_context *tdb, TDB_DATA key,
> 		return map_unix_error_from_tdb(err);
> 	}
> 
>-	if (state.buf == NULL) {
>+	if (state.buf.dptr == NULL && state.buf.dsize != 0) {
> 		return ENOMEM;
> 	}
> 
>diff --git a/lib/util/util_tdb.h b/lib/util/util_tdb.h
>index 3b50789..1f56341 100644
>--- a/lib/util/util_tdb.h
>+++ b/lib/util/util_tdb.h
>@@ -146,6 +146,6 @@ NTSTATUS map_nt_error_from_tdb(enum TDB_ERROR err);
> int map_unix_error_from_tdb(enum TDB_ERROR err);
> 
> int tdb_fetch_talloc(struct tdb_context *tdb, TDB_DATA key,
>-		     TALLOC_CTX *mem_ctx, uint8_t **buf);
>+		     TALLOC_CTX *mem_ctx, TDB_DATA *buf);
> 
> #endif /* _____LIB_UTIL_UTIL_TDB_H__ */
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 2f3b22b..cfd4b35 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -317,4 +317,3 @@
> ^samba3.smb2.credits.skipped_mid.*
> ^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_dbcheck
> ^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_check_missing
>-^samba.tests.messaging.samba.tests.messaging.MessagingTests.test_add_remove_name
>\ No newline at end of file
>-- 
>2.9.3
>
>
>From fd22a3fdbd6e3561f76dbad05730c78f7500a135 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 20 Mar 2017 14:57:41 +1300
>Subject: [PATCH 5/7] server_id: Add runtime check for null termination
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/util/server_id_db.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/lib/util/server_id_db.c b/lib/util/server_id_db.c
>index 937de89..8aeda98 100644
>--- a/lib/util/server_id_db.c
>+++ b/lib/util/server_id_db.c
>@@ -164,6 +164,10 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name,
> 	}
> 
> 	/* We assert that the DB contains a NULL-terminated string */
>+	if (data.dptr[data.dsize - 1] != '\0') {
>+		return EINVAL;
>+	}
>+
> 	ids = (char *)data.dptr;
> 
> 	id = strv_find(ids, idbuf);
>@@ -232,6 +236,10 @@ int server_id_db_lookup(struct server_id_db *db, const char *name,
> 	}
> 
> 	/* We assert that the DB contains a NULL-terminated string */
>+	if (data.dptr[data.dsize - 1] != '\0') {
>+		return EINVAL;
>+	}
>+
> 	ids = (char *)data.dptr;
> 	num_servers = strv_count(ids);
> 
>-- 
>2.9.3
>
>
>From a78ec79a012c4b0e8face1efa4b043180b26e001 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 14 Mar 2017 12:39:13 +1300
>Subject: [PATCH 6/7] pymessaging: Add a hook to run the event loop, make
> callbacks practical
>
>These change allow us to write a messaging server in python.
>
>The previous ping_speed test did not actually test anything, so
>we use .loop_once() to make it actually work.  To enable practial use
>a context is supplied in the tuple with the callback, and the server_id
>for the reply is not placed inside an additional tuple.
>
>In order to get at the internal event context on which to loop, we
>expose imessaging_context in messaging_internal.h and allow the python
>bindings to use that header.
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> python/samba/tests/messaging.py            | 52 +++++++++++++------
> source4/lib/messaging/messaging.c          | 17 +------
> source4/lib/messaging/messaging_internal.h | 36 ++++++++++++++
> source4/lib/messaging/pymessaging.c        | 80 +++++++++++++++++++++++++++---
> 4 files changed, 147 insertions(+), 38 deletions(-)
> create mode 100644 source4/lib/messaging/messaging_internal.h
>
>diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py
>index a70be96..6ee18e7 100644
>--- a/python/samba/tests/messaging.py
>+++ b/python/samba/tests/messaging.py
>@@ -21,8 +21,9 @@
> import samba
> from samba.messaging import Messaging
> from samba.tests import TestCase
>-from samba.dcerpc.server_id import server_id
>+import time
> from samba.ndr import ndr_print
>+from samba.dcerpc import server_id
> import random
> 
> class MessagingTests(TestCase):
>@@ -35,7 +36,8 @@ class MessagingTests(TestCase):
>         x = self.get_context()
>         def callback():
>             pass
>-        msg_type = x.register(callback)
>+        msg_type = x.register((callback, None))
>+        self.assertTrue(isinstance(msg_type, long))
>         x.deregister(callback, msg_type)
> 
>     def test_all_servers(self):
>@@ -54,7 +56,7 @@ class MessagingTests(TestCase):
> 
>     def test_assign_server_id(self):
>         x = self.get_context()
>-        self.assertTrue(isinstance(x.server_id, server_id))
>+        self.assertTrue(isinstance(x.server_id, server_id.server_id))
> 
>     def test_add_remove_name(self):
>         x = self.get_context()
>@@ -69,19 +71,41 @@ class MessagingTests(TestCase):
>                           x.irpc_servers_byname, name)
> 
>     def test_ping_speed(self):
>+        got_ping = {"count": 0}
>+        got_pong = {"count": 0}
>+        timeout = False
>+
>+        msg_pong = 0
>+        msg_ping = 0
>+
>         server_ctx = self.get_context((0, 1))
>-        def ping_callback(src, data):
>-                server_ctx.send(src, data)
>-        def exit_callback():
>-                print "received exit"
>-        msg_ping = server_ctx.register(ping_callback)
>-        msg_exit = server_ctx.register(exit_callback)
>-
>-        def pong_callback():
>-                print "received pong"
>+        def ping_callback(got_ping, msg_type, src, data):
>+            got_ping["count"] += 1
>+            server_ctx.send(src, msg_pong, data)
>+
>+        msg_ping = server_ctx.register((ping_callback, got_ping))
>+
>+        def pong_callback(got_pong, msg_type, src, data):
>+            got_pong["count"] += 1
>+
>         client_ctx = self.get_context((0, 2))
>-        msg_pong = client_ctx.register(pong_callback)
>+        msg_pong = client_ctx.register((pong_callback, got_pong))
> 
>+        # Try both server_id forms (structure and tuple)
>         client_ctx.send((0, 1), msg_ping, "testing")
>-        client_ctx.send((0, 1), msg_ping, "")
> 
>+        client_ctx.send((0, 1), msg_ping, "testing2")
>+
>+        start_time = time.time()
>+
>+        # NOTE WELL: If debugging this with GDB, then the timeout will
>+        # fire while you are trying to understand it.
>+
>+        while (got_ping["count"] < 2 or got_pong["count"] < 2) and not timeout:
>+            client_ctx.loop_once(0.1)
>+            server_ctx.loop_once(0.1)
>+            if time.time() - start_time > 1:
>+                timeout = True
>+
>+        self.assertEqual(got_ping["count"], 2)
>+        self.assertEqual(got_pong["count"], 2)
>diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c
>index 84df934..4d75f09 100644
>--- a/source4/lib/messaging/messaging.c
>+++ b/source4/lib/messaging/messaging.c
>@@ -24,6 +24,7 @@
> #include "lib/util/server_id.h"
> #include "system/filesys.h"
> #include "messaging/messaging.h"
>+#include "messaging/messaging_internal.h"
> #include "../lib/util/dlinklist.h"
> #include "lib/socket/socket.h"
> #include "librpc/gen_ndr/ndr_irpc.h"
>@@ -55,22 +56,6 @@ struct irpc_request {
> 	} incoming;
> };
> 
>-struct imessaging_context {
>-	struct imessaging_context *prev, *next;
>-	struct tevent_context *ev;
>-	struct server_id server_id;
>-	const char *sock_dir;
>-	const char *lock_dir;
>-	struct dispatch_fn **dispatch;
>-	uint32_t num_types;
>-	struct idr_context *dispatch_tree;
>-	struct irpc_list *irpc;
>-	struct idr_context *idr;
>-	struct server_id_db *names;
>-	struct timeval start_time;
>-	void *msg_dgm_ref;
>-};
>-
> /* we have a linked list of dispatch handlers for each msg_type that
>    this messaging server can deal with */
> struct dispatch_fn {
>diff --git a/source4/lib/messaging/messaging_internal.h b/source4/lib/messaging/messaging_internal.h
>new file mode 100644
>index 0000000..93c5c4b
>--- /dev/null
>+++ b/source4/lib/messaging/messaging_internal.h
>@@ -0,0 +1,36 @@
>+/*
>+   Unix SMB/CIFS implementation.
>+
>+   Samba internal messaging functions
>+
>+   Copyright (C) Andrew Tridgell 2004
>+
>+   This program is free software; you can redistribute it and/or modify
>+   it under the terms of the GNU General Public License as published by
>+   the Free Software Foundation; either version 3 of the License, or
>+   (at your option) any later version.
>+
>+   This program is distributed in the hope that it will be useful,
>+   but WITHOUT ANY WARRANTY; without even the implied warranty of
>+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+   GNU General Public License for more details.
>+
>+   You should have received a copy of the GNU General Public License
>+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
>+*/
>+
>+struct imessaging_context {
>+	struct imessaging_context *prev, *next;
>+	struct tevent_context *ev;
>+	struct server_id server_id;
>+	const char *sock_dir;
>+	const char *lock_dir;
>+	struct dispatch_fn **dispatch;
>+	uint32_t num_types;
>+	struct idr_context *dispatch_tree;
>+	struct irpc_list *irpc;
>+	struct idr_context *idr;
>+	struct server_id_db *names;
>+	struct timeval start_time;
>+	void *msg_dgm_ref;
>+};
>diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c
>index b317955..84ceff5 100644
>--- a/source4/lib/messaging/pymessaging.c
>+++ b/source4/lib/messaging/pymessaging.c
>@@ -34,6 +34,7 @@
> #include "librpc/rpc/dcerpc.h"
> #include "librpc/gen_ndr/server_id.h"
> #include <pytalloc.h>
>+#include "messaging_internal.h"
> 
> void initmessaging(void);
> 
>@@ -173,7 +174,8 @@ static void py_msg_callback_wrapper(struct imessaging_context *msg, void *privat
> 			       uint32_t msg_type, 
> 			       struct server_id server_id, DATA_BLOB *data)
> {
>-	PyObject *py_server_id, *callback = (PyObject *)private_data;
>+	PyObject *py_server_id, *callback_and_tuple = (PyObject *)private_data;
>+	PyObject *callback, *py_private;
> 
> 	struct server_id *p_server_id = talloc(NULL, struct server_id);
> 	if (!p_server_id) {
>@@ -182,10 +184,18 @@ static void py_msg_callback_wrapper(struct imessaging_context *msg, void *privat
> 	}
> 	*p_server_id = server_id;
> 
>+	if (!PyArg_ParseTuple(callback_and_tuple, "OO",
>+			      &callback,
>+			      &py_private)) {
>+		return;
>+	}
>+
> 	py_server_id = py_return_ndr_struct("samba.dcerpc.server_id", "server_id", p_server_id, p_server_id);
> 	talloc_unlink(NULL, p_server_id);
> 
>-	PyObject_CallFunction(callback, discard_const_p(char, "i(O)s#"), msg_type,
>+	PyObject_CallFunction(callback, discard_const_p(char, "OiOs#"),
>+			      py_private,
>+			      msg_type,
> 			      py_server_id,
> 			      data->data, data->length);
> }
>@@ -194,24 +204,30 @@ static PyObject *py_imessaging_register(PyObject *self, PyObject *args, PyObject
> {
> 	imessaging_Object *iface = (imessaging_Object *)self;
> 	int msg_type = -1;
>-	PyObject *callback;
>+	PyObject *callback_and_context;
> 	NTSTATUS status;
>-	const char *kwnames[] = { "callback", "msg_type", NULL };
>+	const char *kwnames[] = { "callback_and_context", "msg_type", NULL };
> 	
> 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O|i:register", 
>-		discard_const_p(char *, kwnames), &callback, &msg_type)) {
>+		discard_const_p(char *, kwnames),
>+					 &callback_and_context, &msg_type)) {
>+		return NULL;
>+	}
>+	if (!PyTuple_Check(callback_and_context)
>+	    || PyTuple_Size(callback_and_context) != 2) {
>+		PyErr_SetString(PyExc_ValueError, "Expected of size 2 for callback_and_context");
> 		return NULL;
> 	}
> 
>-	Py_INCREF(callback);
>+	Py_INCREF(callback_and_context);
> 
> 	if (msg_type == -1) {
> 		uint32_t msg_type32 = msg_type;
>-		status = imessaging_register_tmp(iface->msg_ctx, callback,
>+		status = imessaging_register_tmp(iface->msg_ctx, callback_and_context,
> 						py_msg_callback_wrapper, &msg_type32);
> 		msg_type = msg_type32;
> 	} else {
>-		status = imessaging_register(iface->msg_ctx, callback,
>+		status = imessaging_register(iface->msg_ctx, callback_and_context,
> 				    msg_type, py_msg_callback_wrapper);
> 	}
> 	if (NT_STATUS_IS_ERR(status)) {
>@@ -241,6 +257,52 @@ static PyObject *py_imessaging_deregister(PyObject *self, PyObject *args, PyObje
> 	Py_RETURN_NONE;
> }
> 
>+static void simple_timer_handler(struct tevent_context *ev,
>+				 struct tevent_timer *te,
>+				 struct timeval current_time,
>+				 void *private_data)
>+{
>+	return;
>+}
>+
>+static PyObject *py_imessaging_loop_once(PyObject *self, PyObject *args, PyObject *kwargs)
>+{
>+	imessaging_Object *iface = (imessaging_Object *)self;
>+	double offset;
>+	int seconds;
>+	struct timeval next_event;
>+	struct tevent_timer *timer = NULL;
>+	const char *kwnames[] = { "timeout", NULL };
>+
>+	TALLOC_CTX *frame = talloc_stackframe();
>+
>+	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "d",
>+					 discard_const_p(char *, kwnames), &offset)) {
>+		TALLOC_FREE(frame);
>+		return NULL;
>+	}
>+
>+	if (offset != 0.0) {
>+		seconds = offset;
>+		offset -= seconds;
>+		next_event = tevent_timeval_current_ofs(seconds, (int)(offset*1000000));
>+
>+		timer = tevent_add_timer(iface->msg_ctx->ev, frame, next_event, simple_timer_handler,
>+					 NULL);
>+		if (timer == NULL) {
>+			PyErr_NoMemory();
>+			TALLOC_FREE(frame);
>+			return NULL;
>+		}
>+	}
>+
>+	tevent_loop_once(iface->msg_ctx->ev);
>+
>+	TALLOC_FREE(frame);
>+
>+	Py_RETURN_NONE;
>+}
>+
> static PyObject *py_irpc_add_name(PyObject *self, PyObject *args, PyObject *kwargs)
> {
> 	imessaging_Object *iface = (imessaging_Object *)self;
>@@ -374,6 +436,8 @@ static PyMethodDef py_imessaging_methods[] = {
> 		"S.register(callback, msg_type=None) -> msg_type\nRegister a message handler" },
> 	{ "deregister", (PyCFunction)py_imessaging_deregister, METH_VARARGS|METH_KEYWORDS,
> 		"S.deregister(callback, msg_type) -> None\nDeregister a message handler" },
>+	{ "loop_once", (PyCFunction)py_imessaging_loop_once, METH_VARARGS|METH_KEYWORDS,
>+		"S.loop_once(timeout) -> None\nLoop on the internal event context until we get an event (which might be a message calling the callback), timeout after timeout seconds (if not 0)" },
> 	{ "irpc_add_name", (PyCFunction)py_irpc_add_name, METH_VARARGS,
> 		"S.irpc_add_name(name) -> None\nAdd this context to the list of server_id values that are registered for a particular name" },
> 	{ "irpc_remove_name", (PyCFunction)py_irpc_remove_name, METH_VARARGS,
>-- 
>2.9.3
>
>
>From 02d35eda70ea0ad8bf097f4dec32470bf55a0e66 Mon Sep 17 00:00:00 2001
>From: Gary Lockyer <gary@catalyst.net.nz>
>Date: Thu, 16 Mar 2017 16:26:01 +1300
>Subject: [PATCH 7/7] pymessaging: add single element tupple form of the
> server_id
>
>This avoids the python code needing to call getpid() internally,
>while declaring a stable task_id.
>
>Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
>---
> python/samba/tests/messaging.py     | 42 +++++++++++++++++++++++++++++++++++++
> source4/lib/messaging/pymessaging.c |  9 +++++++-
> 2 files changed, 50 insertions(+), 1 deletion(-)
>
>diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py
>index 6ee18e7..e9e6bf1 100644
>--- a/python/samba/tests/messaging.py
>+++ b/python/samba/tests/messaging.py
>@@ -25,6 +25,7 @@ import time
> from samba.ndr import ndr_print
> from samba.dcerpc import server_id
> import random
>+import os
> 
> class MessagingTests(TestCase):
> 
>@@ -109,3 +110,44 @@ class MessagingTests(TestCase):
> 
>         self.assertEqual(got_ping["count"], 2)
>         self.assertEqual(got_pong["count"], 2)
>+
>+    def test_pid_defaulting(self):
>+        got_ping = {"count": 0}
>+        got_pong = {"count": 0}
>+        timeout = False
>+
>+        msg_pong = 0
>+        msg_ping = 0
>+
>+        pid = os.getpid()
>+        server_ctx = self.get_context((pid, 1))
>+        def ping_callback(got_ping, msg_type, src, data):
>+            got_ping["count"] += 1
>+            server_ctx.send(src, msg_pong, data)
>+
>+        msg_ping = server_ctx.register((ping_callback, got_ping))
>+
>+        def pong_callback(got_pong, msg_type, src, data):
>+            got_pong["count"] += 1
>+
>+        client_ctx = self.get_context((2,))
>+        msg_pong = client_ctx.register((pong_callback, got_pong))
>+
>+        # Try 1 an two element tuple forms
>+        client_ctx.send((pid, 1), msg_ping, "testing")
>+
>+        client_ctx.send((1,), msg_ping, "testing2")
>+
>+        start_time = time.time()
>+
>+        # NOTE WELL: If debugging this with GDB, then the timeout will
>+        # fire while you are trying to understand it.
>+
>+        while (got_ping["count"] < 2 or got_pong["count"] < 2) and not timeout:
>+            client_ctx.loop_once(0.1)
>+            server_ctx.loop_once(0.1)
>+            if time.time() - start_time > 1:
>+                timeout = True
>+
>+        self.assertEqual(got_ping["count"], 2)
>+        self.assertEqual(got_pong["count"], 2)
>diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c
>index 84ceff5..54920dd 100644
>--- a/source4/lib/messaging/pymessaging.c
>+++ b/source4/lib/messaging/pymessaging.c
>@@ -62,13 +62,20 @@ static bool server_id_from_py(PyObject *object, struct server_id *server_id)
> 		server_id->task_id = task_id;
> 		server_id->vnn = vnn;
> 		return true;
>-	} else {
>+	} else if (PyTuple_Size(object) == 2) {
> 		unsigned long long pid;
> 		int task_id;
> 		if (!PyArg_ParseTuple(object, "KI", &pid, &task_id))
> 			return false;
> 		*server_id = cluster_id(pid, task_id);
> 		return true;
>+	} else {
>+		unsigned long long pid = getpid();
>+		int task_id;
>+		if (!PyArg_ParseTuple(object, "I", &task_id))
>+			return false;
>+		*server_id = cluster_id(pid, task_id);
>+		return true;
> 	}
> }
> 
>-- 
>2.9.3
>

Actions: View

Attachments on bug 12705: 13099 | 13445