[PATCH v4 0/6] Add DBus disconnect watch support

[PATCH v4 0/6] Add DBus disconnect watch support

[Jukka Rissanen]

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

Hi,

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 (6):
  dbus: Add AddMatch and RemoveMatch support
  gvariant: dbus.h need to be included before private header
  dbus: Add filter rule creator
  unit: dbus: Add test for filter rule
  dbus: Add disconnect watch support
  unit: dbus: Add test for disconnect watch API

 Makefile.am            |   3 +
 ell/dbus-private.h     |  19 +++++
 ell/dbus.c             | 217 +++++++++++++++++++++++++++++++++++++++++++++++++
 ell/dbus.h             |   9 ++
 ell/gvariant-util.c    |   2 +-
 unit/test-dbus-watch.c | 166 +++++++++++++++++++++++++++++++++++++
 6 files changed, 415 insertions(+), 1 deletion(-)
 create mode 100644 unit/test-dbus-watch.c

-- 
2.1.0

[PATCH v4 1/6] dbus: Add AddMatch and RemoveMatch support

[Jukka Rissanen]

[-- Attachment #1: Type: text/plain, Size: 1693 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 4c4ba2d..1d1b166 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -1227,3 +1227,29 @@ LIB_EXPORT bool l_dbus_unregister_interface(struct l_dbus *dbus,
 
 	return _dbus_object_tree_unregister(dbus->tree, path, interface);
 }
+
+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 v4 2/6] gvariant: dbus.h need to be included before private header

[Jukka Rissanen]

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

Needed by dbus watcher unit tests which add stuff to private
header file.
---
 ell/gvariant-util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ell/gvariant-util.c b/ell/gvariant-util.c
index 8fb32c7..7534509 100644
--- a/ell/gvariant-util.c
+++ b/ell/gvariant-util.c
@@ -37,9 +37,9 @@
 #include "queue.h"
 #include "string.h"
 #include "log.h"
+#include "dbus.h"
 #include "dbus-private.h"
 #include "gvariant-private.h"
-#include "dbus.h"
 
 static const char *simple_types = "sogybnqiuxtdh";
 static const char *variable_types = "sogav";
-- 
2.1.0

[PATCH v4 3/6] dbus: Add filter rule creator

[Jukka Rissanen]

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

Create a dbus filter rule here so that the following unit
test is able to test it.
---
 ell/dbus-private.h |  5 +++++
 ell/dbus.c         | 37 +++++++++++++++++++++++++++++++++++++
 ell/dbus.h         |  2 ++
 3 files changed, 44 insertions(+)

diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 9987294..0123ddb 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -212,3 +212,8 @@ int _dbus_kernel_remove_match(int fd, uint64_t cookie);
 
 uint8_t _dbus_get_version(struct l_dbus *dbus);
 int _dbus_get_fd(struct l_dbus *dbus);
+
+struct dbus_filter_data;
+
+void _dbus1_format_rule(struct dbus_filter_data *data, char *rule,
+								size_t size);
diff --git a/ell/dbus.c b/ell/dbus.c
index 1d1b166..a3d8070 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -122,6 +122,19 @@ struct signal_callback {
 	void *user_data;
 };
 
+struct dbus_filter_data {
+	struct l_dbus *dbus;
+	l_dbus_message_func_t handle_func;
+	l_dbus_watch_func_t disconnect_func;
+	char *sender;
+	char *path;
+	char *interface;
+	char *member;
+	char *argument;
+	void *user_data;
+	l_dbus_destroy_func_t destroy_func;
+};
+
 static void message_queue_destroy(void *data)
 {
 	struct message_callback *callback = data;
@@ -1253,3 +1266,27 @@ static void dbus1_bus_remove_match(struct l_dbus *dbus, const char *rule)
 {
 	dbus1_send_match(dbus, rule, "RemoveMatch");
 }
+
+void _dbus1_format_rule(struct dbus_filter_data *data, char *rule,
+			size_t size)
+{
+	int offset;
+
+	offset = snprintf(rule, size, "type='signal'");
+
+	if (data->sender)
+		offset += snprintf(rule + offset, size - offset,
+				",sender='%s'", data->sender);
+	if (data->path)
+		offset += snprintf(rule + offset, size - offset,
+				",path='%s'", data->path);
+	if (data->interface)
+		offset += snprintf(rule + offset, size - offset,
+				",interface='%s'", data->interface);
+	if (data->member)
+		offset += snprintf(rule + offset, size - offset,
+				",member='%s'", data->member);
+	if (data->argument)
+		snprintf(rule + offset, size - offset,
+				",arg0='%s'", data->argument);
+}
diff --git a/ell/dbus.h b/ell/dbus.h
index a6ad109..b2469f3 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -48,6 +48,8 @@ typedef void (*l_dbus_debug_func_t) (const char *str, void *user_data);
 typedef void (*l_dbus_destroy_func_t) (void *user_data);
 typedef void (*l_dbus_interface_setup_func_t) (struct l_dbus_interface *);
 
+typedef void (*l_dbus_watch_func_t) (struct l_dbus *dbus, void *user_data);
+
 struct l_dbus *l_dbus_new(const char *address);
 struct l_dbus *l_dbus_new_default(enum l_dbus_bus bus);
 void l_dbus_destroy(struct l_dbus *dbus);
-- 
2.1.0

[PATCH v4 4/6] unit: dbus: Add test for filter rule

[Jukka Rissanen]

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

Testing the _dbus1_format_rule() function.
---
 Makefile.am            |   3 ++
 unit/test-dbus-watch.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 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..e1d3f9d
--- /dev/null
+++ b/unit/test-dbus-watch.c
@@ -0,0 +1,109 @@
+/*
+ *
+ *  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 dbus_filter_data {
+	struct l_dbus *dbus;
+	l_dbus_message_func_t handle_func;
+	l_dbus_watch_func_t disconnect_func;
+	char *sender;
+	char *path;
+	char *interface;
+	char *member;
+	char *argument;
+	void *user_data;
+	l_dbus_destroy_func_t destroy_func;
+};
+
+struct watch_test {
+	const char *name;
+	const char *expected;
+};
+
+static const struct watch_test match_test = {
+	.name = ":1.101",
+	.expected = "type='signal',"
+		"sender='org.freedesktop.DBus',"
+		"path='/org/freedesktop/DBus',"
+		"interface='org.freedesktop.DBus',"
+		"member='NameOwnerChanged',"
+		"arg0=':1.101'",
+};
+
+static void setup_data(struct dbus_filter_data *data, const char *name)
+{
+	data->sender = l_strdup(DBUS_SERVICE_DBUS);
+	data->path = l_strdup(DBUS_PATH_DBUS);
+	data->interface = l_strdup(DBUS_INTERFACE_DBUS);
+	data->member = l_strdup("NameOwnerChanged");
+	data->argument = l_strdup(name);
+}
+
+static void clean_data(struct dbus_filter_data *data)
+{
+	l_free(data->sender);
+	l_free(data->path);
+	l_free(data->interface);
+	l_free(data->member);
+	l_free(data->argument);
+}
+static void test_match(const void *test_data)
+{
+	const struct watch_test *test = test_data;
+	struct dbus_filter_data data;
+	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
+
+	setup_data(&data, test->name);
+
+	_dbus1_format_rule(&data, rule, sizeof(rule));
+
+	assert(strcmp(rule, test->expected) == 0);
+
+	clean_data(&data);
+}
+
+int main(int argc, char *argv[])
+{
+	l_test_init(&argc, &argv);
+
+	l_test_add("DBus filter", test_match, &match_test);
+
+	return l_test_run();
+}
-- 
2.1.0

[PATCH v4 5/6] dbus: Add disconnect watch support

[Jukka Rissanen]

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

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

diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 0123ddb..868bfb5 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -217,3 +217,17 @@ struct dbus_filter_data;
 
 void _dbus1_format_rule(struct dbus_filter_data *data, char *rule,
 								size_t size);
+void _dbus1_signal_dispatcher(struct l_dbus_message *message, void *user_data);
+struct dbus_filter_data *_dbus1_filter_data_get(struct l_dbus *dbus,
+					l_dbus_message_func_t filter,
+					const char *sender,
+					const char *path,
+					const char *interface,
+					const char *member,
+					const char *argument,
+					l_dbus_watch_func_t disconnect_func,
+					void *user_data,
+					l_dbus_destroy_func_t destroy);
+void _dbus1_filter_data_destroy(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 a3d8070..361d462 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,
@@ -1290,3 +1292,155 @@ void _dbus1_format_rule(struct dbus_filter_data *data, char *rule,
 		snprintf(rule + offset, size - offset,
 				",arg0='%s'", data->argument);
 }
+
+static void add_match(struct dbus_filter_data *data)
+{
+	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
+
+	_dbus1_format_rule(data, rule, sizeof(rule));
+
+	dbus1_bus_add_match(data->dbus, rule);
+}
+
+static void remove_match(struct dbus_filter_data *data)
+{
+	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
+
+	_dbus1_format_rule(data, rule, sizeof(rule));
+
+	dbus1_bus_remove_match(data->dbus, rule);
+}
+
+struct dbus_filter_data *_dbus1_filter_data_get(struct l_dbus *dbus,
+					l_dbus_message_func_t filter,
+					const char *sender,
+					const char *path,
+					const char *interface,
+					const char *member,
+					const char *argument,
+					l_dbus_watch_func_t disconnect_func,
+					void *user_data,
+					l_dbus_destroy_func_t destroy)
+{
+	struct dbus_filter_data *data;
+
+	data = l_new(struct dbus_filter_data, 1);
+
+	data->dbus = dbus;
+	data->handle_func = filter;
+	data->disconnect_func = disconnect_func;
+	data->sender = l_strdup(sender);
+	data->path = l_strdup(path);
+	data->interface = l_strdup(interface);
+	data->member = l_strdup(member);
+	data->argument = l_strdup(argument);
+	data->user_data = user_data;
+	data->destroy_func = destroy;
+
+	return data;
+}
+
+void _dbus1_filter_data_destroy(void *user_data)
+{
+	struct dbus_filter_data *data = user_data;
+
+	l_free(data->sender);
+	l_free(data->path);
+	l_free(data->interface);
+	l_free(data->member);
+	l_free(data->argument);
+
+	if (data->destroy_func)
+		data->destroy_func(data->user_data);
+
+	l_free(data);
+}
+
+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 dbus_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 dbus_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 dbus_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 v4 6/6] unit: dbus: Add test for disconnect watch API

[Jukka Rissanen]

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

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

diff --git a/unit/test-dbus-watch.c b/unit/test-dbus-watch.c
index e1d3f9d..1eeb6ea 100644
--- a/unit/test-dbus-watch.c
+++ b/unit/test-dbus-watch.c
@@ -67,6 +67,10 @@ static const struct watch_test match_test = {
 		"arg0=':1.101'",
 };
 
+static struct watch_test disconnect_test = {
+	.name = ":101.1",
+};
+
 static void setup_data(struct dbus_filter_data *data, const char *name)
 {
 	data->sender = l_strdup(DBUS_SERVICE_DBUS);
@@ -99,11 +103,64 @@ static void test_match(const void *test_data)
 	clean_data(&data);
 }
 
+static void disconnect_cb(struct l_dbus *dbus, void *user_data)
+{
+	int *count = user_data;
+
+	(*count)++;
+}
+
+static void send_filter_signal(struct dbus_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 dbus_filter_data *data;
+	int count = 0;
+
+	data = _dbus1_filter_data_get(NULL, _dbus1_name_owner_changed_filter,
+				DBUS_SERVICE_DBUS, DBUS_PATH_DBUS,
+				DBUS_INTERFACE_DBUS, "NameOwnerChanged",
+				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);
 
 	l_test_add("DBus filter", test_match, &match_test);
 
+	l_test_add("DBus disconnect watch", test_disconnect_watch,
+						&disconnect_test);
+
 	return l_test_run();
 }
-- 
2.1.0

Re: [PATCH v4 2/6] gvariant: dbus.h need to be included before private header

[Denis Kenzior]

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

Hi Jukka,

On 02/20/2015 06:16 AM, Jukka Rissanen wrote:
> Needed by dbus watcher unit tests which add stuff to private
> header file.
> ---
>   ell/gvariant-util.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>

Applied, thanks.

Regards,
-Denis

Re: [PATCH v4 5/6] dbus: Add disconnect watch support

[Denis Kenzior]

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

Hi Jukka,

On 02/20/2015 06:16 AM, Jukka Rissanen wrote:
> Allow caller to monitor the disconnect status of a service.
> ---
>   ell/dbus-private.h |  14 +++++
>   ell/dbus.c         | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ell/dbus.h         |   7 +++
>   3 files changed, 175 insertions(+)
>

I cherry picked parts of this patch and applied it.  Please rebase the 
rest and resubmit.

Regards,
-Denis

Re: [PATCH v4 3/6] dbus: Add filter rule creator

[Denis Kenzior]

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

Hi Jukka,

On 02/20/2015 06:16 AM, Jukka Rissanen wrote:
> Create a dbus filter rule here so that the following unit
> test is able to test it.
> ---
>   ell/dbus-private.h |  5 +++++
>   ell/dbus.c         | 37 +++++++++++++++++++++++++++++++++++++
>   ell/dbus.h         |  2 ++
>   3 files changed, 44 insertions(+)
>

I applied this patch mostly intact, but amended it slightly afterwards.

Regards,
-Denis

Re: [PATCH v4 4/6] unit: dbus: Add test for filter rule

[Denis Kenzior]

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

Hi Jukka,

On 02/20/2015 06:16 AM, Jukka Rissanen wrote:
> Testing the _dbus1_format_rule() function.
> ---
>   Makefile.am            |   3 ++
>   unit/test-dbus-watch.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 112 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..e1d3f9d
> --- /dev/null
> +++ b/unit/test-dbus-watch.c
> @@ -0,0 +1,109 @@
> +/*
> + *
> + *  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 dbus_filter_data {
> +	struct l_dbus *dbus;
> +	l_dbus_message_func_t handle_func;
> +	l_dbus_watch_func_t disconnect_func;
> +	char *sender;
> +	char *path;
> +	char *interface;
> +	char *member;
> +	char *argument;
> +	void *user_data;
> +	l_dbus_destroy_func_t destroy_func;
> +};

Don't ever do this.  If you need to use an opaque data structure, ask 
yourself why.  If it is genuinely required, then expose that in the 
header file.  Do not re-define opaque data structures in a unit test.

> +
> +struct watch_test {
> +	const char *name;
> +	const char *expected;
> +};
> +
> +static const struct watch_test match_test = {
> +	.name = ":1.101",
> +	.expected = "type='signal',"
> +		"sender='org.freedesktop.DBus',"
> +		"path='/org/freedesktop/DBus',"
> +		"interface='org.freedesktop.DBus',"
> +		"member='NameOwnerChanged',"
> +		"arg0=':1.101'",
> +};
> +
> +static void setup_data(struct dbus_filter_data *data, const char *name)
> +{
> +	data->sender = l_strdup(DBUS_SERVICE_DBUS);
> +	data->path = l_strdup(DBUS_PATH_DBUS);
> +	data->interface = l_strdup(DBUS_INTERFACE_DBUS);
> +	data->member = l_strdup("NameOwnerChanged");
> +	data->argument = l_strdup(name);
> +}
> +
> +static void clean_data(struct dbus_filter_data *data)
> +{
> +	l_free(data->sender);
> +	l_free(data->path);
> +	l_free(data->interface);
> +	l_free(data->member);
> +	l_free(data->argument);
> +}
> +static void test_match(const void *test_data)
> +{
> +	const struct watch_test *test = test_data;
> +	struct dbus_filter_data data;
> +	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
> +
> +	setup_data(&data, test->name);

Use _dbus1_filter_data_get

> +
> +	_dbus1_format_rule(&data, rule, sizeof(rule));
> +
> +	assert(strcmp(rule, test->expected) == 0);
> +
> +	clean_data(&data);

Use _dbus1_filter_data_destroy

> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	l_test_init(&argc, &argv);
> +
> +	l_test_add("DBus filter", test_match, &match_test);

Add more cases please, especially focusing on each type of filter 
combination.  E.g. interface only, member only, interface + member, etc.

> +
> +	return l_test_run();
> +}
>

Regards,
-Denis