bus: add a more comprehensive test for the bloom filter logic

.gitignore

@@ -83,6 +83,7 @@
 /tags
 /test-bus-chat
 /test-bus-kernel
+/test-bus-kernel-bloom
 /test-bus-marshal
 /test-bus-match
 /test-bus-memfd

Makefile.am

@@ -1763,6 +1763,7 @@ tests += \
 	test-bus-server \
 	test-bus-match \
 	test-bus-kernel \
+	test-bus-kernel-bloom \
 	test-bus-memfd \
 	test-bus-zero-copy
 
@@ -1836,6 +1837,17 @@ test_bus_kernel_LDADD = \
 	libsystemd-bus.la \
 	libsystemd-id128-internal.la
 
+test_bus_kernel_bloom_SOURCES = \
+	src/libsystemd-bus/test-bus-kernel-bloom.c
+
+test_bus_kernel_bloom_CFLAGS = \
+	$(AM_CFLAGS)
+
+test_bus_kernel_bloom_LDADD = \
+	libsystemd-shared.la \
+	libsystemd-bus.la \
+	libsystemd-id128-internal.la
+
 test_bus_memfd_SOURCES = \
 	src/libsystemd-bus/test-bus-memfd.c
 

TODO

@@ -44,7 +44,13 @@ Features:
   - merge busctl into systemctl or so?
   - synthesize sd_bus_message objects from kernel messages
   - properly implement name registry ioctls for kdbus
-  - get rid of object hash table, use decision tree instead?
+  - get rid of object hash table, use decision tree everyhwere instead?
+  - implement monitor logic
+  - object vtable logic
+  - longer term:
+    * priority queues
+    * worker threads
+    * priority inheritance
 
 * in the final killing spree, detect processes from the root directory, and
   complain loudly if they have argv[0][0] == '@' set.

src/libsystemd-bus/bus-control.c

@@ -385,12 +385,9 @@ int bus_add_match_internal(
 
                                 break;
 
-                        case BUS_MATCH_DESTINATION:
-                                /* The bloom filter does not include
-                                   the destination, since it is only
-                                   available for broadcast messages
-                                   which do not carry a destination
-                                   since they are undirected. */
+                        case BUS_MATCH_MESSAGE_TYPE:
+                                bloom_add_pair(bloom, "message-type", bus_message_type_to_string(c->value_u8));
+                                using_bloom = true;
                                 break;
 
                         case BUS_MATCH_INTERFACE:
@@ -413,11 +410,39 @@ int bus_add_match_internal(
                                 using_bloom = true;
                                 break;
 
-                        case BUS_MATCH_ARG...BUS_MATCH_ARG_LAST:
-                        case BUS_MATCH_ARG_PATH...BUS_MATCH_ARG_PATH_LAST:
-                        case BUS_MATCH_ARG_NAMESPACE...BUS_MATCH_ARG_NAMESPACE_LAST:
-                        case BUS_MATCH_MESSAGE_TYPE:
-                                assert_not_reached("FIXME!");
+                        case BUS_MATCH_ARG...BUS_MATCH_ARG_LAST: {
+                                char buf[sizeof("arg")-1 + 2 + 1];
+
+                                snprintf(buf, sizeof(buf), "arg%u", c->type - BUS_MATCH_ARG);
+                                bloom_add_pair(bloom, buf, c->value_str);
+                                using_bloom = true;
+                                break;
+                        }
+
+                        case BUS_MATCH_ARG_PATH...BUS_MATCH_ARG_PATH_LAST: {
+                                char buf[sizeof("arg")-1 + 2 + sizeof("-slash-prefix")];
+
+                                snprintf(buf, sizeof(buf), "arg%u-slash-prefix", c->type - BUS_MATCH_ARG_PATH);
+                                bloom_add_pair(bloom, buf, c->value_str);
+                                using_bloom = true;
+                                break;
+                        }
+
+                        case BUS_MATCH_ARG_NAMESPACE...BUS_MATCH_ARG_NAMESPACE_LAST: {
+                                char buf[sizeof("arg")-1 + 2 + sizeof("-dot-prefix")];
+
+                                snprintf(buf, sizeof(buf), "arg%u-dot-prefix", c->type - BUS_MATCH_ARG_NAMESPACE);
+                                bloom_add_pair(bloom, buf, c->value_str);
+                                using_bloom = true;
+                                break;
+                        }
+
+                        case BUS_MATCH_DESTINATION:
+                                /* The bloom filter does not include
+                                   the destination, since it is only
+                                   available for broadcast messages
+                                   which do not carry a destination
+                                   since they are undirected. */
                                 break;
 
                         case BUS_MATCH_ROOT:

src/libsystemd-bus/kdbus.h

@@ -67,7 +67,7 @@ struct kdbus_timestamp {
 
 /* Message Item Types */
 enum {
-	KDBUS_MSG_NULL,
+	_KDBUS_MSG_NULL,
 
 	/* Filled in by userspace */
 	KDBUS_MSG_PAYLOAD_VEC,		/* .data_vec, reference to memory area */
@@ -151,7 +151,7 @@ enum {
 };
 
 enum {
-	KDBUS_PAYLOAD_NULL,
+	_KDBUS_PAYLOAD_NULL,
 	KDBUS_PAYLOAD_DBUS1	= 0x4442757356657231ULL, /* 'DBusVer1' */
 	KDBUS_PAYLOAD_GVARIANT	= 0x4756617269616e74ULL, /* 'GVariant' */
 };
@@ -182,13 +182,13 @@ struct kdbus_msg {
 };
 
 enum {
-	KDBUS_POLICY_NULL,
+	_KDBUS_POLICY_NULL,
 	KDBUS_POLICY_NAME,
 	KDBUS_POLICY_ACCESS,
 };
 
 enum {
-	KDBUS_POLICY_ACCESS_NULL,
+	_KDBUS_POLICY_ACCESS_NULL,
 	KDBUS_POLICY_ACCESS_USER,
 	KDBUS_POLICY_ACCESS_GROUP,
 	KDBUS_POLICY_ACCESS_WORLD,
@@ -237,7 +237,7 @@ enum {
 
 /* Items to append to struct kdbus_cmd_hello */
 enum {
-	KDBUS_HELLO_NULL,
+	_KDBUS_HELLO_NULL,
 	KDBUS_HELLO_POOL,	/* kdbus_vec, userspace supplied pool to
 				 * place received messages */
 };
@@ -274,7 +274,7 @@ enum {
 
 /* Items to append to kdbus_cmd_{bus,ep,ns}_make */
 enum {
-	KDBUS_MAKE_NULL,
+	_KDBUS_MAKE_NULL,
 	KDBUS_MAKE_NAME,
 	KDBUS_MAKE_CGROUP,	/* the cgroup hierarchy ID for which to attach
 				 * cgroup membership paths * to messages. */
@@ -346,7 +346,7 @@ struct kdbus_cmd_names {
 };
 
 enum {
-	KDBUS_NAME_INFO_ITEM_NULL,
+	_KDBUS_NAME_INFO_ITEM_NULL,
 	KDBUS_NAME_INFO_ITEM_NAME,	/* userspace → kernel */
 	KDBUS_NAME_INFO_ITEM_SECLABEL,	/* kernel → userspace */
 	KDBUS_NAME_INFO_ITEM_AUDIT,	/* kernel → userspace */
@@ -361,7 +361,7 @@ struct kdbus_cmd_name_info {
 };
 
 enum {
-	KDBUS_MATCH_NULL,
+	_KDBUS_MATCH_NULL,
 	KDBUS_MATCH_BLOOM,		/* Matches a mask blob against KDBUS_MSG_BLOOM */
 	KDBUS_MATCH_SRC_NAME,		/* Matches a name string against KDBUS_MSG_SRC_NAMES */
 	KDBUS_MATCH_NAME_ADD,		/* Matches a name string against KDBUS_MSG_NAME_ADD */

src/libsystemd-bus/test-bus-kernel-bloom.c

@@ -0,0 +1,99 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd 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
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "util.h"
+#include "log.h"
+
+#include "sd-bus.h"
+#include "bus-message.h"
+#include "bus-error.h"
+#include "bus-kernel.h"
+
+static void test_one(
+                const char *path,
+                const char *interface,
+                const char *member,
+                const char *arg0,
+                const char *match,
+                bool good) {
+
+        _cleanup_close_ int bus_ref = -1;
+        _cleanup_free_ char *bus_name = NULL, *address = NULL;
+        _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+        sd_bus *a, *b;
+        int r;
+
+        bus_ref = bus_kernel_create("deine-mutter", &bus_name);
+        if (bus_ref == -ENOENT)
+                exit(EXIT_TEST_SKIP);
+
+        assert_se(bus_ref >= 0);
+
+        address = strappend("kernel:path=", bus_name);
+        assert_se(address);
+
+        r = sd_bus_new(&a);
+        assert_se(r >= 0);
+
+        r = sd_bus_new(&b);
+        assert_se(r >= 0);
+
+        r = sd_bus_set_address(a, address);
+        assert_se(r >= 0);
+
+        r = sd_bus_set_address(b, address);
+        assert_se(r >= 0);
+
+        r = sd_bus_start(a);
+        assert_se(r >= 0);
+
+        r = sd_bus_start(b);
+        assert_se(r >= 0);
+
+        r = sd_bus_add_match(b, match, NULL, NULL);
+        assert_se(r >= 0);
+
+        r = sd_bus_emit_signal(a, path, interface, member, "s", arg0);
+        assert_se(r >= 0);
+
+        r = sd_bus_process(b, &m);
+        assert_se(r >= 0 && (good == !!m));
+
+        sd_bus_unref(a);
+        sd_bus_unref(b);
+}
+
+int main(int argc, char *argv[]) {
+        log_set_max_level(LOG_DEBUG);
+
+        test_one("/foo/bar/waldo", "waldo.com", "Piep", "foobar", "", true);
+        test_one("/foo/bar/waldo", "waldo.com", "Piep", "foobar", "path='/foo/bar/waldo'", true);
+        test_one("/foo/bar/waldo", "waldo.com", "Piep", "foobar", "path='/foo/bar/waldo/tuut'", false);
+        test_one("/foo/bar/waldo", "waldo.com", "Piep", "foobar", "interface='waldo.com'", true);
+        test_one("/foo/bar/waldo", "waldo.com", "Piep", "foobar", "member='Piep'", true);
+        test_one("/foo/bar/waldo", "waldo.com", "Piep", "foobar", "member='Pi_ep'", false);
+        test_one("/foo/bar/waldo", "waldo.com", "Piep", "foobar", "arg0='foobar'", true);
+        test_one("/foo/bar/waldo", "waldo.com", "Piep", "foobar", "arg0='foo_bar'", false);
+        test_one("/foo/bar/waldo", "waldo.com", "Piep", "foobar", "path='/foo/bar/waldo',interface='waldo.com',member='Piep',arg0='foobar'", true);
+        test_one("/foo/bar/waldo", "waldo.com", "Piep", "foobar", "path='/foo/bar/waldo',interface='waldo.com',member='Piep',arg0='foobar2'", false);
+
+        return 0;
+}

src/libsystemd-bus/test-bus-kernel.c

@@ -92,8 +92,8 @@ int main(int argc, char *argv[]) {
 
         printf("unique b: %s\n", ub);
 
-        /* interface='waldo.com',member='Piep' */
-        assert_se(sd_bus_add_match(b, "interface='waldo.com'", NULL, NULL) >= 0);
+        r = sd_bus_add_match(b, "interface='waldo.com',member='Piep'", NULL, NULL);
+        assert_se(r >= 0);
 
         r = sd_bus_emit_signal(a, "/foo/bar/waldo", "waldo.com", "Piep", "sss", "I am a string", "/this/is/a/path", "and.this.a.domain.name");
         assert_se(r >= 0);