[PATCH v5 0/4] Add DBus disconnect watch support

[PATCH v5 0/4] Add DBus disconnect watch support

[Jukka Rissanen]

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

Hi,

v5:
- rebased the set
- fixed filter rule testing and added more tests there

v4:
- minor tweaking here and there
- filter rule and corresponding unit test in a separate patch

v3:
- reworked the unit test

v2:
- moved the code to dbus.c
- reworked the logic in the caller of the watcher so no need to
  remove l_dbus_register() and l_dbus_unregister()

this set allows user to monitor the disconnect status
of a service.

Cheers,
Jukka


Jukka Rissanen (4):
  unit: dbus: Add test for filter rule
  dbus: Add AddMatch and RemoveMatch support
  dbus: Add disconnect watch support
  unit: dbus: Add test for disconnect watch API

 Makefile.am            |   3 +
 ell/dbus-private.h     |   3 +
 ell/dbus.c             | 134 +++++++++++++++++++++++++
 ell/dbus.h             |   7 ++
 unit/test-dbus-watch.c | 268 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 415 insertions(+)
 create mode 100644 unit/test-dbus-watch.c

-- 
2.1.0

[PATCH v5 1/4] unit: dbus: Add test for filter rule

[Jukka Rissanen]

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

Testing _dbus1_filter_format_match() function.
---
 Makefile.am            |   3 +
 unit/test-dbus-watch.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 206 insertions(+)
 create mode 100644 unit/test-dbus-watch.c

diff --git a/Makefile.am b/Makefile.am
index 36a680b..cea1b7a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -107,6 +107,7 @@ unit_tests = unit/test-unit \
 			unit/test-dbus-util \
 			unit/test-dbus-message \
 			unit/test-dbus-service \
+			unit/test-dbus-watch \
 			unit/test-gvariant-util \
 			unit/test-gvariant-message \
 			unit/test-siphash \
@@ -154,6 +155,8 @@ unit_test_dbus_util_LDADD = ell/libell-private.la
 
 unit_test_dbus_service_LDADD = ell/libell-private.la
 
+unit_test_dbus_watch_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-watch.c b/unit/test-dbus-watch.c
new file mode 100644
index 0000000..3868c8f
--- /dev/null
+++ b/unit/test-dbus-watch.c
@@ -0,0 +1,203 @@
+/*
+ *
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2011-2015  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 <assert.h>
+#include <time.h>
+#include <stdio.h>
+
+#include <ell/ell.h>
+#include "ell/dbus-private.h"
+
+#define DBUS_SERVICE_DBUS	"org.freedesktop.DBus"
+#define DBUS_PATH_DBUS		"/org/freedesktop/DBus"
+#define DBUS_INTERFACE_DBUS	"org.freedesktop.DBus"
+#define DBUS_MAXIMUM_MATCH_RULE_LENGTH	1024
+
+struct watch_test {
+	const char *name;
+	const char *service;
+	const char *path;
+	const char *interface;
+	const char *method;
+	const char *expected;
+};
+
+static const struct watch_test match_test_1 = {
+	.name = ":1.101",
+	.service = DBUS_SERVICE_DBUS,
+	.path = DBUS_PATH_DBUS,
+	.interface = DBUS_INTERFACE_DBUS,
+	.method = "NameOwnerChanged",
+	.expected = "type='signal',"
+		"sender='org.freedesktop.DBus',"
+		"path='/org/freedesktop/DBus',"
+		"interface='org.freedesktop.DBus',"
+		"member='NameOwnerChanged',"
+		"arg0=':1.101'",
+};
+
+static const struct watch_test match_test_2 = {
+	.name = ":1.102",
+	.service = NULL,
+	.path = DBUS_PATH_DBUS,
+	.interface = DBUS_INTERFACE_DBUS,
+	.method = "NameOwnerChanged",
+	.expected = "type='signal',"
+		"path='/org/freedesktop/DBus',"
+		"interface='org.freedesktop.DBus',"
+		"member='NameOwnerChanged',"
+		"arg0=':1.102'",
+};
+
+static const struct watch_test match_test_3 = {
+	.name = ":1.102",
+	.service = DBUS_SERVICE_DBUS,
+	.path = NULL,
+	.interface = DBUS_INTERFACE_DBUS,
+	.method = "NameOwnerChanged",
+	.expected = "type='signal',"
+		"sender='org.freedesktop.DBus',"
+		"interface='org.freedesktop.DBus',"
+		"member='NameOwnerChanged',"
+		"arg0=':1.102'",
+};
+
+static const struct watch_test match_test_4 = {
+	.name = ":1.102",
+	.service = DBUS_SERVICE_DBUS,
+	.path = DBUS_PATH_DBUS,
+	.interface = NULL,
+	.method = "NameOwnerChanged",
+	.expected = "type='signal',"
+		"sender='org.freedesktop.DBus',"
+		"path='/org/freedesktop/DBus',"
+		"member='NameOwnerChanged',"
+		"arg0=':1.102'",
+};
+
+static const struct watch_test match_test_5 = {
+	.name = ":1.102",
+	.service = DBUS_SERVICE_DBUS,
+	.path = DBUS_PATH_DBUS,
+	.interface = DBUS_INTERFACE_DBUS,
+	.method = NULL,
+	.expected = "type='signal',"
+		"sender='org.freedesktop.DBus',"
+		"path='/org/freedesktop/DBus',"
+		"interface='org.freedesktop.DBus',"
+		"arg0=':1.102'",
+};
+
+static const struct watch_test match_test_6 = {
+	.name = NULL,
+	.service = DBUS_SERVICE_DBUS,
+	.path = DBUS_PATH_DBUS,
+	.interface = DBUS_INTERFACE_DBUS,
+	.method = "NameOwnerChanged",
+	.expected = "type='signal',"
+		"sender='org.freedesktop.DBus',"
+		"path='/org/freedesktop/DBus',"
+		"interface='org.freedesktop.DBus',"
+		"member='NameOwnerChanged'",
+};
+
+static const struct watch_test match_test_7 = {
+	.name = ":1.101",
+	.service = NULL,
+	.path = NULL,
+	.interface = DBUS_INTERFACE_DBUS,
+	.method = "NameOwnerChanged",
+	.expected = "type='signal',"
+		"interface='org.freedesktop.DBus',"
+		"member='NameOwnerChanged',"
+		"arg0=':1.101'",
+};
+
+static const struct watch_test match_test_8 = {
+	.name = ":1.101",
+	.service = NULL,
+	.path = NULL,
+	.interface = NULL,
+	.method = "NameOwnerChanged",
+	.expected = "type='signal',"
+		"member='NameOwnerChanged',"
+		"arg0=':1.101'",
+};
+
+static const struct watch_test match_test_9 = {
+	.name = NULL,
+	.service = NULL,
+	.path = NULL,
+	.interface = NULL,
+	.method = NULL,
+	.expected = "type='signal'",
+};
+
+static void test_match(const void *test_data)
+{
+	const struct watch_test *test = test_data;
+	struct dbus1_filter_data *data;
+	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
+
+	data = _dbus1_filter_data_get(NULL,
+				NULL,
+				test->service,
+				test->path,
+				test->interface,
+				test->method,
+				test->name,
+				NULL,
+				NULL,
+				NULL);
+
+	_dbus1_filter_format_match(data, rule, sizeof(rule));
+
+	assert(strcmp(rule, test->expected) == 0);
+
+	_dbus1_filter_data_destroy(data);
+}
+
+int main(int argc, char *argv[])
+{
+	l_test_init(&argc, &argv);
+
+	l_test_add("DBus filter NameOwnerChanged", test_match, &match_test_1);
+	l_test_add("DBus filter NULL service", test_match, &match_test_2);
+	l_test_add("DBus filter NULL path", test_match, &match_test_3);
+	l_test_add("DBus filter NULL interface", test_match, &match_test_4);
+	l_test_add("DBus filter NULL method", test_match, &match_test_5);
+	l_test_add("DBus filter NULL argument", test_match, &match_test_6);
+	l_test_add("DBus filter NULL service and path", test_match,
+								&match_test_7);
+	l_test_add("DBus filter NULL service, path and interface", test_match,
+								&match_test_8);
+	l_test_add("DBus filter NULL all fields", test_match, &match_test_9);
+
+	return l_test_run();
+}
-- 
2.1.0

[PATCH v5 2/4] dbus: Add AddMatch and RemoveMatch support

[Jukka Rissanen]

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

We do not check for errors as they are typically unrecoverable
in this case and in most cases ignored anyway.

This is the comment from libdbus API documentation for
dbus_bus_add_match() about error checking:

"If you pass NULL for the error, this function will not block;
the match thus won't be added until you flush the connection,
and if there's an error adding the match you won't find out
about it. This is generally acceptable, since the possible
errors (including a lack of resources in the bus, the connection
having exceeded its quota of active match rules, or the match
rule being unparseable) are generally unrecoverable."
---
 ell/dbus.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/ell/dbus.c b/ell/dbus.c
index 803fdf6..7ab71cd 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -1309,3 +1309,29 @@ void _dbus1_filter_data_destroy(void *user_data)
 
 	l_free(data);
 }
+
+static void dbus1_send_match(struct l_dbus *dbus, const char *rule,
+						const char *method)
+{
+	struct l_dbus_message *message;
+
+	message = l_dbus_message_new_method_call(dbus,
+						DBUS_SERVICE_DBUS,
+						DBUS_PATH_DBUS,
+						DBUS_INTERFACE_DBUS,
+						method);
+
+	l_dbus_message_set_arguments(message, "s", rule);
+
+	send_message(dbus, false, message, NULL, NULL, NULL);
+}
+
+static void dbus1_bus_add_match(struct l_dbus *dbus, const char *rule)
+{
+	dbus1_send_match(dbus, rule, "AddMatch");
+}
+
+static void dbus1_bus_remove_match(struct l_dbus *dbus, const char *rule)
+{
+	dbus1_send_match(dbus, rule, "RemoveMatch");
+}
-- 
2.1.0

[PATCH v5 3/4] dbus: Add disconnect watch support

[Jukka Rissanen]

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

Allow caller to monitor the disconnect status of a service.
---
 ell/dbus-private.h |   3 ++
 ell/dbus.c         | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 ell/dbus.h         |   7 ++++
 3 files changed, 118 insertions(+)

diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 9a5c0c1..e60d09e 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -229,3 +229,6 @@ struct dbus1_filter_data *_dbus1_filter_data_get(struct l_dbus *dbus,
 					void *user_data,
 					l_dbus_destroy_func_t destroy);
 void _dbus1_filter_data_destroy(void *user_data);
+void _dbus1_signal_dispatcher(struct l_dbus_message *message, void *user_data);
+void _dbus1_name_owner_changed_filter(struct l_dbus_message *message,
+							void *user_data);
diff --git a/ell/dbus.c b/ell/dbus.c
index 7ab71cd..712ed4d 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -53,6 +53,8 @@
 #define DBUS_INTERFACE_INTROSPECTABLE	"org.freedesktop.DBus.Introspectable"
 #define DBUS_INTERFACE_PROPERTIES	"org.freedesktop.DBus.Properties"
 
+#define DBUS_MAXIMUM_MATCH_RULE_LENGTH	1024
+
 enum auth_state {
 	WAITING_FOR_OK,
 	WAITING_FOR_AGREE_UNIX_FD,
@@ -1335,3 +1337,109 @@ static void dbus1_bus_remove_match(struct l_dbus *dbus, const char *rule)
 {
 	dbus1_send_match(dbus, rule, "RemoveMatch");
 }
+static void add_match(struct dbus1_filter_data *data)
+{
+	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
+
+	_dbus1_filter_format_match(data, rule, sizeof(rule));
+
+	dbus1_bus_add_match(data->dbus, rule);
+}
+
+static void remove_match(struct dbus1_filter_data *data)
+{
+	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
+
+	_dbus1_filter_format_match(data, rule, sizeof(rule));
+
+	dbus1_bus_remove_match(data->dbus, rule);
+}
+
+static void filter_data_destroy(void *user_data)
+{
+	remove_match(user_data);
+
+	_dbus1_filter_data_destroy(user_data);
+}
+
+void _dbus1_signal_dispatcher(struct l_dbus_message *message, void *user_data)
+{
+	struct dbus1_filter_data *data = user_data;
+	const char *sender, *path, *iface, *member;
+
+	if (_dbus_message_get_type(message) != DBUS_MESSAGE_TYPE_SIGNAL)
+		return;
+
+	sender = l_dbus_message_get_sender(message);
+	if (!sender)
+		return;
+
+	if (data->sender && strcmp(sender, data->sender))
+		return;
+
+	path = l_dbus_message_get_path(message);
+	if (data->path && strcmp(path, data->path))
+		return;
+
+	iface = l_dbus_message_get_interface(message);
+	if (data->interface && strcmp(iface, data->interface))
+		return;
+
+	member = l_dbus_message_get_member(message);
+	if (data->member && strcmp(member, data->member))
+		return;
+
+	if (data->handle_func)
+		data->handle_func(message, data);
+}
+
+void _dbus1_name_owner_changed_filter(struct l_dbus_message *message,
+							void *user_data)
+{
+	struct dbus1_filter_data *data = user_data;
+	char *name, *old, *new;
+
+	if (!l_dbus_message_get_arguments(message, "sss",
+						&name, &old, &new))
+		return;
+
+	if (strcmp(name, data->argument))
+		return;
+
+	if (*new == '\0') {
+		if (data->disconnect_func)
+			data->disconnect_func(data->dbus, data->user_data);
+	}
+}
+
+LIB_EXPORT unsigned int l_dbus_add_disconnect_watch(struct l_dbus *dbus,
+					const char *name,
+					l_dbus_watch_func_t disconnect_func,
+					void *user_data,
+					l_dbus_destroy_func_t destroy)
+{
+	struct dbus1_filter_data *data;
+
+	if (!name)
+		return 0;
+
+	data = _dbus1_filter_data_get(dbus, _dbus1_name_owner_changed_filter,
+				DBUS_SERVICE_DBUS, DBUS_PATH_DBUS,
+				DBUS_INTERFACE_DBUS, "NameOwnerChanged",
+				name,
+				disconnect_func,
+				user_data,
+				destroy);
+	if (!data)
+		return 0;
+
+	add_match(data);
+
+	return l_dbus_register(dbus, _dbus1_signal_dispatcher, data,
+							filter_data_destroy);
+}
+
+LIB_EXPORT bool l_dbus_remove_watch(struct l_dbus *dbus, unsigned int id)
+{
+	return l_dbus_unregister(dbus, id);
+}
diff --git a/ell/dbus.h b/ell/dbus.h
index b2469f3..39f926b 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -198,6 +198,13 @@ bool l_dbus_register_interface(struct l_dbus *dbus,
 				l_dbus_destroy_func_t destroy);
 bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
 					const char *interface);
+unsigned int l_dbus_add_disconnect_watch(struct l_dbus *dbus,
+					const char *name,
+					l_dbus_watch_func_t disconnect_func,
+					void *user_data,
+					l_dbus_destroy_func_t destroy);
+bool l_dbus_remove_watch(struct l_dbus *dbus, unsigned int id);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.1.0

[PATCH v5 4/4] unit: dbus: Add test for disconnect watch API

[Jukka Rissanen]

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

Test the dbus disconnect watch support.
---
 unit/test-dbus-watch.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/unit/test-dbus-watch.c b/unit/test-dbus-watch.c
index 3868c8f..104f432 100644
--- a/unit/test-dbus-watch.c
+++ b/unit/test-dbus-watch.c
@@ -159,6 +159,14 @@ static const struct watch_test match_test_9 = {
 	.expected = "type='signal'",
 };
 
+static struct watch_test disconnect_test = {
+	.name = ":101.1",
+	.service = DBUS_SERVICE_DBUS,
+	.path = DBUS_PATH_DBUS,
+	.interface = DBUS_INTERFACE_DBUS,
+	.method = "NameOwnerChanged",
+};
+
 static void test_match(const void *test_data)
 {
 	const struct watch_test *test = test_data;
@@ -183,6 +191,60 @@ static void test_match(const void *test_data)
 	_dbus1_filter_data_destroy(data);
 }
 
+static void disconnect_cb(struct l_dbus *dbus, void *user_data)
+{
+	int *count = user_data;
+
+	(*count)++;
+}
+
+static void send_filter_signal(struct dbus1_filter_data *data,
+			const char *name, const char *old, const char *new)
+{
+	struct l_dbus_message *message;
+
+	message = _dbus_message_new_signal(2,
+					DBUS_PATH_DBUS,
+					DBUS_INTERFACE_DBUS,
+					"NameOwnerChanged");
+	l_dbus_message_set_arguments(message, "sss", name, old, new);
+	_dbus_message_set_sender(message, DBUS_SERVICE_DBUS);
+	_dbus1_signal_dispatcher(message, data);
+	l_dbus_message_unref(message);
+}
+
+static void test_disconnect_watch(const void *test_data)
+{
+	const struct watch_test *test = test_data;
+	struct dbus1_filter_data *data;
+	int count = 0;
+
+	data = _dbus1_filter_data_get(NULL,
+				_dbus1_name_owner_changed_filter,
+				test->service,
+				test->path,
+				test->interface,
+				test->method,
+				test->name,
+				disconnect_cb,
+				&count,
+				NULL);
+
+	send_filter_signal(data, ":0.1", ":0.1", "");
+	assert(count == 0);
+
+	send_filter_signal(data, ":0.1", ":0.1", ":0.2");
+	assert(count == 0);
+
+	send_filter_signal(data, ":101.1", ":101.1", ":101.1");
+	assert(count == 0);
+
+	send_filter_signal(data, ":101.1", ":101.1", "");
+	assert(count == 1);
+
+	_dbus1_filter_data_destroy(data);
+}
+
 int main(int argc, char *argv[])
 {
 	l_test_init(&argc, &argv);
@@ -199,5 +261,8 @@ int main(int argc, char *argv[])
 								&match_test_8);
 	l_test_add("DBus filter NULL all fields", test_match, &match_test_9);
 
+	l_test_add("DBus disconnect watch", test_disconnect_watch,
+						&disconnect_test);
+
 	return l_test_run();
 }
-- 
2.1.0

Re: [PATCH v5 1/4] unit: dbus: Add test for filter rule

[Denis Kenzior]

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

Hi Jukka,

On 02/23/2015 05:54 AM, Jukka Rissanen wrote:
> Testing _dbus1_filter_format_match() function.
> ---
>   Makefile.am            |   3 +
>   unit/test-dbus-watch.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 206 insertions(+)
>   create mode 100644 unit/test-dbus-watch.c

Applied, thanks.

Regards,
-Denis

Re: [PATCH v5 2/4] dbus: Add AddMatch and RemoveMatch support

[Denis Kenzior]

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

Hi Jukka,

On 02/23/2015 05:54 AM, Jukka Rissanen wrote:
> We do not check for errors as they are typically unrecoverable
> in this case and in most cases ignored anyway.
>
> This is the comment from libdbus API documentation for
> dbus_bus_add_match() about error checking:
>
> "If you pass NULL for the error, this function will not block;
> the match thus won't be added until you flush the connection,
> and if there's an error adding the match you won't find out
> about it. This is generally acceptable, since the possible
> errors (including a lack of resources in the bus, the connection
> having exceeded its quota of active match rules, or the match
> rule being unparseable) are generally unrecoverable."
> ---
>   ell/dbus.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>

Applied, thanks.

Regards,
-Denis

Re: [PATCH v5 3/4] dbus: Add disconnect watch support

[Denis Kenzior]

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

Hi Jukka,

On 02/23/2015 05:54 AM, Jukka Rissanen wrote:
> Allow caller to monitor the disconnect status of a service.
> ---
>   ell/dbus-private.h |   3 ++
>   ell/dbus.c         | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ell/dbus.h         |   7 ++++
>   3 files changed, 118 insertions(+)
>

Applied, thanks.

Regards,
-Denis

Re: [PATCH v5 4/4] unit: dbus: Add test for disconnect watch API

[Denis Kenzior]

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

Hi Jukka,

On 02/23/2015 05:55 AM, Jukka Rissanen wrote:
> Test the dbus disconnect watch support.
> ---
>   unit/test-dbus-watch.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 65 insertions(+)
>

Applied, thanks.

Regards,
-Denis