[RFC 0/6] D-Bus client and proxy API

[RFC 0/6] D-Bus client and proxy API

[Szymon Janc]

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

Hi,

This set adds initial version of D-Bus client and proxy APIs.
With this is it possible to connect to specified service, monitor
proxies being added/removed, set/get properties and call methods
on specific proxy.

First 3 patches are some extensions to message API that were needed for
client code.

This is tested against BlueZ D-Bus API and seems to work fine within
my (simple) application.

Comments are welcome.

Szymon Janc (6):
  dbus: Add l_dbus_message_get_arguments_valist
  dbus: Add l_dbus_message_builder_append_from_valist
  dbus: Use l_dbus_message_builder_append_from_valist in
    append_arguments
  dbus: Add support for proxy interface
  dbus: Add support for setting properties on D-Bus proxy interface
  dbus: Add convenient API for calling method on proxy interface

 Makefile.am        |   2 +
 ell/dbus-client.c  | 732 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 ell/dbus-client.h  | 102 ++++++++
 ell/dbus-message.c | 476 +++++++++++++++++-----------------
 ell/dbus.h         |   6 +
 ell/ell.h          |   1 +
 6 files changed, 1088 insertions(+), 231 deletions(-)
 create mode 100644 ell/dbus-client.c
 create mode 100644 ell/dbus-client.h

-- 
2.14.3

[RFC 1/6] dbus: Add l_dbus_message_get_arguments_valist

[Szymon Janc]

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

This is a variant of l_dbus_message_get_arguments() that accepts
va_list as parameter.
---
 ell/dbus-message.c | 18 +++++++++++++-----
 ell/dbus.h         |  2 ++
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/ell/dbus-message.c b/ell/dbus-message.c
index 7653c03..0314a07 100644
--- a/ell/dbus-message.c
+++ b/ell/dbus-message.c
@@ -1372,12 +1372,11 @@ LIB_EXPORT bool l_dbus_message_is_error(struct l_dbus_message *message)
 	return hdr->message_type == DBUS_MESSAGE_TYPE_ERROR;
 }
 
-LIB_EXPORT bool l_dbus_message_get_arguments(struct l_dbus_message *message,
-						const char *signature, ...)
+LIB_EXPORT bool l_dbus_message_get_arguments_valist(
+					struct l_dbus_message *message,
+					const char *signature, va_list args)
 {
 	struct l_dbus_message_iter iter;
-	va_list args;
-	bool result;
 
 	if (unlikely(!message))
 		return false;
@@ -1402,8 +1401,17 @@ LIB_EXPORT bool l_dbus_message_get_arguments(struct l_dbus_message *message,
 		_dbus1_iter_init(&iter, message, message->signature, NULL,
 				message->body, message->body_size);
 
+	return message_iter_next_entry_valist(&iter, args);
+}
+
+LIB_EXPORT bool l_dbus_message_get_arguments(struct l_dbus_message *message,
+						const char *signature, ...)
+{
+	va_list args;
+	bool result;
+
 	va_start(args, signature);
-	result = message_iter_next_entry_valist(&iter, args);
+	result = l_dbus_message_get_arguments_valist(message, signature, args);
 	va_end(args);
 
 	return result;
diff --git a/ell/dbus.h b/ell/dbus.h
index 6705edd..5c7879f 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -164,6 +164,8 @@ bool l_dbus_message_get_error(struct l_dbus_message *message,
 					const char **name, const char **text);
 bool l_dbus_message_get_arguments(struct l_dbus_message *message,
 						const char *signature, ...);
+bool l_dbus_message_get_arguments_valist(struct l_dbus_message *message,
+					const char *signature, va_list args);
 
 bool l_dbus_message_iter_next_entry(struct l_dbus_message_iter *iter, ...);
 bool l_dbus_message_iter_get_variant(struct l_dbus_message_iter *iter,
-- 
2.14.3

Re: [RFC 1/6] dbus: Add l_dbus_message_get_arguments_valist

[Denis Kenzior]

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

Hi Szymon,

On 11/29/2017 09:16 AM, Szymon Janc wrote:
> This is a variant of l_dbus_message_get_arguments() that accepts
> va_list as parameter.
> ---
>   ell/dbus-message.c | 18 +++++++++++++-----
>   ell/dbus.h         |  2 ++
>   2 files changed, 15 insertions(+), 5 deletions(-)
> 

Applied, thanks.

Regards,
-Denis

[RFC 2/6] dbus: Add l_dbus_message_builder_append_from_valist

[Szymon Janc]

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

This allows to build messages from va_list.
---
 ell/dbus-message.c | 220 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 ell/dbus.h         |   4 +
 2 files changed, 224 insertions(+)

diff --git a/ell/dbus-message.c b/ell/dbus-message.c
index 0314a07..e46ce99 100644
--- a/ell/dbus-message.c
+++ b/ell/dbus-message.c
@@ -1930,6 +1930,226 @@ LIB_EXPORT bool l_dbus_message_builder_append_from_iter(
 	return true;
 }
 
+LIB_EXPORT bool l_dbus_message_builder_append_from_valist(
+					struct l_dbus_message_builder *builder,
+					const char *signature, va_list args)
+{
+	struct builder_driver *driver;
+	char subsig[256];
+	const char *sigend;
+	/* Nesting requires an extra stack entry for the base level */
+	struct container stack[DBUS_MAX_NESTING + 1];
+	unsigned int stack_index = 0;
+
+	if (unlikely(!builder))
+		return false;
+
+	driver = builder->driver;
+
+	stack[stack_index].type = DBUS_CONTAINER_TYPE_STRUCT;
+	stack[stack_index].sig_start = signature;
+	stack[stack_index].sig_end = signature + strlen(signature);
+	stack[stack_index].n_items = 0;
+
+	while (stack_index != 0 || stack[0].sig_start != stack[0].sig_end) {
+		const char *s;
+		const char *str;
+
+		if (stack[stack_index].type == DBUS_CONTAINER_TYPE_ARRAY &&
+				stack[stack_index].n_items == 0)
+			stack[stack_index].sig_start =
+				stack[stack_index].sig_end;
+
+		if (stack[stack_index].sig_start ==
+				stack[stack_index].sig_end) {
+			bool ret;
+
+			/*
+			 * Sanity check in case of an invalid signature that
+			 * isn't caught elsewhere.
+			 */
+			if (unlikely(stack_index == 0))
+				return false;
+
+			switch (stack[stack_index].type) {
+			case DBUS_CONTAINER_TYPE_STRUCT:
+				ret = driver->leave_struct(builder->builder);
+				break;
+			case DBUS_CONTAINER_TYPE_DICT_ENTRY:
+				ret = driver->leave_dict(builder->builder);
+				break;
+			case DBUS_CONTAINER_TYPE_VARIANT:
+				ret = driver->leave_variant(builder->builder);
+				break;
+			case DBUS_CONTAINER_TYPE_ARRAY:
+				ret = driver->leave_array(builder->builder);
+				break;
+			default:
+				ret = false;
+			}
+
+			if (!ret)
+				return false;
+
+			stack_index -= 1;
+			continue;
+		}
+
+		s = stack[stack_index].sig_start;
+
+		if (stack[stack_index].type != DBUS_CONTAINER_TYPE_ARRAY)
+			stack[stack_index].sig_start += 1;
+		else
+			stack[stack_index].n_items -= 1;
+
+		switch (*s) {
+		case 'o':
+		case 's':
+		case 'g':
+			str = va_arg(args, const char *);
+
+			if (!driver->append_basic(builder->builder, *s, str))
+				return false;
+			break;
+		case 'b':
+		case 'y':
+		{
+			uint8_t y = (uint8_t) va_arg(args, int);
+
+			if (!driver->append_basic(builder->builder, *s, &y))
+				return false;
+
+			break;
+		}
+		case 'n':
+		case 'q':
+		{
+			uint16_t n = (uint16_t) va_arg(args, int);
+
+			if (!driver->append_basic(builder->builder, *s, &n))
+				return false;
+
+			break;
+		}
+		case 'i':
+		case 'u':
+		{
+			uint32_t u = va_arg(args, uint32_t);
+
+			if (!driver->append_basic(builder->builder, *s, &u))
+				return false;
+
+			break;
+		}
+		case 'h':
+		{
+			int fd = va_arg(args, int);
+			struct l_dbus_message *message = builder->message;
+
+			if (!driver->append_basic(builder->builder, *s,
+							&message->num_fds))
+				return false;
+
+			if (message->num_fds < L_ARRAY_SIZE(message->fds))
+				message->fds[message->num_fds++] =
+					fcntl(fd, F_DUPFD_CLOEXEC, 3);
+
+			break;
+		}
+		case 'x':
+		case 't':
+		{
+			uint64_t x = va_arg(args, uint64_t);
+
+			if (!driver->append_basic(builder->builder, *s, &x))
+				return false;
+			break;
+		}
+		case 'd':
+		{
+			double d = va_arg(args, double);
+
+			if (!driver->append_basic(builder->builder, *s, &d))
+				return false;
+			break;
+		}
+		case '(':
+		case '{':
+			if (stack_index == DBUS_MAX_NESTING)
+				return false;
+
+			sigend = _dbus_signature_end(s);
+			memcpy(subsig, s + 1, sigend - s - 1);
+			subsig[sigend - s - 1] = '\0';
+
+			if (*s == '(' && !driver->enter_struct(builder->builder, subsig))
+				return false;
+
+			if (*s == '{' && !driver->enter_dict(builder->builder, subsig))
+				return false;
+
+			if (stack[stack_index].type !=
+					DBUS_CONTAINER_TYPE_ARRAY)
+				stack[stack_index].sig_start = sigend + 1;
+
+			stack_index += 1;
+			stack[stack_index].sig_start = s + 1;
+			stack[stack_index].sig_end = sigend;
+			stack[stack_index].n_items = 0;
+			stack[stack_index].type = *s == '(' ?
+						DBUS_CONTAINER_TYPE_STRUCT :
+						DBUS_CONTAINER_TYPE_DICT_ENTRY;
+
+			break;
+		case 'v':
+			if (stack_index == DBUS_MAX_NESTING)
+				return false;
+
+			str = va_arg(args, const char *);
+
+			if (!str)
+				return false;
+
+			if (!driver->enter_variant(builder->builder, str))
+				return false;
+
+			stack_index += 1;
+			stack[stack_index].type = DBUS_CONTAINER_TYPE_VARIANT;
+			stack[stack_index].sig_start = str;
+			stack[stack_index].sig_end = str + strlen(str);
+			stack[stack_index].n_items = 0;
+
+			break;
+		case 'a':
+			if (stack_index == DBUS_MAX_NESTING)
+				return false;
+
+			sigend = _dbus_signature_end(s + 1) + 1;
+			memcpy(subsig, s + 1, sigend - s - 1);
+			subsig[sigend - s - 1] = '\0';
+
+			if (!driver->enter_array(builder->builder, subsig))
+				return false;
+
+			if (stack[stack_index].type !=
+					DBUS_CONTAINER_TYPE_ARRAY)
+				stack[stack_index].sig_start = sigend;
+
+			stack_index += 1;
+			stack[stack_index].sig_start = s + 1;
+			stack[stack_index].sig_end = sigend;
+			stack[stack_index].n_items = va_arg(args, unsigned int);
+			stack[stack_index].type = DBUS_CONTAINER_TYPE_ARRAY;
+
+			break;
+		default:
+			return false;
+		}
+	}
+
+	return true;
+}
+
 LIB_EXPORT struct l_dbus_message *l_dbus_message_builder_finalize(
 					struct l_dbus_message_builder *builder)
 {
diff --git a/ell/dbus.h b/ell/dbus.h
index 5c7879f..1b1ec2b 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -214,6 +214,10 @@ bool l_dbus_message_builder_append_from_iter(
 					struct l_dbus_message_builder *builder,
 					struct l_dbus_message_iter *from);
 
+bool l_dbus_message_builder_append_from_valist(
+					struct l_dbus_message_builder *builder,
+					const char *signature, va_list args);
+
 struct l_dbus_message *l_dbus_message_builder_finalize(
 					struct l_dbus_message_builder *builder);
 
-- 
2.14.3

[RFC 3/6] dbus: Use l_dbus_message_builder_append_from_valist in append_arguments

[Szymon Janc]

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

Those functions do similar things and code duplication can be avoided.
---
 ell/dbus-message.c | 238 +++--------------------------------------------------
 1 file changed, 12 insertions(+), 226 deletions(-)

diff --git a/ell/dbus-message.c b/ell/dbus-message.c
index e46ce99..9878de5 100644
--- a/ell/dbus-message.c
+++ b/ell/dbus-message.c
@@ -1089,239 +1089,25 @@ struct container {
 static bool append_arguments(struct l_dbus_message *message,
 					const char *signature, va_list args)
 {
-	struct dbus_builder *builder;
-	struct builder_driver *driver;
-	char *generated_signature;
-	char subsig[256];
-	const char *sigend;
-	/* Nesting requires an extra stack entry for the base level */
-	struct container stack[DBUS_MAX_NESTING + 1];
-	unsigned int stack_index = 0;
-
-	if (_dbus_message_is_gvariant(message))
-		driver = &gvariant_driver;
-	else
-		driver = &dbus1_driver;
-
-	builder = driver->new(NULL, 0);
-
-	stack[stack_index].type = DBUS_CONTAINER_TYPE_STRUCT;
-	stack[stack_index].sig_start = signature;
-	stack[stack_index].sig_end = signature + strlen(signature);
-	stack[stack_index].n_items = 0;
-
-	while (stack_index != 0 || stack[0].sig_start != stack[0].sig_end) {
-		const char *s;
-		const char *str;
-
-		if (stack[stack_index].type == DBUS_CONTAINER_TYPE_ARRAY &&
-				stack[stack_index].n_items == 0)
-			stack[stack_index].sig_start =
-				stack[stack_index].sig_end;
-
-		if (stack[stack_index].sig_start ==
-				stack[stack_index].sig_end) {
-			bool ret;
-
-			/*
-			 * Sanity check in case of an invalid signature that
-			 * isn't caught elsewhere.
-			 */
-			if (unlikely(stack_index == 0))
-				goto error;
-
-			switch (stack[stack_index].type) {
-			case DBUS_CONTAINER_TYPE_STRUCT:
-				ret = driver->leave_struct(builder);
-				break;
-			case DBUS_CONTAINER_TYPE_DICT_ENTRY:
-				ret = driver->leave_dict(builder);
-				break;
-			case DBUS_CONTAINER_TYPE_VARIANT:
-				ret = driver->leave_variant(builder);
-				break;
-			case DBUS_CONTAINER_TYPE_ARRAY:
-				ret = driver->leave_array(builder);
-				break;
-			default:
-				ret = false;
-			}
-
-			if (!ret)
-				goto error;
-
-			stack_index -= 1;
-			continue;
-		}
-
-		s = stack[stack_index].sig_start;
-
-		if (stack[stack_index].type != DBUS_CONTAINER_TYPE_ARRAY)
-			stack[stack_index].sig_start += 1;
-		else
-			stack[stack_index].n_items -= 1;
-
-		switch (*s) {
-		case 'o':
-		case 's':
-		case 'g':
-			str = va_arg(args, const char *);
-
-			if (!driver->append_basic(builder, *s, str))
-				goto error;
-			break;
-		case 'b':
-		case 'y':
-		{
-			uint8_t y = (uint8_t) va_arg(args, int);
-
-			if (!driver->append_basic(builder, *s, &y))
-				goto error;
-
-			break;
-		}
-		case 'n':
-		case 'q':
-		{
-			uint16_t n = (uint16_t) va_arg(args, int);
-
-			if (!driver->append_basic(builder, *s, &n))
-				goto error;
-
-			break;
-		}
-		case 'i':
-		case 'u':
-		{
-			uint32_t u = va_arg(args, uint32_t);
-
-			if (!driver->append_basic(builder, *s, &u))
-				goto error;
-
-			break;
-		}
-		case 'h':
-		{
-			int fd = va_arg(args, int);
-
-			if (!driver->append_basic(builder, *s,
-							&message->num_fds))
-				goto error;
-
-			if (message->num_fds < L_ARRAY_SIZE(message->fds))
-				message->fds[message->num_fds++] =
-					fcntl(fd, F_DUPFD_CLOEXEC, 3);
-
-			break;
-		}
-		case 'x':
-		case 't':
-		{
-			uint64_t x = va_arg(args, uint64_t);
-
-			if (!driver->append_basic(builder, *s, &x))
-				goto error;
-			break;
-		}
-		case 'd':
-		{
-			double d = va_arg(args, double);
-
-			if (!driver->append_basic(builder, *s, &d))
-				goto error;
-			break;
-		}
-		case '(':
-		case '{':
-			if (stack_index == DBUS_MAX_NESTING)
-				goto error;
-
-			sigend = _dbus_signature_end(s);
-			memcpy(subsig, s + 1, sigend - s - 1);
-			subsig[sigend - s - 1] = '\0';
-
-			if (*s == '(' && !driver->enter_struct(builder, subsig))
-				goto error;
-
-			if (*s == '{' && !driver->enter_dict(builder, subsig))
-				goto error;
-
-			if (stack[stack_index].type !=
-					DBUS_CONTAINER_TYPE_ARRAY)
-				stack[stack_index].sig_start = sigend + 1;
-
-			stack_index += 1;
-			stack[stack_index].sig_start = s + 1;
-			stack[stack_index].sig_end = sigend;
-			stack[stack_index].n_items = 0;
-			stack[stack_index].type = *s == '(' ?
-						DBUS_CONTAINER_TYPE_STRUCT :
-						DBUS_CONTAINER_TYPE_DICT_ENTRY;
-
-			break;
-		case 'v':
-			if (stack_index == DBUS_MAX_NESTING)
-				goto error;
-
-			str = va_arg(args, const char *);
-
-			if (!str)
-				goto error;
-
-			if (!driver->enter_variant(builder, str))
-				goto error;
-
-			stack_index += 1;
-			stack[stack_index].type = DBUS_CONTAINER_TYPE_VARIANT;
-			stack[stack_index].sig_start = str;
-			stack[stack_index].sig_end = str + strlen(str);
-			stack[stack_index].n_items = 0;
-
-			break;
-		case 'a':
-			if (stack_index == DBUS_MAX_NESTING)
-				goto error;
-
-			sigend = _dbus_signature_end(s + 1) + 1;
-			memcpy(subsig, s + 1, sigend - s - 1);
-			subsig[sigend - s - 1] = '\0';
+	struct l_dbus_message_builder *builder;
+	bool ret;
 
-			if (!driver->enter_array(builder, subsig))
-				goto error;
-
-			if (stack[stack_index].type !=
-					DBUS_CONTAINER_TYPE_ARRAY)
-				stack[stack_index].sig_start = sigend;
-
-			stack_index += 1;
-			stack[stack_index].sig_start = s + 1;
-			stack[stack_index].sig_end = sigend;
-			stack[stack_index].n_items = va_arg(args, unsigned int);
-			stack[stack_index].type = DBUS_CONTAINER_TYPE_ARRAY;
+	builder = l_dbus_message_builder_new(message);
+	if (!builder)
+		return false;
 
-			break;
-		default:
-			goto error;
-		}
+	if (!l_dbus_message_builder_append_from_valist(builder, signature, args)) {
+		l_dbus_message_builder_destroy(builder);
+		return false;
 	}
 
-	generated_signature = driver->finish(builder, &message->body,
-						&message->body_size);
-	driver->free(builder);
+	l_dbus_message_builder_finalize(builder);
 
-	if (strcmp(signature, generated_signature))
-		return false;
+	ret = strcmp(signature, builder->message->signature) == 0;
 
-	build_header(message, signature);
-	message->sealed = true;
-	message->signature = generated_signature;
-	message->signature_free = true;
+	l_dbus_message_builder_destroy(builder);
 
-	return true;
-
-error:
-	driver->free(builder);
-	return false;
+	return ret;
 }
 
 LIB_EXPORT bool l_dbus_message_get_error(struct l_dbus_message *message,
-- 
2.14.3

[RFC 4/6] dbus: Add support for proxy interface

[Szymon Janc]

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

---
 Makefile.am       |   2 +
 ell/dbus-client.c | 595 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ell/dbus-client.h |  87 ++++++++
 ell/ell.h         |   1 +
 4 files changed, 685 insertions(+)
 create mode 100644 ell/dbus-client.c
 create mode 100644 ell/dbus-client.h

diff --git a/Makefile.am b/Makefile.am
index a95bac4..19b957e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -32,6 +32,7 @@ pkginclude_HEADERS = ell/ell.h \
 			ell/genl.h \
 			ell/dbus.h \
 			ell/dbus-service.h \
+			ell/dbus-client.h \
 			ell/hwdb.h \
 			ell/cipher.h \
 			ell/random.h \
@@ -74,6 +75,7 @@ ell_libell_la_SOURCES = $(linux_headers) \
 			ell/dbus-message.c \
 			ell/dbus-util.c \
 			ell/dbus-service.c \
+			ell/dbus-client.c \
 			ell/dbus-name-cache.c \
 			ell/dbus-filter.c \
 			ell/gvariant-private.h \
diff --git a/ell/dbus-client.c b/ell/dbus-client.c
new file mode 100644
index 0000000..e6b6ff3
--- /dev/null
+++ b/ell/dbus-client.c
@@ -0,0 +1,595 @@
+/*
+ *
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2011-2014  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2017  Codecoup. 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 "ell.h"
+#include "private.h"
+
+struct l_dbus_client {
+	struct l_dbus *dbus;
+	unsigned int watch;
+	unsigned int added_watch;
+	unsigned int removed_watch;
+	char *service;
+	char *path;
+	uint32_t objects_call;
+
+	l_dbus_watch_func_t connect_cb;
+	void *connect_cb_data;
+	l_dbus_destroy_func_t connect_cb_data_destroy;
+
+	l_dbus_watch_func_t disconnect_cb;
+	void *disconnect_cb_data;
+	l_dbus_destroy_func_t disconnect_cb_data_destroy;
+
+	l_dbus_client_ready_func_t ready_cb;
+	void *ready_cb_data;
+	l_dbus_destroy_func_t ready_cb_data_destroy;
+
+	l_dbus_client_proxy_func_t proxy_added_cb;
+	l_dbus_client_proxy_func_t proxy_removed_cb;
+	l_dbus_client_property_function_t properties_changed_cb;
+	void *proxy_cb_data;
+	l_dbus_destroy_func_t proxy_cb_data_destroy;
+
+	struct l_queue *proxies;
+};
+
+struct proxy_property {
+	char *name;
+	struct l_dbus_message *msg;
+};
+
+struct l_dbus_proxy {
+	int refcount;
+	struct l_dbus_client *client;
+	char *interface;
+	char *path;
+	uint32_t properties_watch;
+	bool ready;
+
+	struct l_queue *properties;
+};
+
+LIB_EXPORT const char *l_dbus_proxy_get_path(struct l_dbus_proxy *proxy)
+{
+	if (unlikely(!proxy))
+		return NULL;
+
+	return proxy->path;
+}
+
+LIB_EXPORT const char *l_dbus_proxy_get_interface(struct l_dbus_proxy *proxy)
+{
+	if (unlikely(!proxy))
+		return NULL;
+
+	return proxy->interface;
+}
+
+static bool find_property_match(const void *a, const void *b)
+{
+	const struct proxy_property *prop = a;
+	const char *name = b;
+
+	return !strcmp(prop->name, name);
+}
+
+static struct proxy_property *find_property(struct l_dbus_proxy *proxy,
+							const char *name)
+{
+	return l_queue_find(proxy->properties, find_property_match, name);
+}
+
+static struct proxy_property *get_property(struct l_dbus_proxy *proxy,
+							const char *name)
+{
+	struct proxy_property *prop;
+
+	prop = find_property(proxy, name);
+	if (prop)
+		return prop;
+
+	prop = l_new(struct proxy_property, 1);
+	prop->name = l_strdup(name);
+
+	l_queue_push_tail(proxy->properties, prop);
+
+	return prop;
+}
+
+LIB_EXPORT bool l_dbus_proxy_get_property(struct l_dbus_proxy *proxy,
+						const char *name,
+						const char *signature, ...)
+{
+	struct proxy_property *prop;
+	va_list args;
+	bool res;
+
+	if (unlikely(!proxy))
+		return false;
+
+	prop = find_property(proxy, name);
+	if (!prop)
+		return false;
+
+	va_start(args, signature);
+	res = l_dbus_message_get_arguments_valist(prop->msg, signature, args);
+	va_end(args);
+
+	return res;
+}
+
+static void property_free(void *data)
+{
+	struct proxy_property *prop = data;
+
+	if (prop->msg)
+		l_dbus_message_unref(prop->msg);
+
+	l_free(prop->name);
+	l_free(prop);
+}
+
+static struct l_dbus_proxy *l_dbus_proxy_ref(struct l_dbus_proxy *proxy)
+{
+	if (unlikely(!proxy))
+		return NULL;
+
+	__sync_fetch_and_add(&proxy->refcount, 1);
+
+	return proxy;
+}
+
+static void l_dbus_proxy_unref(struct l_dbus_proxy *proxy)
+{
+	if (unlikely(!proxy))
+		return;
+
+	if (__sync_sub_and_fetch(&proxy->refcount, 1))
+		return;
+
+	if (proxy->properties_watch)
+		l_dbus_remove_signal_watch(proxy->client->dbus,
+						proxy->properties_watch);
+
+	l_queue_destroy(proxy->properties, property_free);
+	l_free(proxy->interface);
+	l_free(proxy->path);
+	l_free(proxy);
+}
+
+static void proxy_update_property(struct l_dbus_proxy *proxy,
+					const char *name,
+					struct l_dbus_message_iter *property)
+{
+	struct l_dbus_message_builder *builder;
+	struct proxy_property *prop = get_property(proxy, name);
+
+	l_dbus_message_unref(prop->msg);
+
+	if (!property) {
+		prop->msg = NULL;
+		goto done;
+	}
+
+	prop->msg = l_dbus_message_new_signal(proxy->client->dbus, proxy->path,
+							proxy->interface, name);
+	if (!prop->msg)
+		return;
+
+	builder = l_dbus_message_builder_new(prop->msg);
+	l_dbus_message_builder_append_from_iter(builder, property);
+	l_dbus_message_builder_finalize(builder);
+	l_dbus_message_builder_destroy(builder);
+
+done:
+	if (proxy->client->properties_changed_cb && proxy->ready)
+		proxy->client->properties_changed_cb(proxy, name, prop->msg,
+					proxy->client->proxy_cb_data_destroy);
+}
+
+static void proxy_invalidate_properties(struct l_dbus_proxy *proxy,
+					struct l_dbus_message_iter* props)
+{
+	const char *name;
+
+	while (l_dbus_message_iter_next_entry(props, &name))
+		proxy_update_property(proxy, name, NULL);
+}
+
+static void proxy_update_properties(struct l_dbus_proxy *proxy,
+					struct l_dbus_message_iter* props)
+{
+	struct l_dbus_message_iter variant;
+	const char *name;
+
+	while (l_dbus_message_iter_next_entry(props, &name, &variant))
+		proxy_update_property(proxy, name, &variant);
+}
+
+static void properties_changed_callback(struct l_dbus_message *message,
+								void *user_data)
+{
+	struct l_dbus_proxy *proxy = user_data;
+	const char *interface;
+	struct l_dbus_message_iter changed;
+	struct l_dbus_message_iter invalidated;
+
+	if (!l_dbus_message_get_arguments(message, "sa{sv}as", &interface,
+						&changed, &invalidated)) {
+		return;
+	}
+
+	proxy_update_properties(proxy, &changed);
+	proxy_invalidate_properties(proxy, &invalidated);
+}
+
+LIB_EXPORT struct l_dbus_proxy *l_dbus_proxy_new(struct l_dbus_client *client,
+					const char *path, const char *interface)
+{
+	struct l_dbus_proxy *proxy;
+
+	proxy = l_new(struct l_dbus_proxy, 1);
+
+	proxy->properties_watch = l_dbus_add_signal_watch(client->dbus,
+						client->service, path,
+						L_DBUS_INTERFACE_PROPERTIES,
+						"PropertiesChanged",
+						L_DBUS_MATCH_ARGUMENT(0),
+						interface, L_DBUS_MATCH_NONE,
+						properties_changed_callback,
+						proxy);
+	if (!proxy->properties_watch) {
+		l_free(proxy);
+		return NULL;
+	}
+
+	proxy->refcount = 1;
+	proxy->client = client;
+	proxy->interface = l_strdup(interface);
+	proxy->path = l_strdup(path);
+	proxy->properties = l_queue_new();
+
+	l_queue_push_tail(client->proxies, proxy);
+
+	return proxy;
+}
+
+static bool is_ignorable(const char *interface)
+{
+	static const struct {
+		const char *interface;
+	} interfaces_to_ignore[] = {
+		{ L_DBUS_INTERFACE_OBJECT_MANAGER },
+		{ L_DBUS_INTERFACE_INTROSPECTABLE },
+		{ L_DBUS_INTERFACE_PROPERTIES },
+	};
+	size_t i;
+
+	for (i = 0; i < L_ARRAY_SIZE(interfaces_to_ignore); i++)
+		if (!strcmp(interfaces_to_ignore[i].interface, interface))
+			return true;
+
+	return false;
+}
+
+struct find_proxy_data
+{
+	const char *path;
+	const char *interface;
+};
+
+static bool find_proxy_match(const void *a, const void *b)
+{
+	const struct l_dbus_proxy *proxy = a;
+	const struct find_proxy_data *data = b;
+
+	return !strcmp(proxy->interface, data->interface) &&
+					!strcmp(proxy->path, data->path);
+}
+
+static struct l_dbus_proxy *find_proxy(struct l_dbus_client *client,
+					const char *path, const char *interface)
+{
+	struct find_proxy_data data;
+
+	data.path = path;
+	data.interface = interface;
+
+	return l_queue_find(client->proxies, find_proxy_match, &data);
+}
+
+static void parse_interface(struct l_dbus_client *client, const char *path,
+					const char *interface,
+					struct l_dbus_message_iter *properties)
+{
+	struct l_dbus_proxy *proxy;
+	bool proxy_added = false;
+
+	if (is_ignorable(interface))
+		return;
+
+	proxy = find_proxy(client, path, interface);
+	if (!proxy) {
+		proxy = l_dbus_proxy_new(client, path, interface);
+		proxy_added = true;
+	}
+
+	if (!proxy)
+		return;
+
+	proxy_update_properties(proxy, properties);
+
+	if (proxy_added && client->proxy_added_cb) {
+		proxy->ready = true;
+		client->proxy_added_cb(proxy, client->proxy_cb_data);
+	}
+}
+
+static void parse_object(struct l_dbus_client *client, const char *path,
+					struct l_dbus_message_iter *object)
+{
+	const char *interface;
+	struct l_dbus_message_iter properties;
+
+	if (!path)
+		return;
+
+	while (l_dbus_message_iter_next_entry(object, &interface, &properties))
+		parse_interface(client, path, interface, &properties);
+}
+
+static void interfaces_added_callback(struct l_dbus_message *message,
+								void *user_data)
+{
+	struct l_dbus_client *client = user_data;
+	struct l_dbus_message_iter object;
+	const char *path;
+
+	l_dbus_message_get_arguments(message, "oa{sa{sv}}", &path, &object);
+
+	parse_object(client, path, &object);
+}
+
+static void interfaces_removed_callback(struct l_dbus_message *message,
+								void *user_data)
+{
+	struct l_dbus_client *client = user_data;
+	struct l_dbus_message_iter interfaces;
+	const char *interface;
+	const char *path;
+
+	l_dbus_message_get_arguments(message, "oas", &path, &interfaces);
+
+	while (l_dbus_message_iter_next_entry(&interfaces, &interface)) {
+		struct l_dbus_proxy *proxy;
+
+		proxy = find_proxy(client, path, interface);
+		if (!proxy)
+			continue;
+
+		l_queue_remove(proxy->client->proxies, proxy);
+
+		if (client->proxy_removed_cb)
+			client->proxy_removed_cb(proxy, client->proxy_cb_data);
+
+		l_dbus_proxy_unref(proxy);
+	}
+}
+
+static void get_managed_objects_reply(struct l_dbus_message *message,
+								void *user_data)
+{
+	struct l_dbus_client *client = user_data;
+	struct l_dbus_message_iter objects;
+	struct l_dbus_message_iter object;
+	const char *path;
+
+	client->objects_call = 0;
+
+	if (l_dbus_message_is_error(message)) {
+		return;
+	}
+
+	l_dbus_message_get_arguments(message, "a{oa{sa{sv}}}", &objects);
+
+	while (l_dbus_message_iter_next_entry(&objects, &path, &object))
+		parse_object(client, path, &object);
+
+	client->added_watch = l_dbus_add_signal_watch(client->dbus,
+						client->service, "/",
+						L_DBUS_INTERFACE_OBJECT_MANAGER,
+						"InterfacesAdded",
+						L_DBUS_MATCH_NONE,
+						interfaces_added_callback,
+						client);
+
+	client->removed_watch = l_dbus_add_signal_watch(client->dbus,
+						client->service, "/",
+						L_DBUS_INTERFACE_OBJECT_MANAGER,
+						"InterfacesRemoved",
+						L_DBUS_MATCH_NONE,
+						interfaces_removed_callback,
+						client);
+
+	if (client->ready_cb)
+		client->ready_cb(client, client->ready_cb_data);
+}
+
+static void service_appeared_callback(struct l_dbus *dbus, void *user_data)
+{
+	struct l_dbus_client *client = user_data;
+
+	/* TODO should we allow to set different root? */
+	client->objects_call = l_dbus_method_call(dbus, client->service, "/",
+						L_DBUS_INTERFACE_OBJECT_MANAGER,
+						"GetManagedObjects", NULL,
+						get_managed_objects_reply,
+						client, NULL);
+
+	if (client->connect_cb)
+		client->connect_cb(client->dbus, client->connect_cb_data);
+}
+
+static void service_disappeared_callback(struct l_dbus *dbus, void *user_data)
+{
+	struct l_dbus_client *client = user_data;
+
+	if (client->disconnect_cb)
+		client->disconnect_cb(client->dbus, client->disconnect_cb_data);
+
+
+	l_queue_clear(client->proxies,
+				(l_queue_destroy_func_t)l_dbus_proxy_unref);
+}
+
+LIB_EXPORT struct l_dbus_client *l_dbus_client_new(struct l_dbus *dbus,
+					const char *service, const char *path)
+{
+	struct l_dbus_client *client;
+
+	client = l_new(struct l_dbus_client, 1);
+
+	client->dbus = dbus;
+
+	client->watch = l_dbus_add_service_watch(dbus, service,
+						service_appeared_callback,
+						service_disappeared_callback,
+						client, NULL);
+
+	if (!client->watch) {
+		l_free(client);
+		return NULL;
+	}
+
+	client->service = l_strdup(service);
+	client->path = l_strdup(path);
+	client->proxies = l_queue_new();
+
+	return client;
+}
+
+LIB_EXPORT void l_dbus_client_destroy(struct l_dbus_client *client)
+{
+	if (unlikely(!client))
+		return;
+
+	if (client->watch)
+		l_dbus_remove_signal_watch(client->dbus, client->watch);
+
+	if (client->added_watch)
+		l_dbus_remove_signal_watch(client->dbus, client->added_watch);
+
+	if (client->removed_watch)
+		l_dbus_remove_signal_watch(client->dbus, client->removed_watch);
+
+	if (client->connect_cb_data_destroy)
+		client->connect_cb_data_destroy(client->connect_cb_data);
+
+	if (client->disconnect_cb_data_destroy)
+		client->disconnect_cb_data_destroy(client->disconnect_cb_data);
+
+	if (client->ready_cb_data_destroy)
+		client->ready_cb_data_destroy(client->ready_cb_data);
+
+	if (client->proxy_cb_data_destroy)
+		client->proxy_cb_data_destroy(client->proxy_cb_data);
+
+	if (client->objects_call)
+		l_dbus_cancel(client->dbus, client->objects_call)
+;
+	l_queue_destroy(client->proxies,
+				(l_queue_destroy_func_t)l_dbus_proxy_unref);
+
+	l_free(client->service);
+	l_free(client->path);
+	l_free(client);
+}
+
+LIB_EXPORT bool l_dbus_client_set_connect_handler(struct l_dbus_client *client,
+						l_dbus_watch_func_t function,
+						void *user_data,
+						l_dbus_destroy_func_t destroy)
+{
+	if (unlikely(!client))
+		return false;
+
+	client->connect_cb = function;
+	client->connect_cb_data = user_data;
+	client->connect_cb_data_destroy = destroy;
+
+	return true;
+}
+
+LIB_EXPORT bool l_dbus_client_set_disconnect_handler(struct l_dbus_client *client,
+						l_dbus_watch_func_t function,
+						void *user_data,
+						l_dbus_destroy_func_t destroy)
+{
+	if (unlikely(!client))
+		return false;
+
+	client->disconnect_cb = function;
+	client->disconnect_cb_data = user_data;
+	client->disconnect_cb_data_destroy = destroy;
+
+	return true;
+}
+
+LIB_EXPORT bool l_dbus_client_set_ready_handler(struct l_dbus_client *client,
+					l_dbus_client_ready_func_t function,
+					void *user_data,
+					l_dbus_destroy_func_t destroy)
+{
+	if (unlikely(!client))
+		return false;
+
+	client->ready_cb = function;
+	client->ready_cb_data = user_data;
+	client->ready_cb_data_destroy = destroy;
+
+	return true;
+}
+
+LIB_EXPORT bool l_dbus_client_set_proxy_handlers(struct l_dbus_client *client,
+					l_dbus_client_proxy_func_t proxy_added,
+					l_dbus_client_proxy_func_t proxy_removed,
+					l_dbus_client_property_function_t property_changed,
+					void *user_data,
+					l_dbus_destroy_func_t destroy)
+{
+	if (unlikely(!client))
+		return false;
+
+	client->proxy_added_cb = proxy_added;
+	client->proxy_removed_cb = proxy_removed;
+	client->properties_changed_cb = property_changed;
+	client->proxy_cb_data = user_data;
+	client->proxy_cb_data_destroy = destroy;
+
+	return true;
+}
diff --git a/ell/dbus-client.h b/ell/dbus-client.h
new file mode 100644
index 0000000..b32f4a3
--- /dev/null
+++ b/ell/dbus-client.h
@@ -0,0 +1,87 @@
+/*
+ *
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2011-2014  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2017  Codecoup. 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
+ *
+ */
+
+#ifndef __ELL_DBUS_CLIENT_H
+#define __ELL_DBUS_CLIENT_H
+
+#include <stdbool.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct l_dbus;
+struct l_dbus_message;
+struct l_dbus_client;
+struct l_dbus_proxy;
+
+typedef void (*l_dbus_client_ready_func_t)(struct l_dbus_client *client,
+							void *user_data);
+typedef void (*l_dbus_client_proxy_func_t) (struct l_dbus_proxy *proxy,
+							void *user_data);
+typedef void (*l_dbus_client_property_function_t) (struct l_dbus_proxy *proxy,
+						const char *name,
+						struct l_dbus_message *msg,
+						void *user_data);
+
+struct l_dbus_client *l_dbus_client_new(struct l_dbus *dbus,
+					const char *service, const char *path);
+void l_dbus_client_destroy(struct l_dbus_client *client);
+
+bool l_dbus_client_set_connect_handler(struct l_dbus_client *client,
+						l_dbus_watch_func_t function,
+						void *user_data,
+						l_dbus_destroy_func_t destroy);
+
+bool l_dbus_client_set_disconnect_handler(struct l_dbus_client *client,
+						l_dbus_watch_func_t function,
+						void *user_data,
+						l_dbus_destroy_func_t destroy);
+
+bool l_dbus_client_set_ready_handler(struct l_dbus_client *client,
+					l_dbus_client_ready_func_t function,
+					void *user_data,
+					l_dbus_destroy_func_t destroy);
+
+bool l_dbus_client_set_proxy_handlers(struct l_dbus_client *client,
+					l_dbus_client_proxy_func_t proxy_added,
+					l_dbus_client_proxy_func_t proxy_removed,
+					l_dbus_client_property_function_t property_changed,
+					void *user_data,
+					l_dbus_destroy_func_t destroy);
+
+struct l_dbus_proxy *l_dbus_proxy_new(struct l_dbus_client *client,
+							const char *path,
+							const char *interface);
+
+const char *l_dbus_proxy_get_path(struct l_dbus_proxy *proxy);
+
+const char *l_dbus_proxy_get_interface(struct l_dbus_proxy *proxy);
+
+bool l_dbus_proxy_get_property(struct l_dbus_proxy *proxy, const char *name,
+						const char *signature, ...);
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __ELL_DBUS_CLIENT_H */
diff --git a/ell/ell.h b/ell/ell.h
index 2efd1c4..4083ee5 100644
--- a/ell/ell.h
+++ b/ell/ell.h
@@ -51,3 +51,4 @@
 #include <ell/genl.h>
 #include <ell/dbus.h>
 #include <ell/dbus-service.h>
+#include <ell/dbus-client.h>
-- 
2.14.3

Re: [RFC 4/6] dbus: Add support for proxy interface

[Denis Kenzior]

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

Hi Szymon,

On 11/29/2017 09:16 AM, Szymon Janc wrote:
> ---
>   Makefile.am       |   2 +
>   ell/dbus-client.c | 595 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ell/dbus-client.h |  87 ++++++++
>   ell/ell.h         |   1 +
>   4 files changed, 685 insertions(+)
>   create mode 100644 ell/dbus-client.c
>   create mode 100644 ell/dbus-client.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index a95bac4..19b957e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -32,6 +32,7 @@ pkginclude_HEADERS = ell/ell.h \
>   			ell/genl.h \
>   			ell/dbus.h \
>   			ell/dbus-service.h \
> +			ell/dbus-client.h \
>   			ell/hwdb.h \
>   			ell/cipher.h \
>   			ell/random.h \
> @@ -74,6 +75,7 @@ ell_libell_la_SOURCES = $(linux_headers) \
>   			ell/dbus-message.c \
>   			ell/dbus-util.c \
>   			ell/dbus-service.c \
> +			ell/dbus-client.c \
>   			ell/dbus-name-cache.c \
>   			ell/dbus-filter.c \
>   			ell/gvariant-private.h \
> diff --git a/ell/dbus-client.c b/ell/dbus-client.c
> new file mode 100644
> index 0000000..e6b6ff3
> --- /dev/null
> +++ b/ell/dbus-client.c
> @@ -0,0 +1,595 @@
> +/*
> + *
> + *  Embedded Linux library
> + *
> + *  Copyright (C) 2011-2014  Intel Corporation. All rights reserved.
> + *  Copyright (C) 2017  Codecoup. 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 "ell.h"
> +#include "private.h"
> +
> +struct l_dbus_client {
> +	struct l_dbus *dbus;
> +	unsigned int watch;
> +	unsigned int added_watch;
> +	unsigned int removed_watch;
> +	char *service;
> +	char *path;
> +	uint32_t objects_call;
> +
> +	l_dbus_watch_func_t connect_cb;
> +	void *connect_cb_data;
> +	l_dbus_destroy_func_t connect_cb_data_destroy;
> +
> +	l_dbus_watch_func_t disconnect_cb;
> +	void *disconnect_cb_data;
> +	l_dbus_destroy_func_t disconnect_cb_data_destroy;
> +
> +	l_dbus_client_ready_func_t ready_cb;
> +	void *ready_cb_data;
> +	l_dbus_destroy_func_t ready_cb_data_destroy; > +
> +	l_dbus_client_proxy_func_t proxy_added_cb;
> +	l_dbus_client_proxy_func_t proxy_removed_cb;
> +	l_dbus_client_property_function_t properties_changed_cb;
> +	void *proxy_cb_data;
> +	l_dbus_destroy_func_t proxy_cb_data_destroy;
> +
> +	struct l_queue *proxies;
> +};
> +
> +struct proxy_property {
> +	char *name;
> +	struct l_dbus_message *msg;
> +};
> +
> +struct l_dbus_proxy {
> +	int refcount;
> +	struct l_dbus_client *client;
> +	char *interface;
> +	char *path;
> +	uint32_t properties_watch;
> +	bool ready;
> +
> +	struct l_queue *properties;
> +};
> +
> +LIB_EXPORT const char *l_dbus_proxy_get_path(struct l_dbus_proxy *proxy)
> +{
> +	if (unlikely(!proxy))
> +		return NULL;
> +
> +	return proxy->path;
> +}
> +
> +LIB_EXPORT const char *l_dbus_proxy_get_interface(struct l_dbus_proxy *proxy)
> +{
> +	if (unlikely(!proxy))
> +		return NULL;
> +
> +	return proxy->interface;
> +}
> +
> +static bool find_property_match(const void *a, const void *b)

property_match_by_name?

> +{
> +	const struct proxy_property *prop = a;
> +	const char *name = b;
> +
> +	return !strcmp(prop->name, name);
> +}
> +
> +static struct proxy_property *find_property(struct l_dbus_proxy *proxy,
> +							const char *name)
> +{
> +	return l_queue_find(proxy->properties, find_property_match, name);
> +}
> +
> +static struct proxy_property *get_property(struct l_dbus_proxy *proxy,
> +							const char *name)
> +{
> +	struct proxy_property *prop;
> +
> +	prop = find_property(proxy, name);
> +	if (prop)
> +		return prop;
> +
> +	prop = l_new(struct proxy_property, 1);
> +	prop->name = l_strdup(name);
> +
> +	l_queue_push_tail(proxy->properties, prop);
> +
> +	return prop;
> +}
> +
> +LIB_EXPORT bool l_dbus_proxy_get_property(struct l_dbus_proxy *proxy,
> +						const char *name,
> +						const char *signature, ...)
> +{
> +	struct proxy_property *prop;
> +	va_list args;
> +	bool res;
> +
> +	if (unlikely(!proxy))
> +		return false;
> +
> +	prop = find_property(proxy, name);
> +	if (!prop)
> +		return false;
> +
> +	va_start(args, signature);
> +	res = l_dbus_message_get_arguments_valist(prop->msg, signature, args);
> +	va_end(args);
> +
> +	return res;
> +}
> +
> +static void property_free(void *data)
> +{
> +	struct proxy_property *prop = data;
> +
> +	if (prop->msg)
> +		l_dbus_message_unref(prop->msg);
> +
> +	l_free(prop->name);
> +	l_free(prop);
> +}
> +
> +static struct l_dbus_proxy *l_dbus_proxy_ref(struct l_dbus_proxy *proxy)
> +{
> +	if (unlikely(!proxy))
> +		return NULL;
> +
> +	__sync_fetch_and_add(&proxy->refcount, 1);
> +
> +	return proxy;
> +}
> +
> +static void l_dbus_proxy_unref(struct l_dbus_proxy *proxy)
> +{
> +	if (unlikely(!proxy))
> +		return;
> +
> +	if (__sync_sub_and_fetch(&proxy->refcount, 1))
> +		return;
> +
> +	if (proxy->properties_watch)
> +		l_dbus_remove_signal_watch(proxy->client->dbus,
> +						proxy->properties_watch);
> +
> +	l_queue_destroy(proxy->properties, property_free);
> +	l_free(proxy->interface);
> +	l_free(proxy->path);
> +	l_free(proxy);
> +}
> +
> +static void proxy_update_property(struct l_dbus_proxy *proxy,
> +					const char *name,
> +					struct l_dbus_message_iter *property)
> +{
> +	struct l_dbus_message_builder *builder;
> +	struct proxy_property *prop = get_property(proxy, name);
> +
> +	l_dbus_message_unref(prop->msg);
> +
> +	if (!property) {
> +		prop->msg = NULL;
> +		goto done;
> +	}
> +
> +	prop->msg = l_dbus_message_new_signal(proxy->client->dbus, proxy->path,
> +							proxy->interface, name);

So for every property we build a dummy message and put the contents of 
the property into the message, right?  E.g. If Powered = true was 
signaled, we build a signal with member="Powered" and a single byte as 
contents.

I'm okay with this being the first naive solution, but this seems quite 
wasteful.  Can we store the serialized property data without the 
enclosing message?  E.g. by using GVariant serializer and storing the 
raw data?

> +	if (!prop->msg)
> +		return;
> +
> +	builder = l_dbus_message_builder_new(prop->msg);
> +	l_dbus_message_builder_append_from_iter(builder, property);
> +	l_dbus_message_builder_finalize(builder);
> +	l_dbus_message_builder_destroy(builder);
> +
> +done:
> +	if (proxy->client->properties_changed_cb && proxy->ready)

How does proxy->ready get set for manually created proxies?  Or is a 
proxy supposed to be always obtained by the proxy_added callback?  If 
so, why export the proxy_new constructor?

> +		proxy->client->properties_changed_cb(proxy, name, prop->msg,
> +					proxy->client->proxy_cb_data_destroy);
> +}
> +
> +static void proxy_invalidate_properties(struct l_dbus_proxy *proxy,
> +					struct l_dbus_message_iter* props)
> +{
> +	const char *name;
> +
> +	while (l_dbus_message_iter_next_entry(props, &name))
> +		proxy_update_property(proxy, name, NULL);
> +}
> +
> +static void proxy_update_properties(struct l_dbus_proxy *proxy,
> +					struct l_dbus_message_iter* props)
> +{
> +	struct l_dbus_message_iter variant;
> +	const char *name;
> +
> +	while (l_dbus_message_iter_next_entry(props, &name, &variant))
> +		proxy_update_property(proxy, name, &variant);
> +}
> +
> +static void properties_changed_callback(struct l_dbus_message *message,
> +								void *user_data)
> +{
> +	struct l_dbus_proxy *proxy = user_data;
> +	const char *interface;
> +	struct l_dbus_message_iter changed;
> +	struct l_dbus_message_iter invalidated;
> +
> +	if (!l_dbus_message_get_arguments(message, "sa{sv}as", &interface,
> +						&changed, &invalidated)) {
> +		return;
> +	}

No need for {}

> +
> +	proxy_update_properties(proxy, &changed);
> +	proxy_invalidate_properties(proxy, &invalidated);
> +}
> +
> +LIB_EXPORT struct l_dbus_proxy *l_dbus_proxy_new(struct l_dbus_client *client,
> +					const char *path, const char *interface)
> +{
> +	struct l_dbus_proxy *proxy;
> +
> +	proxy = l_new(struct l_dbus_proxy, 1);
> +
> +	proxy->properties_watch = l_dbus_add_signal_watch(client->dbus,
> +						client->service, path,
> +						L_DBUS_INTERFACE_PROPERTIES,
> +						"PropertiesChanged",
> +						L_DBUS_MATCH_ARGUMENT(0),
> +						interface, L_DBUS_MATCH_NONE,
> +						properties_changed_callback,
> +						proxy);
> +	if (!proxy->properties_watch) {
> +		l_free(proxy);
> +		return NULL;
> +	}
> +
> +	proxy->refcount = 1;
> +	proxy->client = client;
> +	proxy->interface = l_strdup(interface);
> +	proxy->path = l_strdup(path);
> +	proxy->properties = l_queue_new();
> +
> +	l_queue_push_tail(client->proxies, proxy);
> +
> +	return proxy;
> +}
> +
> +static bool is_ignorable(const char *interface)
> +{
> +	static const struct {
> +		const char *interface;
> +	} interfaces_to_ignore[] = {
> +		{ L_DBUS_INTERFACE_OBJECT_MANAGER },
> +		{ L_DBUS_INTERFACE_INTROSPECTABLE },
> +		{ L_DBUS_INTERFACE_PROPERTIES },
> +	};
> +	size_t i;
> +
> +	for (i = 0; i < L_ARRAY_SIZE(interfaces_to_ignore); i++)
> +		if (!strcmp(interfaces_to_ignore[i].interface, interface))
> +			return true;
> +
> +	return false;
> +}
> +
> +struct find_proxy_data
> +{
> +	const char *path;
> +	const char *interface;
> +};
> +
> +static bool find_proxy_match(const void *a, const void *b)
> +{
> +	const struct l_dbus_proxy *proxy = a;
> +	const struct find_proxy_data *data = b;
> +
> +	return !strcmp(proxy->interface, data->interface) &&
> +					!strcmp(proxy->path, data->path);
> +}
> +
> +static struct l_dbus_proxy *find_proxy(struct l_dbus_client *client,
> +					const char *path, const char *interface)
> +{
> +	struct find_proxy_data data;
> +
> +	data.path = path;
> +	data.interface = interface;
> +
> +	return l_queue_find(client->proxies, find_proxy_match, &data);

It might be more compact to rewrite this using l_queue_get_entries.  No 
need to define struct find_proxy_data, etc.

> +}
> +
> +static void parse_interface(struct l_dbus_client *client, const char *path,
> +					const char *interface,
> +					struct l_dbus_message_iter *properties)
> +{
> +	struct l_dbus_proxy *proxy;
> +	bool proxy_added = false;
> +
> +	if (is_ignorable(interface))
> +		return;
> +
> +	proxy = find_proxy(client, path, interface);
> +	if (!proxy) {
> +		proxy = l_dbus_proxy_new(client, path, interface);
> +		proxy_added = true;
> +	}
> +
> +	if (!proxy)
> +		return;
> +
> +	proxy_update_properties(proxy, properties);
> +
> +	if (proxy_added && client->proxy_added_cb) {
> +		proxy->ready = true;

Why does setting ready depend on proxy_added_cb being valid?

> +		client->proxy_added_cb(proxy, client->proxy_cb_data);
> +	}
> +}
> +
> +static void parse_object(struct l_dbus_client *client, const char *path,
> +					struct l_dbus_message_iter *object)
> +{
> +	const char *interface;
> +	struct l_dbus_message_iter properties;
> +
> +	if (!path)
> +		return;
> +
> +	while (l_dbus_message_iter_next_entry(object, &interface, &properties))
> +		parse_interface(client, path, interface, &properties);
> +}
> +
> +static void interfaces_added_callback(struct l_dbus_message *message,
> +								void *user_data)
> +{
> +	struct l_dbus_client *client = user_data;
> +	struct l_dbus_message_iter object;
> +	const char *path;
> +
> +	l_dbus_message_get_arguments(message, "oa{sa{sv}}", &path, &object);

Might want an error check here

> +
> +	parse_object(client, path, &object);
> +}
> +
> +static void interfaces_removed_callback(struct l_dbus_message *message,
> +								void *user_data)
> +{
> +	struct l_dbus_client *client = user_data;
> +	struct l_dbus_message_iter interfaces;
> +	const char *interface;
> +	const char *path;
> +
> +	l_dbus_message_get_arguments(message, "oas", &path, &interfaces);
> +
> +	while (l_dbus_message_iter_next_entry(&interfaces, &interface)) {
> +		struct l_dbus_proxy *proxy;
> +
> +		proxy = find_proxy(client, path, interface);
> +		if (!proxy)
> +			continue;
> +
> +		l_queue_remove(proxy->client->proxies, proxy);
> +
> +		if (client->proxy_removed_cb)
> +			client->proxy_removed_cb(proxy, client->proxy_cb_data);
> +
> +		l_dbus_proxy_unref(proxy);
> +	}
> +}
> +
> +static void get_managed_objects_reply(struct l_dbus_message *message,
> +								void *user_data)
> +{
> +	struct l_dbus_client *client = user_data;
> +	struct l_dbus_message_iter objects;
> +	struct l_dbus_message_iter object;
> +	const char *path;
> +
> +	client->objects_call = 0;
> +
> +	if (l_dbus_message_is_error(message)) {
> +		return;
> +	}

No need for {}

> +
> +	l_dbus_message_get_arguments(message, "a{oa{sa{sv}}}", &objects);
> +
> +	while (l_dbus_message_iter_next_entry(&objects, &path, &object))
> +		parse_object(client, path, &object);
> +
> +	client->added_watch = l_dbus_add_signal_watch(client->dbus,
> +						client->service, "/",
> +						L_DBUS_INTERFACE_OBJECT_MANAGER,
> +						"InterfacesAdded",
> +						L_DBUS_MATCH_NONE,
> +						interfaces_added_callback,
> +						client);
> +
> +	client->removed_watch = l_dbus_add_signal_watch(client->dbus,
> +						client->service, "/",
> +						L_DBUS_INTERFACE_OBJECT_MANAGER,
> +						"InterfacesRemoved",
> +						L_DBUS_MATCH_NONE,
> +						interfaces_removed_callback,
> +						client);
> +
> +	if (client->ready_cb)
> +		client->ready_cb(client, client->ready_cb_data);
> +}
> +
> +static void service_appeared_callback(struct l_dbus *dbus, void *user_data)
> +{
> +	struct l_dbus_client *client = user_data;
> +
> +	/* TODO should we allow to set different root? */
> +	client->objects_call = l_dbus_method_call(dbus, client->service, "/",

Should "/" be client->path instead?  Note that ell only exports 
ObjectManager on the root path.  It doesn't export it for any child 
paths.  However, BlueZ might act differently.

> +						L_DBUS_INTERFACE_OBJECT_MANAGER,
> +						"GetManagedObjects", NULL,
> +						get_managed_objects_reply,
> +						client, NULL);
> +
> +	if (client->connect_cb)
> +		client->connect_cb(client->dbus, client->connect_cb_data);
> +}
> +
> +static void service_disappeared_callback(struct l_dbus *dbus, void *user_data)
> +{
> +	struct l_dbus_client *client = user_data;
> +
> +	if (client->disconnect_cb)
> +		client->disconnect_cb(client->dbus, client->disconnect_cb_data);
> +
> +

No double empty lines please

> +	l_queue_clear(client->proxies,
> +				(l_queue_destroy_func_t)l_dbus_proxy_unref);
> +}
> +
> +LIB_EXPORT struct l_dbus_client *l_dbus_client_new(struct l_dbus *dbus,
> +					const char *service, const char *path)
> +{
> +	struct l_dbus_client *client;
> +
> +	client = l_new(struct l_dbus_client, 1);
> +
> +	client->dbus = dbus;
> +
> +	client->watch = l_dbus_add_service_watch(dbus, service,
> +						service_appeared_callback,
> +						service_disappeared_callback,
> +						client, NULL);
> +
> +	if (!client->watch) {
> +		l_free(client);
> +		return NULL;
> +	}
> +
> +	client->service = l_strdup(service);
> +	client->path = l_strdup(path);
> +	client->proxies = l_queue_new();
> +
> +	return client;
> +}
> +
> +LIB_EXPORT void l_dbus_client_destroy(struct l_dbus_client *client)
> +{
> +	if (unlikely(!client))
> +		return;
> +
> +	if (client->watch)
> +		l_dbus_remove_signal_watch(client->dbus, client->watch);
> +
> +	if (client->added_watch)
> +		l_dbus_remove_signal_watch(client->dbus, client->added_watch);
> +
> +	if (client->removed_watch)
> +		l_dbus_remove_signal_watch(client->dbus, client->removed_watch);
> +
> +	if (client->connect_cb_data_destroy)
> +		client->connect_cb_data_destroy(client->connect_cb_data);
> +
> +	if (client->disconnect_cb_data_destroy)
> +		client->disconnect_cb_data_destroy(client->disconnect_cb_data);
> +
> +	if (client->ready_cb_data_destroy)
> +		client->ready_cb_data_destroy(client->ready_cb_data);
> +
> +	if (client->proxy_cb_data_destroy)
> +		client->proxy_cb_data_destroy(client->proxy_cb_data);
> +
> +	if (client->objects_call)
> +		l_dbus_cancel(client->dbus, client->objects_call)
> +;
> +	l_queue_destroy(client->proxies,
> +				(l_queue_destroy_func_t)l_dbus_proxy_unref);
> +
> +	l_free(client->service);
> +	l_free(client->path);
> +	l_free(client);
> +}
> +
> +LIB_EXPORT bool l_dbus_client_set_connect_handler(struct l_dbus_client *client,
> +						l_dbus_watch_func_t function,
> +						void *user_data,
> +						l_dbus_destroy_func_t destroy)
> +{
> +	if (unlikely(!client))
> +		return false;
> +
> +	client->connect_cb = function;
> +	client->connect_cb_data = user_data;
> +	client->connect_cb_data_destroy = destroy;
> +
> +	return true;
> +}
> +
> +LIB_EXPORT bool l_dbus_client_set_disconnect_handler(struct l_dbus_client *client,
> +						l_dbus_watch_func_t function,
> +						void *user_data,
> +						l_dbus_destroy_func_t destroy)
> +{
> +	if (unlikely(!client))
> +		return false;
> +
> +	client->disconnect_cb = function;
> +	client->disconnect_cb_data = user_data;
> +	client->disconnect_cb_data_destroy = destroy;
> +
> +	return true;
> +}
> +
> +LIB_EXPORT bool l_dbus_client_set_ready_handler(struct l_dbus_client *client,
> +					l_dbus_client_ready_func_t function,
> +					void *user_data,
> +					l_dbus_destroy_func_t destroy)
> +{
> +	if (unlikely(!client))
> +		return false;
> +
> +	client->ready_cb = function;
> +	client->ready_cb_data = user_data;
> +	client->ready_cb_data_destroy = destroy;
> +
> +	return true;
> +}
> +
> +LIB_EXPORT bool l_dbus_client_set_proxy_handlers(struct l_dbus_client *client,
> +					l_dbus_client_proxy_func_t proxy_added,
> +					l_dbus_client_proxy_func_t proxy_removed,
> +					l_dbus_client_property_function_t property_changed,
> +					void *user_data,
> +					l_dbus_destroy_func_t destroy)
> +{
> +	if (unlikely(!client))
> +		return false;
> +
> +	client->proxy_added_cb = proxy_added;
> +	client->proxy_removed_cb = proxy_removed;
> +	client->properties_changed_cb = property_changed;
> +	client->proxy_cb_data = user_data;
> +	client->proxy_cb_data_destroy = destroy;
> +
> +	return true;
> +}
> diff --git a/ell/dbus-client.h b/ell/dbus-client.h
> new file mode 100644
> index 0000000..b32f4a3
> --- /dev/null
> +++ b/ell/dbus-client.h
> @@ -0,0 +1,87 @@
> +/*
> + *
> + *  Embedded Linux library
> + *
> + *  Copyright (C) 2011-2014  Intel Corporation. All rights reserved.
> + *  Copyright (C) 2017  Codecoup. 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
> + *
> + */
> +
> +#ifndef __ELL_DBUS_CLIENT_H
> +#define __ELL_DBUS_CLIENT_H
> +
> +#include <stdbool.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +struct l_dbus;
> +struct l_dbus_message;
> +struct l_dbus_client;
> +struct l_dbus_proxy;
> +
> +typedef void (*l_dbus_client_ready_func_t)(struct l_dbus_client *client,
> +							void *user_data);
> +typedef void (*l_dbus_client_proxy_func_t) (struct l_dbus_proxy *proxy,
> +							void *user_data);
> +typedef void (*l_dbus_client_property_function_t) (struct l_dbus_proxy *proxy,
> +						const char *name,
> +						struct l_dbus_message *msg,
> +						void *user_data);
> +
> +struct l_dbus_client *l_dbus_client_new(struct l_dbus *dbus,
> +					const char *service, const char *path);
> +void l_dbus_client_destroy(struct l_dbus_client *client);
> +
> +bool l_dbus_client_set_connect_handler(struct l_dbus_client *client,
> +						l_dbus_watch_func_t function,
> +						void *user_data,
> +						l_dbus_destroy_func_t destroy);
> +
> +bool l_dbus_client_set_disconnect_handler(struct l_dbus_client *client,
> +						l_dbus_watch_func_t function,
> +						void *user_data,
> +						l_dbus_destroy_func_t destroy);
> +
> +bool l_dbus_client_set_ready_handler(struct l_dbus_client *client,
> +					l_dbus_client_ready_func_t function,
> +					void *user_data,
> +					l_dbus_destroy_func_t destroy);
> +
> +bool l_dbus_client_set_proxy_handlers(struct l_dbus_client *client,
> +					l_dbus_client_proxy_func_t proxy_added,
> +					l_dbus_client_proxy_func_t proxy_removed,
> +					l_dbus_client_property_function_t property_changed,
> +					void *user_data,
> +					l_dbus_destroy_func_t destroy);
> +
> +struct l_dbus_proxy *l_dbus_proxy_new(struct l_dbus_client *client,
> +							const char *path,
> +							const char *interface);
> +
> +const char *l_dbus_proxy_get_path(struct l_dbus_proxy *proxy);
> +
> +const char *l_dbus_proxy_get_interface(struct l_dbus_proxy *proxy);
> +
> +bool l_dbus_proxy_get_property(struct l_dbus_proxy *proxy, const char *name,
> +						const char *signature, ...);
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* __ELL_DBUS_CLIENT_H */
> diff --git a/ell/ell.h b/ell/ell.h
> index 2efd1c4..4083ee5 100644
> --- a/ell/ell.h
> +++ b/ell/ell.h
> @@ -51,3 +51,4 @@
>   #include <ell/genl.h>
>   #include <ell/dbus.h>
>   #include <ell/dbus-service.h>
> +#include <ell/dbus-client.h>
> 

Regards,
-Denis

Re: [RFC 4/6] dbus: Add support for proxy interface

[Szymon Janc]

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

Hi Denis,

On Wednesday, 29 November 2017 17:59:14 CET Denis Kenzior wrote:
> Hi Szymon,
> 
> On 11/29/2017 09:16 AM, Szymon Janc wrote:
> > ---
> > 
> >   Makefile.am       |   2 +
> >   ell/dbus-client.c | 595
> >   ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   ell/dbus-client.h |  87 ++++++++
> >   ell/ell.h         |   1 +
> >   4 files changed, 685 insertions(+)
> >   create mode 100644 ell/dbus-client.c
> >   create mode 100644 ell/dbus-client.h
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index a95bac4..19b957e 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -32,6 +32,7 @@ pkginclude_HEADERS = ell/ell.h \
> > 
> >   			ell/genl.h \
> >   			ell/dbus.h \
> >   			ell/dbus-service.h \
> > 
> > +			ell/dbus-client.h \
> > 
> >   			ell/hwdb.h \
> >   			ell/cipher.h \
> >   			ell/random.h \
> > 
> > @@ -74,6 +75,7 @@ ell_libell_la_SOURCES = $(linux_headers) \
> > 
> >   			ell/dbus-message.c \
> >   			ell/dbus-util.c \
> >   			ell/dbus-service.c \
> > 
> > +			ell/dbus-client.c \
> > 
> >   			ell/dbus-name-cache.c \
> >   			ell/dbus-filter.c \
> >   			ell/gvariant-private.h \
> > 
> > diff --git a/ell/dbus-client.c b/ell/dbus-client.c
> > new file mode 100644
> > index 0000000..e6b6ff3
> > --- /dev/null
> > +++ b/ell/dbus-client.c
> > @@ -0,0 +1,595 @@
> > +/*
> > + *
> > + *  Embedded Linux library
> > + *
> > + *  Copyright (C) 2011-2014  Intel Corporation. All rights reserved.
> > + *  Copyright (C) 2017  Codecoup. 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 "ell.h"
> > +#include "private.h"
> > +
> > +struct l_dbus_client {
> > +	struct l_dbus *dbus;
> > +	unsigned int watch;
> > +	unsigned int added_watch;
> > +	unsigned int removed_watch;
> > +	char *service;
> > +	char *path;
> > +	uint32_t objects_call;
> > +
> > +	l_dbus_watch_func_t connect_cb;
> > +	void *connect_cb_data;
> > +	l_dbus_destroy_func_t connect_cb_data_destroy;
> > +
> > +	l_dbus_watch_func_t disconnect_cb;
> > +	void *disconnect_cb_data;
> > +	l_dbus_destroy_func_t disconnect_cb_data_destroy;
> > +
> > +	l_dbus_client_ready_func_t ready_cb;
> > +	void *ready_cb_data;
> > +	l_dbus_destroy_func_t ready_cb_data_destroy; > +
> > +	l_dbus_client_proxy_func_t proxy_added_cb;
> > +	l_dbus_client_proxy_func_t proxy_removed_cb;
> > +	l_dbus_client_property_function_t properties_changed_cb;
> > +	void *proxy_cb_data;
> > +	l_dbus_destroy_func_t proxy_cb_data_destroy;
> > +
> > +	struct l_queue *proxies;
> > +};
> > +
> > +struct proxy_property {
> > +	char *name;
> > +	struct l_dbus_message *msg;
> > +};
> > +
> > +struct l_dbus_proxy {
> > +	int refcount;
> > +	struct l_dbus_client *client;
> > +	char *interface;
> > +	char *path;
> > +	uint32_t properties_watch;
> > +	bool ready;
> > +
> > +	struct l_queue *properties;
> > +};
> > +
> > +LIB_EXPORT const char *l_dbus_proxy_get_path(struct l_dbus_proxy *proxy)
> > +{
> > +	if (unlikely(!proxy))
> > +		return NULL;
> > +
> > +	return proxy->path;
> > +}
> > +
> > +LIB_EXPORT const char *l_dbus_proxy_get_interface(struct l_dbus_proxy
> > *proxy) +{
> > +	if (unlikely(!proxy))
> > +		return NULL;
> > +
> > +	return proxy->interface;
> > +}
> > +
> > +static bool find_property_match(const void *a, const void *b)
> 
> property_match_by_name?
> 
> > +{
> > +	const struct proxy_property *prop = a;
> > +	const char *name = b;
> > +
> > +	return !strcmp(prop->name, name);
> > +}
> > +
> > +static struct proxy_property *find_property(struct l_dbus_proxy *proxy,
> > +							const char *name)
> > +{
> > +	return l_queue_find(proxy->properties, find_property_match, name);
> > +}
> > +
> > +static struct proxy_property *get_property(struct l_dbus_proxy *proxy,
> > +							const char *name)
> > +{
> > +	struct proxy_property *prop;
> > +
> > +	prop = find_property(proxy, name);
> > +	if (prop)
> > +		return prop;
> > +
> > +	prop = l_new(struct proxy_property, 1);
> > +	prop->name = l_strdup(name);
> > +
> > +	l_queue_push_tail(proxy->properties, prop);
> > +
> > +	return prop;
> > +}
> > +
> > +LIB_EXPORT bool l_dbus_proxy_get_property(struct l_dbus_proxy *proxy,
> > +						const char *name,
> > +						const char *signature, ...)
> > +{
> > +	struct proxy_property *prop;
> > +	va_list args;
> > +	bool res;
> > +
> > +	if (unlikely(!proxy))
> > +		return false;
> > +
> > +	prop = find_property(proxy, name);
> > +	if (!prop)
> > +		return false;
> > +
> > +	va_start(args, signature);
> > +	res = l_dbus_message_get_arguments_valist(prop->msg, signature, args);
> > +	va_end(args);
> > +
> > +	return res;
> > +}
> > +
> > +static void property_free(void *data)
> > +{
> > +	struct proxy_property *prop = data;
> > +
> > +	if (prop->msg)
> > +		l_dbus_message_unref(prop->msg);
> > +
> > +	l_free(prop->name);
> > +	l_free(prop);
> > +}
> > +
> > +static struct l_dbus_proxy *l_dbus_proxy_ref(struct l_dbus_proxy *proxy)
> > +{
> > +	if (unlikely(!proxy))
> > +		return NULL;
> > +
> > +	__sync_fetch_and_add(&proxy->refcount, 1);
> > +
> > +	return proxy;
> > +}
> > +
> > +static void l_dbus_proxy_unref(struct l_dbus_proxy *proxy)
> > +{
> > +	if (unlikely(!proxy))
> > +		return;
> > +
> > +	if (__sync_sub_and_fetch(&proxy->refcount, 1))
> > +		return;
> > +
> > +	if (proxy->properties_watch)
> > +		l_dbus_remove_signal_watch(proxy->client->dbus,
> > +						proxy->properties_watch);
> > +
> > +	l_queue_destroy(proxy->properties, property_free);
> > +	l_free(proxy->interface);
> > +	l_free(proxy->path);
> > +	l_free(proxy);
> > +}
> > +
> > +static void proxy_update_property(struct l_dbus_proxy *proxy,
> > +					const char *name,
> > +					struct l_dbus_message_iter *property)
> > +{
> > +	struct l_dbus_message_builder *builder;
> > +	struct proxy_property *prop = get_property(proxy, name);
> > +
> > +	l_dbus_message_unref(prop->msg);
> > +
> > +	if (!property) {
> > +		prop->msg = NULL;
> > +		goto done;
> > +	}
> > +
> > +	prop->msg = l_dbus_message_new_signal(proxy->client->dbus, proxy-
>path,
> > +							proxy->interface, name);
> 
> So for every property we build a dummy message and put the contents of
> the property into the message, right?  E.g. If Powered = true was
> signaled, we build a signal with member="Powered" and a single byte as
> contents.
> 
> I'm okay with this being the first naive solution, but this seems quite
> wasteful.  Can we store the serialized property data without the
> enclosing message?  E.g. by using GVariant serializer and storing the
> raw data?

Yeah, I was aware of that when sending patches :) Just wasn't sure on what 
else could I use. I can fix that in subsequent patch if this is OK?

> 
> > +	if (!prop->msg)
> > +		return;
> > +
> > +	builder = l_dbus_message_builder_new(prop->msg);
> > +	l_dbus_message_builder_append_from_iter(builder, property);
> > +	l_dbus_message_builder_finalize(builder);
> > +	l_dbus_message_builder_destroy(builder);
> > +
> > +done:
> > +	if (proxy->client->properties_changed_cb && proxy->ready)
> 
> How does proxy->ready get set for manually created proxies?  Or is a
> proxy supposed to be always obtained by the proxy_added callback?  If
> so, why export the proxy_new constructor?

I expected proxy being obtained from callback so I'll not export proxy_new for 
time being. Acctually ref counting for proxy won't be needed anymore so I'll 
just remove it.

> 
> > +		proxy->client->properties_changed_cb(proxy, name, prop->msg,
> > +					proxy->client->proxy_cb_data_destroy);
> > +}
> > +
> > +static void proxy_invalidate_properties(struct l_dbus_proxy *proxy,
> > +					struct l_dbus_message_iter* props)
> > +{
> > +	const char *name;
> > +
> > +	while (l_dbus_message_iter_next_entry(props, &name))
> > +		proxy_update_property(proxy, name, NULL);
> > +}
> > +
> > +static void proxy_update_properties(struct l_dbus_proxy *proxy,
> > +					struct l_dbus_message_iter* props)
> > +{
> > +	struct l_dbus_message_iter variant;
> > +	const char *name;
> > +
> > +	while (l_dbus_message_iter_next_entry(props, &name, &variant))
> > +		proxy_update_property(proxy, name, &variant);
> > +}
> > +
> > +static void properties_changed_callback(struct l_dbus_message *message,
> > +								void *user_data)
> > +{
> > +	struct l_dbus_proxy *proxy = user_data;
> > +	const char *interface;
> > +	struct l_dbus_message_iter changed;
> > +	struct l_dbus_message_iter invalidated;
> > +
> > +	if (!l_dbus_message_get_arguments(message, "sa{sv}as", &interface,
> > +						&changed, &invalidated)) {
> > +		return;
> > +	}
> 
> No need for {}

Fixed.

> 
> > +
> > +	proxy_update_properties(proxy, &changed);
> > +	proxy_invalidate_properties(proxy, &invalidated);
> > +}
> > +
> > +LIB_EXPORT struct l_dbus_proxy *l_dbus_proxy_new(struct l_dbus_client
> > *client, +					const char *path, const char *interface)
> > +{
> > +	struct l_dbus_proxy *proxy;
> > +
> > +	proxy = l_new(struct l_dbus_proxy, 1);
> > +
> > +	proxy->properties_watch = l_dbus_add_signal_watch(client->dbus,
> > +						client->service, path,
> > +						L_DBUS_INTERFACE_PROPERTIES,
> > +						"PropertiesChanged",
> > +						L_DBUS_MATCH_ARGUMENT(0),
> > +						interface, L_DBUS_MATCH_NONE,
> > +						properties_changed_callback,
> > +						proxy);
> > +	if (!proxy->properties_watch) {
> > +		l_free(proxy);
> > +		return NULL;
> > +	}
> > +
> > +	proxy->refcount = 1;
> > +	proxy->client = client;
> > +	proxy->interface = l_strdup(interface);
> > +	proxy->path = l_strdup(path);
> > +	proxy->properties = l_queue_new();
> > +
> > +	l_queue_push_tail(client->proxies, proxy);
> > +
> > +	return proxy;
> > +}
> > +
> > +static bool is_ignorable(const char *interface)
> > +{
> > +	static const struct {
> > +		const char *interface;
> > +	} interfaces_to_ignore[] = {
> > +		{ L_DBUS_INTERFACE_OBJECT_MANAGER },
> > +		{ L_DBUS_INTERFACE_INTROSPECTABLE },
> > +		{ L_DBUS_INTERFACE_PROPERTIES },
> > +	};
> > +	size_t i;
> > +
> > +	for (i = 0; i < L_ARRAY_SIZE(interfaces_to_ignore); i++)
> > +		if (!strcmp(interfaces_to_ignore[i].interface, interface))
> > +			return true;
> > +
> > +	return false;
> > +}
> > +
> > +struct find_proxy_data
> > +{
> > +	const char *path;
> > +	const char *interface;
> > +};
> > +
> > +static bool find_proxy_match(const void *a, const void *b)
> > +{
> > +	const struct l_dbus_proxy *proxy = a;
> > +	const struct find_proxy_data *data = b;
> > +
> > +	return !strcmp(proxy->interface, data->interface) &&
> > +					!strcmp(proxy->path, data->path);
> > +}
> > +
> > +static struct l_dbus_proxy *find_proxy(struct l_dbus_client *client,
> > +					const char *path, const char *interface)
> > +{
> > +	struct find_proxy_data data;
> > +
> > +	data.path = path;
> > +	data.interface = interface;
> > +
> > +	return l_queue_find(client->proxies, find_proxy_match, &data);
> 
> It might be more compact to rewrite this using l_queue_get_entries.  No
> need to define struct find_proxy_data, etc.

Fixed.

> 
> > +}
> > +
> > +static void parse_interface(struct l_dbus_client *client, const char
> > *path, +					const char *interface,
> > +					struct l_dbus_message_iter *properties)
> > +{
> > +	struct l_dbus_proxy *proxy;
> > +	bool proxy_added = false;
> > +
> > +	if (is_ignorable(interface))
> > +		return;
> > +
> > +	proxy = find_proxy(client, path, interface);
> > +	if (!proxy) {
> > +		proxy = l_dbus_proxy_new(client, path, interface);
> > +		proxy_added = true;
> > +	}
> > +
> > +	if (!proxy)
> > +		return;
> > +
> > +	proxy_update_properties(proxy, properties);
> > +
> > +	if (proxy_added && client->proxy_added_cb) {
> > +		proxy->ready = true;
> 
> Why does setting ready depend on proxy_added_cb being valid?

Ups, fixed.

> 
> > +		client->proxy_added_cb(proxy, client->proxy_cb_data);
> > +	}
> > +}
> > +
> > +static void parse_object(struct l_dbus_client *client, const char *path,
> > +					struct l_dbus_message_iter *object)
> > +{
> > +	const char *interface;
> > +	struct l_dbus_message_iter properties;
> > +
> > +	if (!path)
> > +		return;
> > +
> > +	while (l_dbus_message_iter_next_entry(object, &interface, 
&properties))
> > +		parse_interface(client, path, interface, &properties);
> > +}
> > +
> > +static void interfaces_added_callback(struct l_dbus_message *message,
> > +								void *user_data)
> > +{
> > +	struct l_dbus_client *client = user_data;
> > +	struct l_dbus_message_iter object;
> > +	const char *path;
> > +
> > +	l_dbus_message_get_arguments(message, "oa{sa{sv}}", &path, &object);
> 
> Might want an error check here

Right, fixed.

> 
> > +
> > +	parse_object(client, path, &object);
> > +}
> > +
> > +static void interfaces_removed_callback(struct l_dbus_message *message,
> > +								void *user_data)
> > +{
> > +	struct l_dbus_client *client = user_data;
> > +	struct l_dbus_message_iter interfaces;
> > +	const char *interface;
> > +	const char *path;
> > +
> > +	l_dbus_message_get_arguments(message, "oas", &path, &interfaces);
> > +
> > +	while (l_dbus_message_iter_next_entry(&interfaces, &interface)) {
> > +		struct l_dbus_proxy *proxy;
> > +
> > +		proxy = find_proxy(client, path, interface);
> > +		if (!proxy)
> > +			continue;
> > +
> > +		l_queue_remove(proxy->client->proxies, proxy);
> > +
> > +		if (client->proxy_removed_cb)
> > +			client->proxy_removed_cb(proxy, client->proxy_cb_data);
> > +
> > +		l_dbus_proxy_unref(proxy);
> > +	}
> > +}
> > +
> > +static void get_managed_objects_reply(struct l_dbus_message *message,
> > +								void *user_data)
> > +{
> > +	struct l_dbus_client *client = user_data;
> > +	struct l_dbus_message_iter objects;
> > +	struct l_dbus_message_iter object;
> > +	const char *path;
> > +
> > +	client->objects_call = 0;
> > +
> > +	if (l_dbus_message_is_error(message)) {
> > +		return;
> > +	}
> 
> No need for {}

Fixed.

> 
> > +
> > +	l_dbus_message_get_arguments(message, "a{oa{sa{sv}}}", &objects);
> > +
> > +	while (l_dbus_message_iter_next_entry(&objects, &path, &object))
> > +		parse_object(client, path, &object);
> > +
> > +	client->added_watch = l_dbus_add_signal_watch(client->dbus,
> > +						client->service, "/",
> > +						L_DBUS_INTERFACE_OBJECT_MANAGER,
> > +						"InterfacesAdded",
> > +						L_DBUS_MATCH_NONE,
> > +						interfaces_added_callback,
> > +						client);
> > +
> > +	client->removed_watch = l_dbus_add_signal_watch(client->dbus,
> > +						client->service, "/",
> > +						L_DBUS_INTERFACE_OBJECT_MANAGER,
> > +						"InterfacesRemoved",
> > +						L_DBUS_MATCH_NONE,
> > +						interfaces_removed_callback,
> > +						client);
> > +
> > +	if (client->ready_cb)
> > +		client->ready_cb(client, client->ready_cb_data);
> > +}
> > +
> > +static void service_appeared_callback(struct l_dbus *dbus, void
> > *user_data) +{
> > +	struct l_dbus_client *client = user_data;
> > +
> > +	/* TODO should we allow to set different root? */
> > +	client->objects_call = l_dbus_method_call(dbus, client->service, "/",
> 
> Should "/" be client->path instead?  Note that ell only exports
> ObjectManager on the root path.  It doesn't export it for any child
> paths.  However, BlueZ might act differently.

BlueZ also exports it on "/". 
Maybe to cover all we could have root param in l_dbus_client_new()?
Other option would be to add eg l_dbus_client_new_full() that would
allow to specify root while l_dbus_client_new() would always use "/"?

> 
> > +						L_DBUS_INTERFACE_OBJECT_MANAGER,
> > +						"GetManagedObjects", NULL,
> > +						get_managed_objects_reply,
> > +						client, NULL);
> > +
> > +	if (client->connect_cb)
> > +		client->connect_cb(client->dbus, client->connect_cb_data);
> > +}
> > +
> > +static void service_disappeared_callback(struct l_dbus *dbus, void
> > *user_data) +{
> > +	struct l_dbus_client *client = user_data;
> > +
> > +	if (client->disconnect_cb)
> > +		client->disconnect_cb(client->dbus, client->disconnect_cb_data);
> > +
> > +
> 
> No double empty lines please

Fixed.

> 
> > +	l_queue_clear(client->proxies,
> > +				(l_queue_destroy_func_t)l_dbus_proxy_unref);
> > +}
> > +
> > +LIB_EXPORT struct l_dbus_client *l_dbus_client_new(struct l_dbus *dbus,
> > +					const char *service, const char *path)
> > +{
> > +	struct l_dbus_client *client;
> > +
> > +	client = l_new(struct l_dbus_client, 1);
> > +
> > +	client->dbus = dbus;
> > +
> > +	client->watch = l_dbus_add_service_watch(dbus, service,
> > +						service_appeared_callback,
> > +						service_disappeared_callback,
> > +						client, NULL);
> > +
> > +	if (!client->watch) {
> > +		l_free(client);
> > +		return NULL;
> > +	}
> > +
> > +	client->service = l_strdup(service);
> > +	client->path = l_strdup(path);
> > +	client->proxies = l_queue_new();
> > +
> > +	return client;
> > +}
> > +
> > +LIB_EXPORT void l_dbus_client_destroy(struct l_dbus_client *client)
> > +{
> > +	if (unlikely(!client))
> > +		return;
> > +
> > +	if (client->watch)
> > +		l_dbus_remove_signal_watch(client->dbus, client->watch);
> > +
> > +	if (client->added_watch)
> > +		l_dbus_remove_signal_watch(client->dbus, client->added_watch);
> > +
> > +	if (client->removed_watch)
> > +		l_dbus_remove_signal_watch(client->dbus, client->removed_watch);
> > +
> > +	if (client->connect_cb_data_destroy)
> > +		client->connect_cb_data_destroy(client->connect_cb_data);
> > +
> > +	if (client->disconnect_cb_data_destroy)
> > +		client->disconnect_cb_data_destroy(client->disconnect_cb_data);
> > +
> > +	if (client->ready_cb_data_destroy)
> > +		client->ready_cb_data_destroy(client->ready_cb_data);
> > +
> > +	if (client->proxy_cb_data_destroy)
> > +		client->proxy_cb_data_destroy(client->proxy_cb_data);
> > +
> > +	if (client->objects_call)
> > +		l_dbus_cancel(client->dbus, client->objects_call)
> > +;
> > +	l_queue_destroy(client->proxies,
> > +				(l_queue_destroy_func_t)l_dbus_proxy_unref);
> > +
> > +	l_free(client->service);
> > +	l_free(client->path);
> > +	l_free(client);
> > +}
> > +
> > +LIB_EXPORT bool l_dbus_client_set_connect_handler(struct l_dbus_client
> > *client, +						l_dbus_watch_func_t function,
> > +						void *user_data,
> > +						l_dbus_destroy_func_t destroy)
> > +{
> > +	if (unlikely(!client))
> > +		return false;
> > +
> > +	client->connect_cb = function;
> > +	client->connect_cb_data = user_data;
> > +	client->connect_cb_data_destroy = destroy;
> > +
> > +	return true;
> > +}
> > +
> > +LIB_EXPORT bool l_dbus_client_set_disconnect_handler(struct l_dbus_client
> > *client, +						l_dbus_watch_func_t function,
> > +						void *user_data,
> > +						l_dbus_destroy_func_t destroy)
> > +{
> > +	if (unlikely(!client))
> > +		return false;
> > +
> > +	client->disconnect_cb = function;
> > +	client->disconnect_cb_data = user_data;
> > +	client->disconnect_cb_data_destroy = destroy;
> > +
> > +	return true;
> > +}
> > +
> > +LIB_EXPORT bool l_dbus_client_set_ready_handler(struct l_dbus_client
> > *client, +					l_dbus_client_ready_func_t function,
> > +					void *user_data,
> > +					l_dbus_destroy_func_t destroy)
> > +{
> > +	if (unlikely(!client))
> > +		return false;
> > +
> > +	client->ready_cb = function;
> > +	client->ready_cb_data = user_data;
> > +	client->ready_cb_data_destroy = destroy;
> > +
> > +	return true;
> > +}
> > +
> > +LIB_EXPORT bool l_dbus_client_set_proxy_handlers(struct l_dbus_client
> > *client, +					l_dbus_client_proxy_func_t proxy_added,
> > +					l_dbus_client_proxy_func_t proxy_removed,
> > +					l_dbus_client_property_function_t property_changed,
> > +					void *user_data,
> > +					l_dbus_destroy_func_t destroy)
> > +{
> > +	if (unlikely(!client))
> > +		return false;
> > +
> > +	client->proxy_added_cb = proxy_added;
> > +	client->proxy_removed_cb = proxy_removed;
> > +	client->properties_changed_cb = property_changed;
> > +	client->proxy_cb_data = user_data;
> > +	client->proxy_cb_data_destroy = destroy;
> > +
> > +	return true;
> > +}
> > diff --git a/ell/dbus-client.h b/ell/dbus-client.h
> > new file mode 100644
> > index 0000000..b32f4a3
> > --- /dev/null
> > +++ b/ell/dbus-client.h
> > @@ -0,0 +1,87 @@
> > +/*
> > + *
> > + *  Embedded Linux library
> > + *
> > + *  Copyright (C) 2011-2014  Intel Corporation. All rights reserved.
> > + *  Copyright (C) 2017  Codecoup. 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 + *
> > + */
> > +
> > +#ifndef __ELL_DBUS_CLIENT_H
> > +#define __ELL_DBUS_CLIENT_H
> > +
> > +#include <stdbool.h>
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +struct l_dbus;
> > +struct l_dbus_message;
> > +struct l_dbus_client;
> > +struct l_dbus_proxy;
> > +
> > +typedef void (*l_dbus_client_ready_func_t)(struct l_dbus_client *client,
> > +							void *user_data);
> > +typedef void (*l_dbus_client_proxy_func_t) (struct l_dbus_proxy *proxy,
> > +							void *user_data);
> > +typedef void (*l_dbus_client_property_function_t) (struct l_dbus_proxy
> > *proxy, +						const char *name,
> > +						struct l_dbus_message *msg,
> > +						void *user_data);
> > +
> > +struct l_dbus_client *l_dbus_client_new(struct l_dbus *dbus,
> > +					const char *service, const char *path);
> > +void l_dbus_client_destroy(struct l_dbus_client *client);
> > +
> > +bool l_dbus_client_set_connect_handler(struct l_dbus_client *client,
> > +						l_dbus_watch_func_t function,
> > +						void *user_data,
> > +						l_dbus_destroy_func_t destroy);
> > +
> > +bool l_dbus_client_set_disconnect_handler(struct l_dbus_client *client,
> > +						l_dbus_watch_func_t function,
> > +						void *user_data,
> > +						l_dbus_destroy_func_t destroy);
> > +
> > +bool l_dbus_client_set_ready_handler(struct l_dbus_client *client,
> > +					l_dbus_client_ready_func_t function,
> > +					void *user_data,
> > +					l_dbus_destroy_func_t destroy);
> > +
> > +bool l_dbus_client_set_proxy_handlers(struct l_dbus_client *client,
> > +					l_dbus_client_proxy_func_t proxy_added,
> > +					l_dbus_client_proxy_func_t proxy_removed,
> > +					l_dbus_client_property_function_t property_changed,
> > +					void *user_data,
> > +					l_dbus_destroy_func_t destroy);
> > +
> > +struct l_dbus_proxy *l_dbus_proxy_new(struct l_dbus_client *client,
> > +							const char *path,
> > +							const char *interface);
> > +
> > +const char *l_dbus_proxy_get_path(struct l_dbus_proxy *proxy);
> > +
> > +const char *l_dbus_proxy_get_interface(struct l_dbus_proxy *proxy);
> > +
> > +bool l_dbus_proxy_get_property(struct l_dbus_proxy *proxy, const char
> > *name, +						const char *signature, ...);
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* __ELL_DBUS_CLIENT_H */
> > diff --git a/ell/ell.h b/ell/ell.h
> > index 2efd1c4..4083ee5 100644
> > --- a/ell/ell.h
> > +++ b/ell/ell.h
> > @@ -51,3 +51,4 @@
> > 
> >   #include <ell/genl.h>
> >   #include <ell/dbus.h>
> >   #include <ell/dbus-service.h>
> > 
> > +#include <ell/dbus-client.h>
> 
> Regards,
> -Denis


-- 
pozdrawiam
Szymon Janc

Re: [RFC 4/6] dbus: Add support for proxy interface

[Denis Kenzior]

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

Hi Szymon,

>> I'm okay with this being the first naive solution, but this seems quite
>> wasteful.  Can we store the serialized property data without the
>> enclosing message?  E.g. by using GVariant serializer and storing the
>> raw data?
> 
> Yeah, I was aware of that when sending patches :) Just wasn't sure on what
> else could I use. I can fix that in subsequent patch if this is OK?
> 

That's fine.

>>> +	/* TODO should we allow to set different root? */
>>> +	client->objects_call = l_dbus_method_call(dbus, client->service, "/",
>>
>> Should "/" be client->path instead?  Note that ell only exports
>> ObjectManager on the root path.  It doesn't export it for any child
>> paths.  However, BlueZ might act differently.
> 
> BlueZ also exports it on "/".
> Maybe to cover all we could have root param in l_dbus_client_new()?
> Other option would be to add eg l_dbus_client_new_full() that would
> allow to specify root while l_dbus_client_new() would always use "/"?
> 

If you have no usecase for this right now I would simply not worry about 
it for now.  Just drop the client parameter from client_new and if we 
ever need this at some point we can either add a _set function or a new 
constructor.

Regards,
-Denis

[RFC 5/6] dbus: Add support for setting properties on D-Bus proxy interface

[Szymon Janc]

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

l_dbus_proxy_set_property function allows to set properties via proxy
interface.
---
 ell/dbus-client.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ell/dbus-client.h |  8 +++++
 2 files changed, 104 insertions(+)

diff --git a/ell/dbus-client.c b/ell/dbus-client.c
index e6b6ff3..3f8c49f 100644
--- a/ell/dbus-client.c
+++ b/ell/dbus-client.c
@@ -182,6 +182,102 @@ static void l_dbus_proxy_unref(struct l_dbus_proxy *proxy)
 	l_free(proxy);
 }
 
+struct set_prop_data
+{
+	struct l_dbus_proxy *proxy;
+	l_dbus_client_proxy_result_func_t result;
+	void *user_data;
+	l_dbus_destroy_func_t destroy;
+};
+
+static void set_prop_data_free(void *user_data)
+{
+	struct set_prop_data* prop = user_data;
+
+	if (prop->destroy)
+		prop->destroy(prop->user_data);
+
+	l_dbus_proxy_unref(prop->proxy);
+	l_free(prop);
+}
+
+static void set_prop_reply(struct l_dbus_message *message, void *user_data)
+{
+	struct set_prop_data* prop = user_data;
+
+	prop->result(prop->proxy, message, prop->user_data);
+}
+
+LIB_EXPORT bool l_dbus_proxy_set_property(struct l_dbus_proxy *proxy,
+				l_dbus_client_proxy_result_func_t result,
+				void *user_data, l_dbus_destroy_func_t destroy,
+				const char *name, const char *signature, ...)
+{
+	struct l_dbus_client *client = proxy->client;
+	struct l_dbus_message_builder *builder;
+	struct set_prop_data *set_prop_data;
+	struct l_dbus_message *message;
+	struct proxy_property *prop;
+	va_list args;
+
+	if (unlikely(!proxy))
+		return false;
+
+	prop = find_property(proxy, name);
+	if (!prop)
+		return false;
+
+	if (strcmp(l_dbus_message_get_signature(prop->msg), signature))
+		return false;
+
+	message = l_dbus_message_new_method_call(client->dbus, client->service,
+						proxy->path,
+						L_DBUS_INTERFACE_PROPERTIES,
+						"Set");
+	if (!message)
+		return false;
+
+	builder = l_dbus_message_builder_new(message);
+	if (!builder) {
+		l_dbus_message_unref(message);
+		return false;
+	}
+
+	l_dbus_message_builder_append_basic(builder, 's', proxy->interface);
+	l_dbus_message_builder_append_basic(builder, 's', name);
+
+	l_dbus_message_builder_enter_variant(builder, signature);
+
+	va_start(args, signature);
+	l_dbus_message_builder_append_from_valist(builder, signature, args);
+	va_end(args);
+
+	l_dbus_message_builder_leave_variant(builder);
+
+	l_dbus_message_builder_finalize(builder);
+	l_dbus_message_builder_destroy(builder);
+
+
+	if (!result) {
+		return l_dbus_send(client->dbus, message);
+	}
+
+	set_prop_data = l_new(struct set_prop_data, 1);
+	set_prop_data->proxy = l_dbus_proxy_ref(proxy);
+	set_prop_data->result = result;
+	set_prop_data->user_data = user_data;
+	set_prop_data->destroy = destroy;
+
+	if (!l_dbus_send_with_reply(client->dbus, message, set_prop_reply,
+					set_prop_data, set_prop_data_free)) {
+		l_dbus_proxy_unref(set_prop_data->proxy);
+		l_free(set_prop_data);
+		return false;
+	}
+
+	return true;
+}
+
 static void proxy_update_property(struct l_dbus_proxy *proxy,
 					const char *name,
 					struct l_dbus_message_iter *property)
diff --git a/ell/dbus-client.h b/ell/dbus-client.h
index b32f4a3..a65d8bb 100644
--- a/ell/dbus-client.h
+++ b/ell/dbus-client.h
@@ -39,6 +39,9 @@ typedef void (*l_dbus_client_ready_func_t)(struct l_dbus_client *client,
 							void *user_data);
 typedef void (*l_dbus_client_proxy_func_t) (struct l_dbus_proxy *proxy,
 							void *user_data);
+typedef void (*l_dbus_client_proxy_result_func_t) (struct l_dbus_proxy *proxy,
+						struct l_dbus_message *result,
+						void *user_data);
 typedef void (*l_dbus_client_property_function_t) (struct l_dbus_proxy *proxy,
 						const char *name,
 						struct l_dbus_message *msg,
@@ -80,6 +83,11 @@ const char *l_dbus_proxy_get_interface(struct l_dbus_proxy *proxy);
 
 bool l_dbus_proxy_get_property(struct l_dbus_proxy *proxy, const char *name,
 						const char *signature, ...);
+
+bool l_dbus_proxy_set_property(struct l_dbus_proxy *proxy,
+				l_dbus_client_proxy_result_func_t result,
+				void *user_data, l_dbus_destroy_func_t destroy,
+				const char *name, const char *signature, ...);
 #ifdef __cplusplus
 }
 #endif
-- 
2.14.3

Re: [RFC 5/6] dbus: Add support for setting properties on D-Bus proxy interface

[Denis Kenzior]

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

Hi Szymon,

On 11/29/2017 09:16 AM, Szymon Janc wrote:
> l_dbus_proxy_set_property function allows to set properties via proxy
> interface.
> ---
>   ell/dbus-client.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ell/dbus-client.h |  8 +++++
>   2 files changed, 104 insertions(+)
> 
> diff --git a/ell/dbus-client.c b/ell/dbus-client.c
> index e6b6ff3..3f8c49f 100644
> --- a/ell/dbus-client.c
> +++ b/ell/dbus-client.c
> @@ -182,6 +182,102 @@ static void l_dbus_proxy_unref(struct l_dbus_proxy *proxy)
>   	l_free(proxy);
>   }
>   
> +struct set_prop_data

I hate the name.  Maybe set_property_request?

> +{
> +	struct l_dbus_proxy *proxy;
> +	l_dbus_client_proxy_result_func_t result;
> +	void *user_data;
> +	l_dbus_destroy_func_t destroy;
> +};
> +
> +static void set_prop_data_free(void *user_data)
> +{
> +	struct set_prop_data* prop = user_data;
> +
> +	if (prop->destroy)
> +		prop->destroy(prop->user_data);
> +
> +	l_dbus_proxy_unref(prop->proxy);
> +	l_free(prop);
> +}
> +
> +static void set_prop_reply(struct l_dbus_message *message, void *user_data)
> +{
> +	struct set_prop_data* prop = user_data;
> +
> +	prop->result(prop->proxy, message, prop->user_data);
> +}
> +
> +LIB_EXPORT bool l_dbus_proxy_set_property(struct l_dbus_proxy *proxy,
> +				l_dbus_client_proxy_result_func_t result,
> +				void *user_data, l_dbus_destroy_func_t destroy,
> +				const char *name, const char *signature, ...)
> +{
> +	struct l_dbus_client *client = proxy->client;
> +	struct l_dbus_message_builder *builder;
> +	struct set_prop_data *set_prop_data;
> +	struct l_dbus_message *message;
> +	struct proxy_property *prop;
> +	va_list args;
> +
> +	if (unlikely(!proxy))
> +		return false;
> +
> +	prop = find_property(proxy, name);
> +	if (!prop)
> +		return false;
> +
> +	if (strcmp(l_dbus_message_get_signature(prop->msg), signature))
> +		return false;
> +
> +	message = l_dbus_message_new_method_call(client->dbus, client->service,
> +						proxy->path,
> +						L_DBUS_INTERFACE_PROPERTIES,
> +						"Set");
> +	if (!message)
> +		return false;
> +
> +	builder = l_dbus_message_builder_new(message);
> +	if (!builder) {
> +		l_dbus_message_unref(message);
> +		return false;
> +	}
> +
> +	l_dbus_message_builder_append_basic(builder, 's', proxy->interface);
> +	l_dbus_message_builder_append_basic(builder, 's', name);
> +
> +	l_dbus_message_builder_enter_variant(builder, signature);
> +
> +	va_start(args, signature);
> +	l_dbus_message_builder_append_from_valist(builder, signature, args);
> +	va_end(args);
> +
> +	l_dbus_message_builder_leave_variant(builder);
> +
> +	l_dbus_message_builder_finalize(builder);
> +	l_dbus_message_builder_destroy(builder);
> +
> +
> +	if (!result) {
> +		return l_dbus_send(client->dbus, message);
> +	}

No need for {}

> +
> +	set_prop_data = l_new(struct set_prop_data, 1);
> +	set_prop_data->proxy = l_dbus_proxy_ref(proxy);

So its a bit weird that a proxy_removed might have been called, yet an 
outstanding call for that proxy is ongoing...  No idea how to fix this 
nicely but some options:

- cancel all pending requests when the proxy is destroyed
- set a destroyed flag on the proxy and don't bother calling the 
callback in set_prop_reply
- something else?

> +	set_prop_data->result = result;
> +	set_prop_data->user_data = user_data;
> +	set_prop_data->destroy = destroy;
> +
> +	if (!l_dbus_send_with_reply(client->dbus, message, set_prop_reply,
> +					set_prop_data, set_prop_data_free)) {
> +		l_dbus_proxy_unref(set_prop_data->proxy);
> +		l_free(set_prop_data);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>   static void proxy_update_property(struct l_dbus_proxy *proxy,
>   					const char *name,
>   					struct l_dbus_message_iter *property)
> diff --git a/ell/dbus-client.h b/ell/dbus-client.h
> index b32f4a3..a65d8bb 100644
> --- a/ell/dbus-client.h
> +++ b/ell/dbus-client.h
> @@ -39,6 +39,9 @@ typedef void (*l_dbus_client_ready_func_t)(struct l_dbus_client *client,
>   							void *user_data);
>   typedef void (*l_dbus_client_proxy_func_t) (struct l_dbus_proxy *proxy,
>   							void *user_data);
> +typedef void (*l_dbus_client_proxy_result_func_t) (struct l_dbus_proxy *proxy,
> +						struct l_dbus_message *result,
> +						void *user_data);
>   typedef void (*l_dbus_client_property_function_t) (struct l_dbus_proxy *proxy,
>   						const char *name,
>   						struct l_dbus_message *msg,
> @@ -80,6 +83,11 @@ const char *l_dbus_proxy_get_interface(struct l_dbus_proxy *proxy);
>   
>   bool l_dbus_proxy_get_property(struct l_dbus_proxy *proxy, const char *name,
>   						const char *signature, ...);
> +
> +bool l_dbus_proxy_set_property(struct l_dbus_proxy *proxy,
> +				l_dbus_client_proxy_result_func_t result,
> +				void *user_data, l_dbus_destroy_func_t destroy,
> +				const char *name, const char *signature, ...);
>   #ifdef __cplusplus
>   }
>   #endif
> 

Regards,
-Denis

Re: [RFC 5/6] dbus: Add support for setting properties on D-Bus proxy interface

[Szymon Janc]

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

On Wednesday, 29 November 2017 18:05:14 CET Denis Kenzior wrote:
> Hi Szymon,
> 
> On 11/29/2017 09:16 AM, Szymon Janc wrote:
> > l_dbus_proxy_set_property function allows to set properties via proxy
> > interface.
> > ---
> > 
> >   ell/dbus-client.c | 96
> >   +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   ell/dbus-client.h |  8 +++++
> >   2 files changed, 104 insertions(+)
> > 
> > diff --git a/ell/dbus-client.c b/ell/dbus-client.c
> > index e6b6ff3..3f8c49f 100644
> > --- a/ell/dbus-client.c
> > +++ b/ell/dbus-client.c
> > @@ -182,6 +182,102 @@ static void l_dbus_proxy_unref(struct l_dbus_proxy
> > *proxy)> 
> >   	l_free(proxy);
> >   
> >   }
> > 
> > +struct set_prop_data
> 
> I hate the name.  Maybe set_property_request?

I've named it "method_call_request" as it will also be used for tracking proxy 
method calls (and seting property is just a special method call).

> 
> > +{
> > +	struct l_dbus_proxy *proxy;
> > +	l_dbus_client_proxy_result_func_t result;
> > +	void *user_data;
> > +	l_dbus_destroy_func_t destroy;
> > +};
> > +
> > +static void set_prop_data_free(void *user_data)
> > +{
> > +	struct set_prop_data* prop = user_data;
> > +
> > +	if (prop->destroy)
> > +		prop->destroy(prop->user_data);
> > +
> > +	l_dbus_proxy_unref(prop->proxy);
> > +	l_free(prop);
> > +}
> > +
> > +static void set_prop_reply(struct l_dbus_message *message, void
> > *user_data) +{
> > +	struct set_prop_data* prop = user_data;
> > +
> > +	prop->result(prop->proxy, message, prop->user_data);
> > +}
> > +
> > +LIB_EXPORT bool l_dbus_proxy_set_property(struct l_dbus_proxy *proxy,
> > +				l_dbus_client_proxy_result_func_t result,
> > +				void *user_data, l_dbus_destroy_func_t destroy,
> > +				const char *name, const char *signature, ...)
> > +{
> > +	struct l_dbus_client *client = proxy->client;
> > +	struct l_dbus_message_builder *builder;
> > +	struct set_prop_data *set_prop_data;
> > +	struct l_dbus_message *message;
> > +	struct proxy_property *prop;
> > +	va_list args;
> > +
> > +	if (unlikely(!proxy))
> > +		return false;
> > +
> > +	prop = find_property(proxy, name);
> > +	if (!prop)
> > +		return false;
> > +
> > +	if (strcmp(l_dbus_message_get_signature(prop->msg), signature))
> > +		return false;
> > +
> > +	message = l_dbus_message_new_method_call(client->dbus, client-
>service,
> > +						proxy->path,
> > +						L_DBUS_INTERFACE_PROPERTIES,
> > +						"Set");
> > +	if (!message)
> > +		return false;
> > +
> > +	builder = l_dbus_message_builder_new(message);
> > +	if (!builder) {
> > +		l_dbus_message_unref(message);
> > +		return false;
> > +	}
> > +
> > +	l_dbus_message_builder_append_basic(builder, 's', proxy->interface);
> > +	l_dbus_message_builder_append_basic(builder, 's', name);
> > +
> > +	l_dbus_message_builder_enter_variant(builder, signature);
> > +
> > +	va_start(args, signature);
> > +	l_dbus_message_builder_append_from_valist(builder, signature, args);
> > +	va_end(args);
> > +
> > +	l_dbus_message_builder_leave_variant(builder);
> > +
> > +	l_dbus_message_builder_finalize(builder);
> > +	l_dbus_message_builder_destroy(builder);
> > +
> > +
> > +	if (!result) {
> > +		return l_dbus_send(client->dbus, message);
> > +	}
> 
> No need for {}

Fixed.

> 
> > +
> > +	set_prop_data = l_new(struct set_prop_data, 1);
> > +	set_prop_data->proxy = l_dbus_proxy_ref(proxy);
> 
> So its a bit weird that a proxy_removed might have been called, yet an
> outstanding call for that proxy is ongoing...  No idea how to fix this
> nicely but some options:
> 
> - cancel all pending requests when the proxy is destroyed
> - set a destroyed flag on the proxy and don't bother calling the
> callback in set_prop_reply
> - something else?

Right, I've miss that. I'll go with queue of pending calls and cancel those 
when proxy is destroyed.

> 
> > +	set_prop_data->result = result;
> > +	set_prop_data->user_data = user_data;
> > +	set_prop_data->destroy = destroy;
> > +
> > +	if (!l_dbus_send_with_reply(client->dbus, message, set_prop_reply,
> > +					set_prop_data, set_prop_data_free)) {
> > +		l_dbus_proxy_unref(set_prop_data->proxy);
> > +		l_free(set_prop_data);
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > 
> >   static void proxy_update_property(struct l_dbus_proxy *proxy,
> >   
> >   					const char *name,
> >   					struct l_dbus_message_iter *property)
> > 
> > diff --git a/ell/dbus-client.h b/ell/dbus-client.h
> > index b32f4a3..a65d8bb 100644
> > --- a/ell/dbus-client.h
> > +++ b/ell/dbus-client.h
> > @@ -39,6 +39,9 @@ typedef void (*l_dbus_client_ready_func_t)(struct
> > l_dbus_client *client,> 
> >   							void *user_data);
> >   
> >   typedef void (*l_dbus_client_proxy_func_t) (struct l_dbus_proxy *proxy,
> >   
> >   							void *user_data);
> > 
> > +typedef void (*l_dbus_client_proxy_result_func_t) (struct l_dbus_proxy
> > *proxy, +						struct l_dbus_message *result,
> > +						void *user_data);
> > 
> >   typedef void (*l_dbus_client_property_function_t) (struct l_dbus_proxy
> >   *proxy,>   
> >   						const char *name,
> >   						struct l_dbus_message *msg,
> > 
> > @@ -80,6 +83,11 @@ const char *l_dbus_proxy_get_interface(struct
> > l_dbus_proxy *proxy);> 
> >   bool l_dbus_proxy_get_property(struct l_dbus_proxy *proxy, const char
> >   *name,>   
> >   						const char *signature, ...);
> > 
> > +
> > +bool l_dbus_proxy_set_property(struct l_dbus_proxy *proxy,
> > +				l_dbus_client_proxy_result_func_t result,
> > +				void *user_data, l_dbus_destroy_func_t destroy,
> > +				const char *name, const char *signature, ...);
> > 
> >   #ifdef __cplusplus
> >   }
> >   #endif
> 
> Regards,
> -Denis


-- 
pozdrawiam
Szymon Janc

Re: [RFC 5/6] dbus: Add support for setting properties on D-Bus proxy interface

[Denis Kenzior]

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

Hi Szymon,

>> So its a bit weird that a proxy_removed might have been called, yet an >> outstanding call for that proxy is ongoing...  No idea how to fix this
>> nicely but some options:
>>
>> - cancel all pending requests when the proxy is destroyed
>> - set a destroyed flag on the proxy and don't bother calling the
>> callback in set_prop_reply
>> - something else?
> 
> Right, I've miss that. I'll go with queue of pending calls and cancel those
> when proxy is destroyed.
> 

So another option would be to introduce l_dbus_cancel_by_group, similar 
to how ofono.git/drivers/mbimmodem/mbim.h does things.  The idea is that 
we add a group parameter to dbus_send_full and store the gid as part of 
the dbus message_callback.  Then you can cancel all pending messages 
with a given gid.

Proxy would then obtain a unique gid when its created and we could get 
rid of the queue for the proxy's pending messages.

Roughly:
uint32_t _dbus_send_with_reply_full(struct l_dbus *dbus,
                                 struct l_dbus_message *message,
				uint32_t gid,
                                 l_dbus_message_func_t function,
                                 void *user_data,
				l_dbus_destroy_func_t destroy);
bool _dbus_cancel_group(struct l_dbus *dbus, uint32_t gid);

uint32_t _dbus_generate_gid(struct l_dbus *dbus);

Regards,
-Denis

[RFC 6/6] dbus: Add convenient API for calling method on proxy interface

[Szymon Janc]

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

This is simple wrapper around l_dbus_method_call.
---
 ell/dbus-client.c | 41 +++++++++++++++++++++++++++++++++++++++++
 ell/dbus-client.h |  7 +++++++
 2 files changed, 48 insertions(+)

diff --git a/ell/dbus-client.c b/ell/dbus-client.c
index 3f8c49f..4eacd01 100644
--- a/ell/dbus-client.c
+++ b/ell/dbus-client.c
@@ -278,6 +278,47 @@ LIB_EXPORT bool l_dbus_proxy_set_property(struct l_dbus_proxy *proxy,
 	return true;
 }
 
+struct proxy_call_data
+{
+	struct l_dbus_proxy *proxy;
+	l_dbus_client_proxy_result_func_t reply;
+	void *user_data;
+	l_dbus_destroy_func_t destroy;
+};
+
+static void proxy_call_data_reply(struct l_dbus_message *message,
+							void *user_data)
+{
+	struct proxy_call_data *data = user_data;
+
+	if (data->reply)
+		data->reply(data->proxy, message, data->user_data);
+}
+
+LIB_EXPORT uint32_t l_dbus_proxy_method_call(struct l_dbus_proxy *proxy,
+						const char *method,
+						l_dbus_message_func_t setup,
+						l_dbus_client_proxy_result_func_t reply,
+						void *user_data,
+						l_dbus_destroy_func_t destroy)
+{
+	struct proxy_call_data *call_data;
+
+	if (unlikely(!proxy))
+		return 0;
+
+	call_data = l_new(struct proxy_call_data, 1);
+	call_data->proxy = proxy;
+	call_data->reply = reply;
+	call_data->user_data = user_data;
+	call_data->destroy = destroy;
+
+	return l_dbus_method_call(proxy->client->dbus, proxy->client->service,
+					proxy->path, proxy->interface, method,
+					setup, proxy_call_data_reply, call_data,
+					l_free);
+}
+
 static void proxy_update_property(struct l_dbus_proxy *proxy,
 					const char *name,
 					struct l_dbus_message_iter *property)
diff --git a/ell/dbus-client.h b/ell/dbus-client.h
index a65d8bb..ebfbd77 100644
--- a/ell/dbus-client.h
+++ b/ell/dbus-client.h
@@ -88,6 +88,13 @@ bool l_dbus_proxy_set_property(struct l_dbus_proxy *proxy,
 				l_dbus_client_proxy_result_func_t result,
 				void *user_data, l_dbus_destroy_func_t destroy,
 				const char *name, const char *signature, ...);
+
+uint32_t l_dbus_proxy_method_call(struct l_dbus_proxy *proxy,
+				const char *method,
+				l_dbus_message_func_t setup,
+				l_dbus_client_proxy_result_func_t reply,
+				void *user_data, l_dbus_destroy_func_t destroy);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.14.3