[PATCH 01/10] dbus: Send InterfacesRemoved on object unregister

[PATCH 01/10] dbus: Send InterfacesRemoved on object unregister

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

Make sure we call _dbus_object_tree_remove_interface to do all the
necessary steps of interface removal when an object from the tree
together with its interfaces.
---
 ell/dbus-service.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 1e3bb66..53e2512 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -733,13 +733,24 @@ bool _dbus_object_tree_object_destroy(struct _dbus_object_tree *tree,
 					const char *path)
 {
 	struct object_node *node;
+	const struct l_queue_entry *entry;
+	const struct interface_instance *instance;
 
-	node = l_hashmap_remove(tree->objects, path);
+	node = l_hashmap_lookup(tree->objects, path);
 	if (!node)
 		return false;
 
-	l_queue_destroy(node->instances,
-			(l_queue_destroy_func_t) interface_instance_free);
+	while ((entry = l_queue_get_entries(node->instances))) {
+		instance = entry->data;
+
+		if (!_dbus_object_tree_remove_interface(tree, path,
+						instance->interface->name))
+			return false;
+	}
+
+	l_hashmap_remove(tree->objects, path);
+
+	l_queue_destroy(node->instances, NULL);
 	node->instances = NULL;
 
 	if (node->destroy) {
-- 
2.5.0

[PATCH 02/10] dbus: Error checking in l_dbus_property_changed

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 4898 bytes --]

Before adding l_dbus_property_changed, _dbus_object_tree_property_changed
was only used internally and no path/interface/property errors were
possible.  With l_dbus_property_changed they need to be validated.

While there also make sure that we don't call
l_dbus_message_builder_finalize after a getter has returned an error
and may have left the builder state inconsistent since that could
probably cause l_dbus_message_builder_finalize to segfault.  We already
take care to do that everywhere else.
---
 ell/dbus-private.h |  2 +-
 ell/dbus-service.c | 44 ++++++++++++++++++++++++++++----------------
 ell/dbus-service.h |  2 +-
 3 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 1af7201..873396d 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -197,7 +197,7 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
 					struct l_dbus *dbus,
 					struct l_dbus_message *message);
 
-void _dbus_object_tree_property_changed(struct l_dbus *dbus,
+bool _dbus_object_tree_property_changed(struct l_dbus *dbus,
 					const char *path,
 					const char *interface_name,
 					const char *property_name,
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 53e2512..afd0ed9 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -765,7 +765,7 @@ bool _dbus_object_tree_object_destroy(struct _dbus_object_tree *tree,
 }
 
 /* Send the signals associated with a property value change */
-void _dbus_object_tree_property_changed(struct l_dbus *dbus,
+bool _dbus_object_tree_property_changed(struct l_dbus *dbus,
 					const char *path,
 					const char *interface_name,
 					const char *property_name,
@@ -782,12 +782,18 @@ void _dbus_object_tree_property_changed(struct l_dbus *dbus,
 	bool r;
 
 	object = l_hashmap_lookup(tree->objects, path);
+	if (!object)
+		return false;
 
 	instance = l_queue_find(object->instances, match_interface_instance,
 				interface_name);
+	if (!instance)
+		return false;
 
 	property = _dbus_interface_find_property(instance->interface,
 							property_name);
+	if (!property)
+		return false;
 
 	signature = property->metainfo + strlen(property->metainfo) + 1;
 
@@ -811,12 +817,14 @@ void _dbus_object_tree_property_changed(struct l_dbus *dbus,
 						instance->user_data);
 		}
 
-		l_dbus_message_builder_leave_variant(builder);
+		if (r) {
+			l_dbus_message_builder_leave_variant(builder);
 
-		l_dbus_message_builder_finalize(builder);
-		l_dbus_message_builder_destroy(builder);
-		if (r)
+			l_dbus_message_builder_finalize(builder);
 			l_dbus_send(dbus, signal);
+		}
+
+		l_dbus_message_builder_destroy(builder);
 	}
 
 	if (l_queue_find(object->instances, match_interface_instance,
@@ -844,17 +852,21 @@ void _dbus_object_tree_property_changed(struct l_dbus *dbus,
 						instance->user_data);
 		}
 
-		l_dbus_message_builder_leave_variant(builder);
-		l_dbus_message_builder_leave_dict(builder);
-		l_dbus_message_builder_leave_array(builder);
-		l_dbus_message_builder_enter_array(builder, "s");
-		l_dbus_message_builder_leave_array(builder);
+		if (r) {
+			l_dbus_message_builder_leave_variant(builder);
+			l_dbus_message_builder_leave_dict(builder);
+			l_dbus_message_builder_leave_array(builder);
+			l_dbus_message_builder_enter_array(builder, "s");
+			l_dbus_message_builder_leave_array(builder);
 
-		l_dbus_message_builder_finalize(builder);
-		l_dbus_message_builder_destroy(builder);
-		if (r)
+			l_dbus_message_builder_finalize(builder);
 			l_dbus_send(dbus, signal);
+		}
+
+		l_dbus_message_builder_destroy(builder);
 	}
+
+	return true;
 }
 
 static void set_property_complete(struct l_dbus *dbus,
@@ -1340,12 +1352,12 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
 	return true;
 }
 
-LIB_EXPORT void l_dbus_property_changed(struct l_dbus *dbus, const char *path,
+LIB_EXPORT bool l_dbus_property_changed(struct l_dbus *dbus, const char *path,
 					const char *interface,
 					const char *property)
 {
-	_dbus_object_tree_property_changed(dbus, path, interface, property,
-						NULL);
+	return _dbus_object_tree_property_changed(dbus, path, interface,
+							property, NULL);
 }
 
 static struct l_dbus_message *properties_get(struct l_dbus *dbus,
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index 619d9a6..2243b99 100644
--- a/ell/dbus-service.h
+++ b/ell/dbus-service.h
@@ -83,7 +83,7 @@ bool l_dbus_interface_property(struct l_dbus_interface *interface,
 				l_dbus_property_get_cb_t getter,
 				l_dbus_property_set_cb_t setter);
 
-void l_dbus_property_changed(struct l_dbus *dbus, const char *path,
+bool l_dbus_property_changed(struct l_dbus *dbus, const char *path,
 				const char *interface, const char *property);
 
 #ifdef __cplusplus
-- 
2.5.0

[PATCH 03/10] unit: More Dbus object tree sanity tests

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3928 bytes --]

Test situations where interfaces are present on some nodes in the object
tree.
---
 unit/test-dbus-service.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 79 insertions(+), 3 deletions(-)

diff --git a/unit/test-dbus-service.c b/unit/test-dbus-service.c
index 2438e68..8a395b1 100644
--- a/unit/test-dbus-service.c
+++ b/unit/test-dbus-service.c
@@ -172,7 +172,7 @@ static void test_introspect_interface(const void *test_data)
 	l_free(xml);
 }
 
-static void test_dbus_object_tree(const void *test_data)
+static void test_dbus_object_tree_1(const void *test_data)
 {
 	struct _dbus_object_tree *tree;
 	struct object_node *leaf1, *leaf2, *leaf3;
@@ -226,6 +226,79 @@ static void test_dbus_object_tree(const void *test_data)
 	_dbus_object_tree_free(tree);
 }
 
+static void setup_dummy_interface(struct l_dbus_interface *iface)
+{
+}
+
+static void interface_destroy(void *data)
+{
+	bool *destroyed = data;
+
+	*destroyed = true;
+}
+
+static void test_dbus_object_tree_2(const void *test_data)
+{
+	struct _dbus_object_tree *tree;
+	bool destroyed[2] = { false, false };
+
+	tree = _dbus_object_tree_new();
+	assert(tree);
+
+	assert(_dbus_object_tree_new_object(tree, "/foo", NULL, NULL));
+	assert(_dbus_object_tree_new_object(tree, "/foo/bar/baz", NULL, NULL));
+
+	assert(_dbus_object_tree_lookup(tree, "/foo"));
+	assert(_dbus_object_tree_lookup(tree, "/foo/bar"));
+	assert(_dbus_object_tree_lookup(tree, "/foo/bar/baz"));
+
+	assert(_dbus_object_tree_register_interface(tree, "org.example",
+						setup_dummy_interface,
+						interface_destroy, false));
+
+	assert(_dbus_object_tree_add_interface(tree, "/foo",
+						"org.example", &destroyed[0]));
+	assert(!_dbus_object_tree_add_interface(tree, "/foo",
+						"org.other", NULL));
+	assert(_dbus_object_tree_add_interface(tree, "/foo/bar/baz",
+						"org.example", &destroyed[1]));
+
+	assert(!_dbus_object_tree_remove_interface(tree, "/foo", "org.other"));
+	assert(_dbus_object_tree_remove_interface(tree, "/foo", "org.example"));
+	assert(destroyed[0]);
+
+	assert(_dbus_object_tree_object_destroy(tree, "/foo"));
+	assert(!_dbus_object_tree_object_destroy(tree, "/foo/bar"));
+
+	assert(_dbus_object_tree_lookup(tree, "/foo"));
+	assert(_dbus_object_tree_lookup(tree, "/foo/bar"));
+	assert(_dbus_object_tree_lookup(tree, "/foo/bar/baz"));
+
+	destroyed[0] = false;
+	assert(_dbus_object_tree_add_interface(tree, "/foo",
+						"org.example", &destroyed[0]));
+
+	assert(_dbus_object_tree_object_destroy(tree, "/foo/bar/baz"));
+
+	assert(_dbus_object_tree_lookup(tree, "/foo"));
+	assert(!_dbus_object_tree_lookup(tree, "/foo/bar"));
+	assert(!_dbus_object_tree_lookup(tree, "/foo/bar/baz"));
+
+	assert(_dbus_object_tree_object_destroy(tree, "/foo"));
+	assert(destroyed[0] && destroyed[1]);
+	assert(!_dbus_object_tree_lookup(tree, "/foo"));
+
+	destroyed[0] = false;
+	assert(_dbus_object_tree_add_interface(tree, "/foo",
+						"org.example", &destroyed[0]));
+
+	assert(!_dbus_object_tree_unregister_interface(tree, "org.other"));
+	assert(_dbus_object_tree_unregister_interface(tree, "org.example"));
+	assert(destroyed[0]);
+
+	_dbus_object_tree_free(tree);
+}
+
 static struct l_dbus_message *get_modems_callback(struct l_dbus *dbus,
 					struct l_dbus_message *message,
 					void *user_data)
@@ -372,8 +445,11 @@ int main(int argc, char *argv[])
 	l_test_add("Test Interface Introspection", test_introspect_interface,
 			&interface_test);
 
-	l_test_add("_dbus_object_tree Sanity Tests",
-					test_dbus_object_tree, NULL);
+	l_test_add("_dbus_object_tree Sanity Tests 1",
+					test_dbus_object_tree_1, NULL);
+
+	l_test_add("_dbus_object_tree Sanity Tests 2",
+					test_dbus_object_tree_2, NULL);
 
 	l_test_add("_dbus_object_tree Introspection",
 					test_dbus_object_tree_introspection,
-- 
2.5.0

[PATCH 04/10] unit: DBus old-style property getter tests

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 11128 bytes --]

---
 Makefile.am                 |   3 +
 unit/test-dbus-properties.c | 401 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 404 insertions(+)
 create mode 100644 unit/test-dbus-properties.c

diff --git a/Makefile.am b/Makefile.am
index 406a650..d0d06c5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -137,6 +137,7 @@ dbus_tests = unit/test-kdbus \
 			unit/test-dbus-message \
 			unit/test-dbus-service \
 			unit/test-dbus-watch \
+			unit/test-dbus-properties \
 			unit/test-gvariant-util \
 			unit/test-gvariant-message
 
@@ -186,6 +187,8 @@ unit_test_dbus_service_LDADD = ell/libell-private.la
 
 unit_test_dbus_watch_LDADD = ell/libell-private.la
 
+unit_test_dbus_properties_LDADD = ell/libell-private.la
+
 unit_test_gvariant_util_LDADD = ell/libell-private.la
 
 unit_test_gvariant_message_LDADD = ell/libell-private.la
diff --git a/unit/test-dbus-properties.c b/unit/test-dbus-properties.c
new file mode 100644
index 0000000..ff9ecbf
--- /dev/null
+++ b/unit/test-dbus-properties.c
@@ -0,0 +1,401 @@
+/*
+ *
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2011-2014  Intel Corporation. All rights reserved.
+ *
+ *  This library 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.
+ *
+ *  This library 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 this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+
+#include <ell/ell.h>
+
+#define TEST_BUS_ADDRESS "unix:path=/tmp/ell-test-bus"
+
+static pid_t dbus_daemon_pid = -1;
+
+static void start_dbus_daemon(void)
+{
+	char *prg_argv[6];
+	char *prg_envp[1];
+	pid_t pid;
+
+	prg_argv[0] = "/usr/bin/dbus-daemon";
+	prg_argv[1] = "--session";
+	prg_argv[2] = "--address=" TEST_BUS_ADDRESS;
+	prg_argv[3] = "--nopidfile";
+	prg_argv[4] = "--nofork";
+	prg_argv[5] = NULL;
+
+	prg_envp[0] = NULL;
+
+	l_info("launching dbus-daemon");
+
+	pid = fork();
+	if (pid < 0) {
+		l_error("failed to fork new process");
+		return;
+	}
+
+	if (pid == 0) {
+		execve(prg_argv[0], prg_argv, prg_envp);
+		exit(EXIT_SUCCESS);
+	}
+
+	l_info("dbus-daemon process %d created", pid);
+
+	dbus_daemon_pid = pid;
+}
+
+static void signal_handler(struct l_signal *signal, uint32_t signo,
+							void *user_data)
+{
+	switch (signo) {
+	case SIGINT:
+	case SIGTERM:
+		l_info("Terminate");
+		l_main_quit();
+		break;
+	case SIGCHLD:
+		while (1) {
+			pid_t pid;
+			int status;
+
+			pid = waitpid(WAIT_ANY, &status, WNOHANG);
+			if (pid < 0 || pid == 0)
+				break;
+
+			l_info("process %d terminated with status=%d\n",
+								pid, status);
+
+			if (pid == dbus_daemon_pid) {
+				dbus_daemon_pid = -1;
+				l_main_quit();
+			}
+		}
+		break;
+	}
+}
+
+static struct l_dbus *dbus;
+
+struct dbus_test {
+	const char *name;
+	void (*start)(struct l_dbus *dbus, void *);
+	void *data;
+};
+
+static bool success;
+static struct l_queue *tests;
+
+static void test_add(const char *name,
+			void (*start)(struct l_dbus *dbus, void *),
+			void *test_data)
+{
+	struct dbus_test *test = l_new(struct dbus_test, 1);
+
+	test->name = name;
+	test->start = start;
+	test->data = test_data;
+
+	if (!tests)
+		tests = l_queue_new();
+
+	l_queue_push_tail(tests, test);
+}
+
+static void test_next()
+{
+	struct dbus_test *test = l_queue_pop_head(tests);
+
+	if (!test) {
+		success = true;
+		l_main_quit();
+		return;
+	}
+
+	l_info("TEST: %s", test->name);
+
+	test->start(dbus, test->data);
+
+	l_free(test);
+}
+
+#define test_assert(cond)	\
+	do {	\
+		if (!(cond)) {	\
+			l_info("TEST FAILED in %s at %s:%i: %s",	\
+				__func__, __FILE__, __LINE__,	\
+				L_STRINGIFY(cond));	\
+			l_main_quit();	\
+			return;	\
+		}	\
+	} while (0)
+
+static void request_name_setup(struct l_dbus_message *message, void *user_data)
+{
+	const char *name = "org.test";
+
+	l_dbus_message_set_arguments(message, "su", name, 0);
+}
+
+static void request_name_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	const char *error, *text;
+	uint32_t result;
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("error=%s", error);
+		l_error("message=%s", text);
+		l_main_quit();
+		return;
+	}
+
+	if (!l_dbus_message_get_arguments(message, "u", &result)) {
+		l_main_quit();
+		return;
+	}
+
+	l_info("request name result=%d", result);
+
+	test_next();
+}
+
+static void ready_callback(void *user_data)
+{
+	l_info("ready");
+}
+
+static void disconnect_callback(void *user_data)
+{
+	l_info("Disconnected from DBus");
+	l_main_quit();
+}
+
+static bool test_string_getter(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_builder *builder,
+				void *user_data)
+{
+	return l_dbus_message_builder_append_basic(builder, 's', "foo");
+}
+
+static bool setter_called;
+
+static void test_string_setter(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_iter *new_value,
+				l_dbus_property_complete_cb_t complete,
+				void *user_data)
+{
+	const char *strvalue;
+
+	test_assert(l_dbus_message_iter_get_variant(new_value, "s", &strvalue));
+	test_assert(!strcmp(strvalue, "bar"));
+
+	setter_called = true;
+
+	complete(dbus, message, NULL);
+}
+
+static bool test_int_getter(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_builder *builder,
+				void *user_data)
+{
+	uint32_t u = 5;
+
+	return l_dbus_message_builder_append_basic(builder, 'u', &u);
+}
+
+static void test_int_setter(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_iter *new_value,
+				l_dbus_property_complete_cb_t complete,
+				void *user_data)
+{
+	uint32_t u;
+
+	test_assert(l_dbus_message_iter_get_variant(new_value, "u", &u));
+	test_assert(u == 42);
+
+	setter_called = true;
+
+	complete(dbus, message, NULL);
+}
+
+static void test_error_setter(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_iter *new_value,
+				l_dbus_property_complete_cb_t complete,
+				void *user_data)
+{
+	setter_called = true;
+
+	complete(dbus, message, l_dbus_message_new_error(message,
+						"org.test.Error", "Error"));
+}
+
+static void setup_test_interface(struct l_dbus_interface *interface)
+{
+	l_dbus_interface_property(interface, "String", 0, "s",
+					test_string_getter, test_string_setter);
+	l_dbus_interface_property(interface, "Integer", 0, "u",
+					test_int_getter, test_int_setter);
+	l_dbus_interface_property(interface, "Readonly", 0, "s",
+					test_string_getter, NULL);
+	l_dbus_interface_property(interface, "SetError", 0, "s",
+					test_string_getter, test_error_setter);
+}
+
+static void validate_properties(struct l_dbus_message_iter *dict)
+{
+	struct l_dbus_message_iter variant;
+	const char *name, *strval;
+	uint32_t intval;
+
+	test_assert(l_dbus_message_iter_next_entry(dict, &name, &variant));
+	test_assert(!strcmp(name, "String"));
+	test_assert(l_dbus_message_iter_get_variant(&variant, "s", &strval));
+	test_assert(!strcmp(strval, "foo"));
+
+	test_assert(l_dbus_message_iter_next_entry(dict, &name, &variant));
+	test_assert(!strcmp(name, "Integer"));
+	test_assert(l_dbus_message_iter_get_variant(&variant, "u", &intval));
+	test_assert(intval == 5);
+
+	test_assert(l_dbus_message_iter_next_entry(dict, &name, &variant));
+	test_assert(!strcmp(name, "Readonly"));
+	test_assert(l_dbus_message_iter_get_variant(&variant, "s", &strval));
+	test_assert(!strcmp(strval, "foo"));
+
+	test_assert(l_dbus_message_iter_next_entry(dict, &name, &variant));
+	test_assert(!strcmp(name, "SetError"));
+	test_assert(l_dbus_message_iter_get_variant(&variant, "s", &strval));
+	test_assert(!strcmp(strval, "foo"));
+
+	test_assert(!l_dbus_message_iter_next_entry(dict, &name, &variant));
+}
+
+static void get_properties_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message_iter dict;
+
+	test_assert(!l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(l_dbus_message_get_arguments(message, "a{sv}", &dict));
+
+	validate_properties(&dict);
+
+	test_next();
+}
+
+static void test_old_get(struct l_dbus *dbus, void *test_data)
+{
+	struct l_dbus_message *call =
+		l_dbus_message_new_method_call(dbus, "org.test", "/test",
+						"org.test", "GetProperties");
+
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, ""));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, get_properties_callback,
+						NULL, NULL));
+}
+
+int main(int argc, char *argv[])
+{
+	struct l_signal *signal;
+	sigset_t mask;
+	int i;
+
+	sigemptyset(&mask);
+	sigaddset(&mask, SIGINT);
+	sigaddset(&mask, SIGTERM);
+	sigaddset(&mask, SIGCHLD);
+
+	signal = l_signal_create(&mask, signal_handler, NULL, NULL);
+
+	l_log_set_stderr();
+
+	start_dbus_daemon();
+
+	for (i = 0; i < 10; i++) {
+		usleep(200 * 1000);
+
+		dbus = l_dbus_new(TEST_BUS_ADDRESS);
+		if (dbus)
+			break;
+	}
+
+	l_dbus_set_ready_handler(dbus, ready_callback, dbus, NULL);
+	l_dbus_set_disconnect_handler(dbus, disconnect_callback, NULL, NULL);
+
+	l_dbus_method_call(dbus, "org.freedesktop.DBus",
+				"/org/freedesktop/DBus",
+				"org.freedesktop.DBus", "RequestName",
+				request_name_setup,
+				request_name_callback, NULL, NULL);
+
+	if (!l_dbus_register_interface(dbus, "org.test", setup_test_interface,
+					NULL, true)) {
+		l_info("Unable to register interface");
+		goto done;
+	}
+
+	if (!l_dbus_object_add_interface(dbus, "/test", "org.test", NULL)) {
+		l_info("Unable to instantiate interface");
+		goto done;
+	}
+
+	if (!l_dbus_object_add_interface(dbus, "/test",
+				"org.freedesktop.DBus.Properties", NULL)) {
+		l_info("Unable to instantiate the properties interface");
+		goto done;
+	}
+
+	if (!l_dbus_object_manager_enable(dbus)) {
+		l_info("Unable to enable Object Manager");
+		goto done;
+	}
+
+	test_add("Legacy properties get", test_old_get, NULL);
+
+	l_main_run();
+
+	l_queue_destroy(tests, l_free);
+
+done:
+	l_dbus_destroy(dbus);
+
+	if (dbus_daemon_pid > 0)
+		kill(dbus_daemon_pid, SIGKILL);
+
+	l_signal_remove(signal);
+
+	if (!success)
+		abort();
+
+	return 0;
+}
-- 
2.5.0

[PATCH 05/10] unit: DBus old-style property setter tests

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3754 bytes --]

---
 unit/test-dbus-properties.c | 102 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/unit/test-dbus-properties.c b/unit/test-dbus-properties.c
index ff9ecbf..d61a6b9 100644
--- a/unit/test-dbus-properties.c
+++ b/unit/test-dbus-properties.c
@@ -324,6 +324,107 @@ static void test_old_get(struct l_dbus *dbus, void *test_data)
 						NULL, NULL));
 }
 
+static void set_invalid_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	test_assert(l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(!setter_called);
+
+	test_next();
+}
+
+static void old_set_error_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(setter_called);
+	setter_called = false;
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+						"org.test", "SetProperty");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "sv", "Invalid",
+							"s", "bar"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, set_invalid_callback,
+						NULL, NULL));
+}
+
+static void old_set_ro_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(!setter_called);
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+						"org.test", "SetProperty");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "sv", "SetError",
+							"s", "bar"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, old_set_error_callback,
+						NULL, NULL));
+}
+
+static void old_set_int_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(!l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(l_dbus_message_get_arguments(message, ""));
+	test_assert(setter_called);
+	setter_called = false;
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+						"org.test", "SetProperty");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "sv", "Readonly",
+							"s", "bar"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, old_set_ro_callback,
+						NULL, NULL));
+}
+
+static void old_set_string_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(!l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(l_dbus_message_get_arguments(message, ""));
+	test_assert(setter_called);
+	setter_called = false;
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+						"org.test", "SetProperty");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "sv", "Integer",
+							"u", 42));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, old_set_int_callback,
+						NULL, NULL));
+}
+
+static void test_old_set(struct l_dbus *dbus, void *test_data)
+{
+	struct l_dbus_message *call =
+		l_dbus_message_new_method_call(dbus, "org.test", "/test",
+						"org.test", "SetProperty");
+
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "sv", "String",
+							"s", "bar"));
+
+	test_assert(!setter_called);
+	test_assert(l_dbus_send_with_reply(dbus, call, old_set_string_callback,
+						NULL, NULL));
+}
+
 int main(int argc, char *argv[])
 {
 	struct l_signal *signal;
@@ -381,6 +482,7 @@ int main(int argc, char *argv[])
 	}
 
 	test_add("Legacy properties get", test_old_get, NULL);
+	test_add("Legacy properties set", test_old_set, NULL);
 
 	l_main_run();
 
-- 
2.5.0

[PATCH 06/10] unit: org.freedesktop.DBus.Properties getter tests

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3041 bytes --]

---
 unit/test-dbus-properties.c | 73 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/unit/test-dbus-properties.c b/unit/test-dbus-properties.c
index d61a6b9..cca3ef8 100644
--- a/unit/test-dbus-properties.c
+++ b/unit/test-dbus-properties.c
@@ -425,6 +425,78 @@ static void test_old_set(struct l_dbus *dbus, void *test_data)
 						NULL, NULL));
 }
 
+static void new_get_invalid_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(l_dbus_message_get_error(message, NULL, NULL));
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"GetAll");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "s", "org.test"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, get_properties_callback,
+						NULL, NULL));
+}
+
+static void new_get_bad_if_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(l_dbus_message_get_error(message, NULL, NULL));
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"Get");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "ss",
+							"org.test", "Invalid"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, new_get_invalid_callback,
+						NULL, NULL));
+}
+
+static void new_get_callback(struct l_dbus_message *message, void *user_data)
+{
+	struct l_dbus_message_iter variant;
+	const char *strval;
+	struct l_dbus_message *call;
+
+	test_assert(!l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(l_dbus_message_get_arguments(message, "v", &variant));
+	test_assert(l_dbus_message_iter_get_variant(&variant, "s", &strval));
+	test_assert(!strcmp(strval, "foo"));
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"Get");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "ss", "org.invalid",
+							"String"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, new_get_bad_if_callback,
+						NULL, NULL));
+}
+
+static void test_new_get(struct l_dbus *dbus, void *test_data)
+{
+	struct l_dbus_message *call =
+		l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"Get");
+
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "ss",
+							"org.test", "String"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, new_get_callback,
+						NULL, NULL));
+}
+
 int main(int argc, char *argv[])
 {
 	struct l_signal *signal;
@@ -483,6 +555,7 @@ int main(int argc, char *argv[])
 
 	test_add("Legacy properties get", test_old_get, NULL);
 	test_add("Legacy properties set", test_old_set, NULL);
+	test_add("org.freedesktop.DBus.Properties get", test_new_get, NULL);
 
 	l_main_run();
 
-- 
2.5.0

[PATCH 07/10] unit: org.freedesktop.DBus.Properties setter tests

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 4452 bytes --]

---
 unit/test-dbus-properties.c | 119 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/unit/test-dbus-properties.c b/unit/test-dbus-properties.c
index cca3ef8..19a25f5 100644
--- a/unit/test-dbus-properties.c
+++ b/unit/test-dbus-properties.c
@@ -497,6 +497,124 @@ static void test_new_get(struct l_dbus *dbus, void *test_data)
 						NULL, NULL));
 }
 
+static void new_set_bad_if_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(!setter_called);
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"Set");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "ssv", "org.test",
+							"Invalid", "s", "bar"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, set_invalid_callback,
+						NULL, NULL));
+}
+
+static void new_set_error_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(setter_called);
+	setter_called = false;
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"Set");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "ssv", "org.invalid",
+							"String", "s", "bar"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, new_set_bad_if_callback,
+						NULL, NULL));
+}
+
+static void new_set_ro_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(!setter_called);
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"Set");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "ssv", "org.test",
+							"SetError",
+							"s", "bar"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, new_set_error_callback,
+						NULL, NULL));
+}
+
+static void new_set_int_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(!l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(l_dbus_message_get_arguments(message, ""));
+	test_assert(setter_called);
+	setter_called = false;
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"Set");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "ssv", "org.test",
+							"Readonly",
+							"s", "bar"));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, new_set_ro_callback,
+						NULL, NULL));
+}
+
+static void new_set_string_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message *call;
+
+	test_assert(!l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(l_dbus_message_get_arguments(message, ""));
+	test_assert(setter_called);
+	setter_called = false;
+
+	call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"Set");
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "ssv", "org.test",
+							"Integer", "u", 42));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, new_set_int_callback,
+						NULL, NULL));
+}
+
+static void test_new_set(struct l_dbus *dbus, void *test_data)
+{
+	struct l_dbus_message *call =
+		l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"Set");
+
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, "ssv", "org.test",
+							"String", "s", "bar"));
+
+	test_assert(!setter_called);
+	test_assert(l_dbus_send_with_reply(dbus, call, new_set_string_callback,
+						NULL, NULL));
+}
+
 int main(int argc, char *argv[])
 {
 	struct l_signal *signal;
@@ -556,6 +674,7 @@ int main(int argc, char *argv[])
 	test_add("Legacy properties get", test_old_get, NULL);
 	test_add("Legacy properties set", test_old_set, NULL);
 	test_add("org.freedesktop.DBus.Properties get", test_new_get, NULL);
+	test_add("org.freedesktop.DBus.Properties set", test_new_set, NULL);
 
 	l_main_run();
 
-- 
2.5.0

[PATCH 08/10] unit: Property change notifications signal tests

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 5071 bytes --]

---
 unit/test-dbus-properties.c | 133 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

diff --git a/unit/test-dbus-properties.c b/unit/test-dbus-properties.c
index 19a25f5..289ece7 100644
--- a/unit/test-dbus-properties.c
+++ b/unit/test-dbus-properties.c
@@ -615,11 +615,134 @@ static void test_new_set(struct l_dbus *dbus, void *test_data)
 						NULL, NULL));
 }
 
+static struct l_timeout *signal_timeout;
+
+static void signal_timeout_callback(struct l_timeout *timeout, void *user_data)
+{
+	signal_timeout = NULL;
+	test_assert(false);
+}
+
+static bool old_signal_received, new_signal_received;
+static const char *signal_string_value;
+
+static void test_signal_callback(struct l_dbus_message *message)
+{
+	const char *interface, *member, *property, *value;
+	struct l_dbus_message_iter variant, changed, invalidated;
+	struct l_dbus_message *call;
+
+	if (!signal_timeout)
+		return;
+
+	interface = l_dbus_message_get_interface(message);
+	member = l_dbus_message_get_member(message);
+
+	if (!strcmp(interface, "org.test") &&
+			!strcmp(member, "PropertyChanged")) {
+		test_assert(l_dbus_message_get_arguments(message, "sv",
+								&property,
+								&variant));
+		test_assert(!strcmp(property, "String"));
+		test_assert(l_dbus_message_iter_get_variant(&variant, "s",
+								&value));
+		test_assert(!strcmp(value, signal_string_value));
+
+		test_assert(!old_signal_received);
+		old_signal_received = true;
+	}
+
+	if (!strcmp(interface, "org.freedesktop.DBus.Properties") &&
+			!strcmp(member, "PropertiesChanged")) {
+		test_assert(l_dbus_message_get_arguments(message, "sa{sv}as",
+								&interface,
+								&changed,
+								&invalidated));
+		test_assert(!strcmp(interface, "org.test"));
+
+		test_assert(l_dbus_message_iter_next_entry(&changed, &property,
+								&variant));
+		test_assert(!strcmp(property, "String"));
+		test_assert(l_dbus_message_iter_get_variant(&variant, "s",
+								&value));
+		test_assert(!strcmp(value, signal_string_value));
+
+		test_assert(!l_dbus_message_iter_next_entry(&changed, &property,
+								&variant));
+		test_assert(!l_dbus_message_iter_next_entry(&invalidated,
+								&property));
+
+		test_assert(!new_signal_received);
+		new_signal_received = true;
+	}
+
+	if (!old_signal_received || !new_signal_received)
+		return;
+
+	l_timeout_remove(signal_timeout);
+	signal_timeout = NULL;
+
+	if (!strcmp(signal_string_value, "foo")) {
+		/* Now repeat the test for the signal triggered by Set */
+
+		old_signal_received = false;
+		new_signal_received = false;
+
+		signal_timeout = l_timeout_create(1, signal_timeout_callback,
+							NULL, NULL);
+		test_assert(signal_timeout);
+
+		signal_string_value = "bar";
+
+		call = l_dbus_message_new_method_call(dbus, "org.test", "/test",
+					"org.freedesktop.DBus.Properties",
+					"Set");
+		test_assert(call);
+		test_assert(l_dbus_message_set_arguments(call, "ssv",
+							"org.test", "String",
+							"s", "bar"));
+
+		test_assert(!setter_called);
+		test_assert(l_dbus_send(dbus, call));
+	} else {
+		test_assert(setter_called);
+		setter_called = false;
+
+		test_next();
+	}
+}
+
+static void test_property_signals(struct l_dbus *dbus, void *test_data)
+{
+	old_signal_received = false;
+	new_signal_received = false;
+
+	signal_timeout = l_timeout_create(1, signal_timeout_callback,
+						NULL, NULL);
+	test_assert(signal_timeout);
+
+	signal_string_value = "foo";
+
+	test_assert(l_dbus_property_changed(dbus, "/test",
+						"org.test", "String"));
+}
+
+static void signal_message(struct l_dbus_message *message, void *user_data)
+{
+	const char *path;
+
+	path = l_dbus_message_get_path(message);
+
+	if (!strcmp(path, "/test"))
+		test_signal_callback(message);
+}
+
 int main(int argc, char *argv[])
 {
 	struct l_signal *signal;
 	sigset_t mask;
 	int i;
+	struct l_dbus_message *call;
 
 	sigemptyset(&mask);
 	sigaddset(&mask, SIGINT);
@@ -643,6 +766,15 @@ int main(int argc, char *argv[])
 	l_dbus_set_ready_handler(dbus, ready_callback, dbus, NULL);
 	l_dbus_set_disconnect_handler(dbus, disconnect_callback, NULL, NULL);
 
+	l_dbus_register(dbus, signal_message, NULL, NULL);
+
+	call = l_dbus_message_new_method_call(dbus, "org.freedesktop.DBus",
+						"/org/freedesktop/DBus",
+						"org.freedesktop.DBus",
+						"AddMatch");
+	l_dbus_message_set_arguments(call, "s", "type=signal,sender=org.test");
+	l_dbus_send(dbus, call);
+
 	l_dbus_method_call(dbus, "org.freedesktop.DBus",
 				"/org/freedesktop/DBus",
 				"org.freedesktop.DBus", "RequestName",
@@ -675,6 +807,7 @@ int main(int argc, char *argv[])
 	test_add("Legacy properties set", test_old_set, NULL);
 	test_add("org.freedesktop.DBus.Properties get", test_new_get, NULL);
 	test_add("org.freedesktop.DBus.Properties set", test_new_set, NULL);
+	test_add("Property changed signals", test_property_signals, NULL);
 
 	l_main_run();
 
-- 
2.5.0

[PATCH 09/10] unit: org.freedesktop.DBus.ObjectManager.GetManagedObject test

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 2858 bytes --]

---
 unit/test-dbus-properties.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/unit/test-dbus-properties.c b/unit/test-dbus-properties.c
index 289ece7..acbab29 100644
--- a/unit/test-dbus-properties.c
+++ b/unit/test-dbus-properties.c
@@ -727,6 +727,68 @@ static void test_property_signals(struct l_dbus *dbus, void *test_data)
 						"org.test", "String"));
 }
 
+static void object_manager_callback(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct l_dbus_message_iter objects, interfaces, properties, variant;
+	const char *path, *interface, *name;
+	bool object_manager_found = false;
+	bool test_found = false;
+	bool properties_found = false;
+
+	test_assert(!l_dbus_message_get_error(message, NULL, NULL));
+	test_assert(l_dbus_message_get_arguments(message, "a{oa{sa{sv}}}",
+							&objects));
+
+	while (l_dbus_message_iter_next_entry(&objects, &path, &interfaces)) {
+		while (l_dbus_message_iter_next_entry(&interfaces, &interface,
+							&properties)) {
+			if (!strcmp(path, "/") && !strcmp(interface,
+					"org.freedesktop.DBus.ObjectManager")) {
+				test_assert(!object_manager_found);
+				object_manager_found = true;
+				test_assert(!l_dbus_message_iter_next_entry(
+							&properties, &name,
+							&variant));
+			}
+
+			if (!strcmp(path, "/test") && !strcmp(interface,
+					"org.freedesktop.DBus.Properties")) {
+				test_assert(!properties_found);
+				properties_found = true;
+				test_assert(!l_dbus_message_iter_next_entry(
+							&properties, &name,
+							&variant));
+			}
+
+			if (!strcmp(path, "/test") && !strcmp(interface,
+								"org.test")) {
+				test_assert(!test_found);
+				test_found = true;
+				validate_properties(&properties);
+			}
+		}
+	}
+
+	test_assert(object_manager_found && test_found && properties_found);
+
+	test_next();
+}
+
+static void test_object_manager_get(struct l_dbus *dbus, void *test_data)
+{
+	struct l_dbus_message *call =
+		l_dbus_message_new_method_call(dbus, "org.test", "/",
+					"org.freedesktop.DBus.ObjectManager",
+					"GetManagedObjects");
+
+	test_assert(call);
+	test_assert(l_dbus_message_set_arguments(call, ""));
+
+	test_assert(l_dbus_send_with_reply(dbus, call, object_manager_callback,
+						NULL, NULL));
+}
+
 static void signal_message(struct l_dbus_message *message, void *user_data)
 {
 	const char *path;
@@ -808,6 +870,8 @@ int main(int argc, char *argv[])
 	test_add("org.freedesktop.DBus.Properties get", test_new_get, NULL);
 	test_add("org.freedesktop.DBus.Properties set", test_new_set, NULL);
 	test_add("Property changed signals", test_property_signals, NULL);
+	test_add("org.freedesktop.DBus.ObjectManager get",
+			test_object_manager_get, NULL);
 
 	l_main_run();
 
-- 
2.5.0

Re: [PATCH 01/10] dbus: Send InterfacesRemoved on object unregister

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

Hi Andrew,

On 02/15/2016 10:44 AM, Andrew Zaborowski wrote:
> Make sure we call _dbus_object_tree_remove_interface to do all the
> necessary steps of interface removal when an object from the tree
> together with its interfaces.
> ---
>   ell/dbus-service.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
>

All 10 patches applied, thanks.

Regards,
-Denis