From: Lennart Poettering Date: Sun, 19 May 2013 22:21:56 +0000 (+0200) Subject: bus: add a more comprehensive test for the bloom filter logic X-Git-Tag: v205~214 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=86312ab8de59c1066d6d2b456f3a9106ce3e0991;hp=c78196699d3d805b2237896a1d2b8efeec6068d0;ds=sidebyside bus: add a more comprehensive test for the bloom filter logic --- diff --git a/.gitignore b/.gitignore index 2dd1d715d..c2e745672 100644 --- a/.gitignore +++ b/.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 diff --git a/Makefile.am b/Makefile.am index 66f12e903..2b2efcc74 100644 --- a/Makefile.am +++ b/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 diff --git a/TODO b/TODO index 561e7ceb3..f8a1b1b57 100644 --- a/TODO +++ b/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. diff --git a/src/libsystemd-bus/bus-control.c b/src/libsystemd-bus/bus-control.c index ae0d7f976..66f713082 100644 --- a/src/libsystemd-bus/bus-control.c +++ b/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: diff --git a/src/libsystemd-bus/kdbus.h b/src/libsystemd-bus/kdbus.h index 214bf5119..e10a1547a 100644 --- a/src/libsystemd-bus/kdbus.h +++ b/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 */ diff --git a/src/libsystemd-bus/test-bus-kernel-bloom.c b/src/libsystemd-bus/test-bus-kernel-bloom.c new file mode 100644 index 000000000..02d9a98be --- /dev/null +++ b/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 . +***/ + +#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; +} diff --git a/src/libsystemd-bus/test-bus-kernel.c b/src/libsystemd-bus/test-bus-kernel.c index 0a2457a6d..680dcde5b 100644 --- a/src/libsystemd-bus/test-bus-kernel.c +++ b/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);