10:36 a.m.
Hello guys,
In order to get ell ready for ObjectManager support, I started to work on the
very first necessary feature: the org.freedesktop.DBus.Properties interface.
I wanted to go with an API that we all know already, which has proven to be
easy to use, which works well and so on: the GDBus API. The other big reason
is that if we have to move from GDBus to ell, it would take a very little
amount of time to do it since it would be a tiny naming issue and that's it.
No logic rework at all. I know this will help a lot when moving our projects
from GDBus to ell.
There is already a dbus service API in ell. I wanted to keep it, wrapping the
gdbus-like API within the existing functions. However that has proven to be
impossible unless I would modify the existing API. Thus my conclusion: if I
have to break the existing API, let's do it all. So I finnally got rid of the
existing API.
That said, I kept the internals as much as they are. There are actually nice
improvements in it compared to GDBus. All objects hold only their subpath, and
not the whole path every time, interface instances reduce the memory usage (one
interface might be used in 1+ instances), etc...
I also kept the current unit tests, adding module test for this new interface.
(it really runs on top of DBus, so it's not a unit test anymore). Tried it with
valgrind, which went well.
I am sending it as an RFC/PATCH, since I would like to get your inputs on some
parts. Like the struct _dbus_pending_data for instance, used to answer a Set()
method call and/or when sending a PropertiesChanged() signal. I tried not to
bloat it, but there are information like the whole patch which is required.
About that one: is it better to do what I did, or to hold a pointer on the
object and recreate the path from it? I went for the quick and easy one,
storing the whole path: after all such pending data are not there for long. But
if you think recreating the path is not a bad idea to keep a bit of memory, I
can change it.
Also, patch 4, due to the port of the constants structure, it looses the
pre-built signature, thus adding the necessity to add a compare_signature()
function as in GDBus. It was nice to get this full pre-built signature that
could be compared at once. If you have ideas how to get it without changing the
API... Like changing the macro L_DBUS_ARGS() or L_DBUS_METHOD, so it would set
a const char * in_signature/out_signature in struct l_dbus_method for instance?
About other improvements:
- patch 1 is a tiny helper I required in patch 15, since ell checks the
signature of the variant we try to set, before calling the set function.
- patch 2 and 3 are there to separate the code properly. It hads thus a getter
function for the object tree, but there is no shared structure informations
between dbus.c and dbus-service.c. I figured out it was cleaner that way imo.
Minor issues:
- I put the error message macros in dbus-private.h, figuring that some other
part of the code could need it. But maybe it's better no to share those?
- When it's required to send an empty reply, it would be good to have a helper
function I think, which wraps l_dbus_message_new_method_return() +
l_dbus_message_set_arguments + l_dbus_send together (see patch 15 in
function l_dbus_pending_property_success).
Next:
- Finally implementing ObjectManager interface, with the 2 functions that
enable/disable it as in GDBus.
- We really need to implement DBus signal watch/filters in ell
- That last task done, we could provide a nice client proxy API as GDBus
does as well.
Tomasz Bursztyka (21):
dbus: Add a utility function to check an message iter signature
dbus: Add a private getter for the unique dbus tree in dbus struct
dbus: All interface related code is found in dbus-service.c
dbus: Refactor the interface API so it uses constants methods array
unit: Apply the DBus interface methods API change to the tests
examples: Apply the DBus interface methods API change
dbus: Refactor the interface API so it uses constant signals array
unit: Apply the DBus interface signals API change to the tests
examples: Apply the DBus interface signals API change
dbus: Refactor the interface API so it uses constant properties array
unit: Apply the DBus interface properties API change to the tests
examples: Apply the DBus interface properties API change
dbus: Add support for org.freedesktop.DBus.Properties.Get method
dbus: Add support for org.freedesktop.DBus.Properties.GetAll method
dbus: Add support for org.freedesktop.DBus.Properties.Set method
dbus: Add support for checking if a property exists
unit: Apply the changes related to interface property API
examples: Apply the changes related to interface property API
dbus: Add org.freedesktop.DBus.Properties.PropertiesChanged signal
unit: Add a test for the org.freedesktop.DBus.Properties interface
git: Ignoring the org.freedesktop.DBus.Properties interface test
binary
.gitignore | 1 +
Makefile.am | 3 +
ell/dbus-message.c | 13 +
ell/dbus-private.h | 48 ++-
ell/dbus-service.c | 999 +++++++++++++++++++++++++++++--------------
ell/dbus-service.h | 158 ++++++-
ell/dbus.c | 29 +-
ell/dbus.h | 11 +-
examples/dbus-service.c | 43 +-
unit/test-dbus-freedesktop.c | 347 +++++++++++++++
unit/test-dbus-service.c | 80 ++--
unit/test-kdbus.c | 13 +-
12 files changed, 1302 insertions(+), 443 deletions(-)
create mode 100644 unit/test-dbus-freedesktop.c
--
2.0.4
10:36 a.m.
New subject: [PATCH 01/21] dbus: Add a utility function to check an message iter signature
It will be useful sometime to verify the signature of a message iter
before doing anything with it. It will be useful when one knows the
signature and wants to defer the parsing to another part if only the
signature matches.
---
ell/dbus-message.c | 13 +++++++++++++
ell/dbus.h | 2 ++
2 files changed, 15 insertions(+)
diff --git a/ell/dbus-message.c b/ell/dbus-message.c
index 159df82..79e9e26 100644
--- a/ell/dbus-message.c
+++ b/ell/dbus-message.c
@@ -1331,6 +1331,19 @@ LIB_EXPORT bool l_dbus_message_iter_get_variant(
return result;
}
+LIB_EXPORT bool l_dbus_message_iter_has_signature(
+ struct l_dbus_message_iter *iter,
+ const char *signature)
+{
+ if (unlikely(!iter))
+ return false;
+
+ if (!iter->sig_start || strcmp(iter->sig_start, signature))
+ return false;
+
+ return true;
+}
+
void _dbus_message_set_sender(struct l_dbus_message *message,
const char *sender)
{
diff --git a/ell/dbus.h b/ell/dbus.h
index a6ad109..6dc9fc6 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -148,6 +148,8 @@ bool l_dbus_message_iter_is_valid(struct l_dbus_message_iter *iter);
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,
const char *signature, ...);
+bool l_dbus_message_iter_has_signature(struct l_dbus_message_iter *iter,
+ const char *signature);
bool l_dbus_message_set_arguments(struct l_dbus_message *message,
const char *signature, ...);
--
2.0.4
10:36 a.m.
New subject: [PATCH 02/21] dbus: Add a private getter for the unique dbus tree in dbus struct
This will be useful for properly refactoring all interface related code
into dbus-service.c.
---
ell/dbus-private.h | 1 +
ell/dbus.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 9987294..a34f444 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -210,5 +210,6 @@ int _dbus_kernel_add_match(int fd, uint64_t bloom_size, uint64_t
bloom_n_hash,
uint64_t *out_cookie);
int _dbus_kernel_remove_match(int fd, uint64_t cookie);
+struct _dbus_object_tree *_dbus_object_tree_get_from_dbus(struct l_dbus *dbus);
uint8_t _dbus_get_version(struct l_dbus *dbus);
int _dbus_get_fd(struct l_dbus *dbus);
diff --git a/ell/dbus.c b/ell/dbus.c
index 4c4ba2d..d5d58a9 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -1199,6 +1199,11 @@ int _dbus_get_fd(struct l_dbus *dbus)
return l_io_get_fd(dbus->io);
}
+struct _dbus_object_tree *_dbus_object_tree_get_from_dbus(struct l_dbus *dbus)
+{
+ return dbus->tree;
+}
+
LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
const char *path, const char *interface,
l_dbus_interface_setup_func_t setup_func,
--
2.0.4
10:36 a.m.
New subject: [PATCH 03/21] dbus: All interface related code is found in dbus-service.c
This is necessary so it will be possible to register an interface at
once through description tables. Also makes the overall code split
more relevant.
---
ell/dbus-service.c | 35 +++++++++++++++++++++++++++++++++++
ell/dbus-service.h | 10 ++++++++++
ell/dbus.c | 29 -----------------------------
ell/dbus.h | 9 ---------
4 files changed, 45 insertions(+), 38 deletions(-)
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index e082226..a38de64 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -848,3 +848,38 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
return true;
}
+
+LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
+ const char *path, const char *interface,
+ l_dbus_interface_setup_func_t setup_func,
+ void *user_data,
+ l_dbus_destroy_func_t destroy)
+{
+ struct _dbus_object_tree *tree;
+
+ if (unlikely(!dbus))
+ return false;
+
+ tree = _dbus_object_tree_get_from_dbus(dbus);
+ if (unlikely(!tree))
+ return false;
+
+ return _dbus_object_tree_register(tree, path, interface,
+ setup_func, user_data, destroy);
+}
+
+LIB_EXPORT bool l_dbus_unregister_interface(struct l_dbus *dbus,
+ const char *path,
+ const char *interface)
+{
+ struct _dbus_object_tree *tree;
+
+ if (unlikely(!dbus))
+ return false;
+
+ tree = _dbus_object_tree_get_from_dbus(dbus);
+ if (unlikely(!tree))
+ return false;
+
+ return _dbus_object_tree_unregister(tree, path, interface);
+}
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index ce53082..6071740 100644
--- a/ell/dbus-service.h
+++ b/ell/dbus-service.h
@@ -49,10 +49,20 @@ enum l_dbus_property_flag {
L_DBUS_PROPERTY_FLAG_WRITABLE = 2,
};
+typedef void (*l_dbus_interface_setup_func_t) (struct l_dbus_interface *);
typedef struct l_dbus_message *(*l_dbus_interface_method_cb_t) (struct l_dbus *,
struct l_dbus_message *message,
void *user_data);
+bool l_dbus_register_interface(struct l_dbus *dbus,
+ const char *path, const char *interface,
+ l_dbus_interface_setup_func_t setup_func,
+ void *user_data,
+ l_dbus_destroy_func_t destroy);
+
+bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
+ const char *interface);
+
bool l_dbus_interface_method(struct l_dbus_interface *interface,
const char *name, uint32_t flags,
l_dbus_interface_method_cb_t cb,
diff --git a/ell/dbus.c b/ell/dbus.c
index d5d58a9..ff7e814 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -1203,32 +1203,3 @@ struct _dbus_object_tree *_dbus_object_tree_get_from_dbus(struct
l_dbus *dbus)
{
return dbus->tree;
}
-
-LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
- const char *path, const char *interface,
- l_dbus_interface_setup_func_t setup_func,
- void *user_data,
- l_dbus_destroy_func_t destroy)
-{
- if (unlikely(!dbus))
- return false;
-
- if (unlikely(!dbus->tree))
- return false;
-
- return _dbus_object_tree_register(dbus->tree, path, interface,
- setup_func, user_data, destroy);
-}
-
-LIB_EXPORT bool l_dbus_unregister_interface(struct l_dbus *dbus,
- const char *path,
- const char *interface)
-{
- if (unlikely(!dbus))
- return false;
-
- if (unlikely(!dbus->tree))
- return false;
-
- return _dbus_object_tree_unregister(dbus->tree, path, interface);
-}
diff --git a/ell/dbus.h b/ell/dbus.h
index 6dc9fc6..cec7c8c 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -46,7 +46,6 @@ typedef void (*l_dbus_disconnect_func_t) (void *user_data);
typedef void (*l_dbus_debug_func_t) (const char *str, void *user_data);
typedef void (*l_dbus_destroy_func_t) (void *user_data);
-typedef void (*l_dbus_interface_setup_func_t) (struct l_dbus_interface *);
struct l_dbus *l_dbus_new(const char *address);
struct l_dbus *l_dbus_new_default(enum l_dbus_bus bus);
@@ -190,14 +189,6 @@ bool l_dbus_message_builder_leave_variant(
struct l_dbus_message *l_dbus_message_builder_finalize(
struct l_dbus_message_builder *builder);
-
-bool l_dbus_register_interface(struct l_dbus *dbus,
- const char *path, const char *interface,
- l_dbus_interface_setup_func_t setup_func,
- void *user_data,
- l_dbus_destroy_func_t destroy);
-bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
- const char *interface);
#ifdef __cplusplus
}
#endif
--
2.0.4
10:36 a.m.
New subject: [PATCH 04/21] dbus: Refactor the interface API so it uses constants methods array
This makes ELL DBus interface API more in line with what exists in
GDBus. The only drawback is when processing incoming message: the
signature comparison is slightly slower since it does not compare it at
once. This will need to be fixed if that proves to be an issue.
---
ell/dbus-private.h | 15 +++--
ell/dbus-service.c | 190 ++++++++++++++++++-----------------------------------
ell/dbus-service.h | 57 ++++++++++++++--
3 files changed, 126 insertions(+), 136 deletions(-)
diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index a34f444..0fe5946 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -58,7 +58,8 @@ struct dbus_header {
struct dbus_builder;
struct l_string;
struct l_dbus_interface;
-struct _dbus_method;
+struct l_dbus_argument;
+struct l_dbus_method;
struct _dbus_signal;
struct _dbus_property;
struct l_dbus_message_iter;
@@ -141,8 +142,10 @@ bool _dbus_valid_bus_name(const char *bus_name);
bool _dbus_header_is_valid(void *data, size_t size);
-void _dbus_method_introspection(struct _dbus_method *info,
- struct l_string *buf);
+void _dbus_print_arguments(const struct l_dbus_argument *args,
+ const char *direction, struct l_string *buf);
+void _dbus_method_introspection(const struct l_dbus_method *info,
+ struct l_string *buf);
void _dbus_signal_introspection(struct _dbus_signal *info,
struct l_string *buf);
void _dbus_property_introspection(struct _dbus_property *info,
@@ -153,7 +156,10 @@ void _dbus_interface_introspection(struct l_dbus_interface
*interface,
struct l_dbus_interface *_dbus_interface_new(const char *interface);
void _dbus_interface_free(struct l_dbus_interface *interface);
-struct _dbus_method *_dbus_interface_find_method(struct l_dbus_interface *i,
+bool _dbus_interface_methods(struct l_dbus_interface *interface,
+ const struct l_dbus_method *methods);
+
+const struct l_dbus_method *_dbus_interface_find_method(struct l_dbus_interface *i,
const char *method);
struct _dbus_signal *_dbus_interface_find_signal(struct l_dbus_interface *i,
const char *signal);
@@ -171,6 +177,7 @@ void _dbus_object_tree_prune_node(struct object_node *node);
bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
const char *path, const char *interface,
+ const struct l_dbus_method *methods,
void (*setup_func)(struct l_dbus_interface *),
void *user_data, void (*destroy) (void *));
bool _dbus_object_tree_unregister(struct _dbus_object_tree *tree,
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index a38de64..6f2eee7 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -47,13 +47,6 @@ static const char *static_introspectable =
"\t\t\t<arg name=\"xml\" type=\"s\"
direction=\"out\"/>\n"
"\t\t</method>\n\t</interface>\n";
-struct _dbus_method {
- l_dbus_interface_method_cb_t cb;
- uint32_t flags;
- unsigned char name_len;
- char metainfo[];
-};
-
struct _dbus_signal {
uint32_t flags;
unsigned char name_len;
@@ -67,7 +60,7 @@ struct _dbus_property {
};
struct l_dbus_interface {
- struct l_queue *methods;
+ const struct l_dbus_method *methods;
struct l_queue *signals;
struct l_queue *properties;
char name[];
@@ -97,44 +90,29 @@ struct _dbus_object_tree {
struct object_node *root;
};
-void _dbus_method_introspection(struct _dbus_method *info,
- struct l_string *buf)
-{
- const char *sig;
- const char *end;
- const char *pname;
- unsigned int offset = info->name_len + 1;
-
- l_string_append_printf(buf, "\t\t<method name=\"%s\">\n",
- info->metainfo);
-
- sig = info->metainfo + offset;
- offset += strlen(sig) + 1;
-
- for (; *sig; sig++) {
- end = _dbus_signature_end(sig);
- pname = info->metainfo + offset;
-
- l_string_append_printf(buf, "\t\t\t<arg name=\"%s\" "
- "type=\"%.*s\" direction=\"in\"/>\n",
- pname, (int) (end - sig + 1), sig);
- sig = end;
- offset += strlen(pname) + 1;
+void _dbus_print_arguments(const struct l_dbus_argument *args,
+ const char *direction, struct l_string *buf)
+{
+ for (; args && args->name; args++) {
+ l_string_append_printf(buf,
+ "\t\t\t<arg name=\"%s\" type=\"%s\"",
+ args->name, args->signature);
+ if (direction)
+ l_string_append_printf(buf,
+ " direction=\"%s\"/>\n",
+ direction);
+ else
+ l_string_append_printf(buf, "/>\n");
}
+}
- sig = info->metainfo + offset;
- offset += strlen(sig) + 1;
+void _dbus_method_introspection(const struct l_dbus_method *info,
+ struct l_string *buf)
+{
+ l_string_append_printf(buf, "\t\t<method name=\"%s\">\n",
info->name);
- for (; *sig; sig++) {
- end = _dbus_signature_end(sig);
- pname = info->metainfo + offset;
-
- l_string_append_printf(buf, "\t\t\t<arg name=\"%s\" "
- "type=\"%.*s\" direction=\"out\"/>\n",
- pname, (int) (end - sig + 1), sig);
- sig = end;
- offset += strlen(pname) + 1;
- }
+ _dbus_print_arguments(info->in_args, "in", buf);
+ _dbus_print_arguments(info->out_args, "out", buf);
if (info->flags & L_DBUS_METHOD_FLAG_DEPRECATED)
l_string_append(buf, "\t\t\t<annotation name=\""
@@ -209,11 +187,13 @@ void _dbus_property_introspection(struct _dbus_property *info,
void _dbus_interface_introspection(struct l_dbus_interface *interface,
struct l_string *buf)
{
+ const struct l_dbus_method *method;
+
l_string_append_printf(buf, "\t<interface name=\"%s\">\n",
interface->name);
- l_queue_foreach(interface->methods,
- (l_queue_foreach_func_t) _dbus_method_introspection, buf);
+ for (method = interface->methods; method && method->name; method++)
+ _dbus_method_introspection(method, buf);
l_queue_foreach(interface->signals,
(l_queue_foreach_func_t) _dbus_signal_introspection, buf);
l_queue_foreach(interface->properties,
@@ -256,74 +236,18 @@ static bool size_params(const char *signature, va_list args,
unsigned int *len)
return true;
}
-LIB_EXPORT bool l_dbus_interface_method(struct l_dbus_interface *interface,
- const char *name, uint32_t flags,
- l_dbus_interface_method_cb_t cb,
- const char *return_sig,
- const char *param_sig, ...)
+bool _dbus_interface_methods(struct l_dbus_interface *interface,
+ const struct l_dbus_method *methods)
{
- va_list args;
- unsigned int return_info_len;
- unsigned int param_info_len;
- struct _dbus_method *info;
- char *p;
-
- if (!_dbus_valid_method(name))
- return false;
-
- if (unlikely(!return_sig || !param_sig))
- return false;
-
- if (return_sig[0] && !_dbus_valid_signature(return_sig))
+ if (unlikely(!interface))
return false;
- if (param_sig[0] && !_dbus_valid_signature(param_sig))
+ if (unlikely(!methods))
return false;
- /* Pre-calculate the needed meta-info length */
- return_info_len = strlen(return_sig) + 1;
- param_info_len = strlen(param_sig) + 1;
-
- va_start(args, param_sig);
-
- if (!size_params(return_sig, args, &return_info_len))
- goto error;
-
- if (!size_params(param_sig, args, ¶m_info_len))
- goto error;
-
- va_end(args);
-
- info = l_malloc(sizeof(*info) + return_info_len +
- param_info_len + strlen(name) + 1);
- info->cb = cb;
- info->flags = flags;
- info->name_len = strlen(name);
- strcpy(info->metainfo, name);
-
- va_start(args, param_sig);
-
- /*
- * We store param signature + parameter names first, to speed up
- * lookups during the message dispatch procedures.
- */
- p = info->metainfo + info->name_len + param_info_len + 1;
- p = stpcpy(p, return_sig) + 1;
- p = copy_params(p, return_sig, args);
-
- p = info->metainfo + info->name_len + 1;
- p = stpcpy(p, param_sig) + 1;
- p = copy_params(p, param_sig, args);
-
- va_end(args);
-
- l_queue_push_tail(interface->methods, info);
+ interface->methods = methods;
return true;
-
-error:
- va_end(args);
- return false;
}
LIB_EXPORT bool l_dbus_interface_signal(struct l_dbus_interface *interface,
@@ -428,7 +352,6 @@ struct l_dbus_interface *_dbus_interface_new(const char *name)
interface = l_malloc(sizeof(*interface) + strlen(name) + 1);
- interface->methods = l_queue_new();
interface->signals = l_queue_new();
interface->properties = l_queue_new();
@@ -439,28 +362,23 @@ struct l_dbus_interface *_dbus_interface_new(const char *name)
void _dbus_interface_free(struct l_dbus_interface *interface)
{
- l_queue_destroy(interface->methods, l_free);
l_queue_destroy(interface->signals, l_free);
l_queue_destroy(interface->properties, l_free);
l_free(interface);
}
-static bool match_method(const void *a, const void *b)
+const struct l_dbus_method *_dbus_interface_find_method(struct l_dbus_interface *i,
+ const char *method)
{
- const struct _dbus_method *method = a;
- const char *name = b;
-
- if (!strcmp(method->metainfo, name))
- return true;
+ const struct l_dbus_method *m;
- return false;
-}
+ for (m = i->methods; m && m->name; m++) {
+ if (!strcmp(m->name, method))
+ return m;
+ }
-struct _dbus_method *_dbus_interface_find_method(struct l_dbus_interface *i,
- const char *method)
-{
- return l_queue_find(i->methods, match_method, (char *) method);
+ return NULL;
}
static bool match_signal(const void *a, const void *b)
@@ -667,6 +585,7 @@ void _dbus_object_tree_prune_node(struct object_node *node)
bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
const char *path, const char *interface,
+ const struct l_dbus_method *methods,
void (*setup_func)(struct l_dbus_interface *),
void *user_data, void (*destroy) (void *))
{
@@ -704,6 +623,7 @@ bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
dbi = l_hashmap_lookup(tree->interfaces, interface);
if (!dbi) {
dbi = _dbus_interface_new(interface);
+ dbi->methods = methods;
setup_func(dbi);
l_hashmap_insert(tree->interfaces, dbi->name, dbi);
}
@@ -783,6 +703,26 @@ void _dbus_object_tree_introspect(struct _dbus_object_tree *tree,
l_string_append(buf, "</node>\n");
}
+static bool compare_signature(const struct l_dbus_argument *args,
+ const char *sig)
+{
+ const char *p = NULL;
+
+ for (; args && args->signature && *sig; args++) {
+ p = args->signature;
+
+ for (; *sig && *p; sig++, p++) {
+ if (*p != *sig)
+ return false;
+ }
+ }
+
+ if (*sig || (p && *p) || (args && args->signature))
+ return false;
+
+ return true;
+}
+
bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
struct l_dbus *dbus,
struct l_dbus_message *message)
@@ -791,10 +731,9 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
const char *interface;
const char *member;
const char *msg_sig;
- const char *sig;
struct object_node *node;
struct interface_instance *instance;
- struct _dbus_method *method;
+ const struct l_dbus_method *method;
struct l_dbus_message *reply;
path = l_dbus_message_get_path(message);
@@ -837,9 +776,7 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
if (!method)
return false;
- sig = method->metainfo + method->name_len + 1;
-
- if (strcmp(msg_sig, sig))
+ if (!compare_signature(method->in_args, msg_sig))
return false;
reply = method->cb(dbus, message, instance->user_data);
@@ -851,6 +788,7 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
const char *path, const char *interface,
+ const struct l_dbus_method *methods,
l_dbus_interface_setup_func_t setup_func,
void *user_data,
l_dbus_destroy_func_t destroy)
@@ -864,7 +802,7 @@ LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
if (unlikely(!tree))
return false;
- return _dbus_object_tree_register(tree, path, interface,
+ return _dbus_object_tree_register(tree, path, interface, methods,
setup_func, user_data, destroy);
}
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index 6071740..f3dceae 100644
--- a/ell/dbus-service.h
+++ b/ell/dbus-service.h
@@ -54,8 +54,59 @@ typedef struct l_dbus_message *(*l_dbus_interface_method_cb_t) (struct
l_dbus *,
struct l_dbus_message *message,
void *user_data);
+struct l_dbus_argument {
+ const char *name;
+ const char *signature;
+};
+
+struct l_dbus_method {
+ const char *name;
+ l_dbus_interface_method_cb_t cb;
+ enum l_dbus_method_flag flags;
+ const struct l_dbus_argument *in_args;
+ const struct l_dbus_argument *out_args;
+};
+
+#define L_DBUS_ARGS(args...) (const struct l_dbus_argument[]) { args, { } }
+
+#define L_DBUS_METHOD(_name, _in_args, _out_args, _cb) \
+ .name = _name, \
+ .in_args = _in_args, \
+ .out_args = _out_args, \
+ .cb = _cb
+
+#define L_DBUS_DEPRECATED_METHOD(_name, _in_args, _out_args, _cb) \
+ .name = _name, \
+ .in_args = _in_args, \
+ .out_args = _out_args, \
+ .cb = _cb, \
+ .flags = L_DBUS_METHOD_FLAG_DEPRECATED
+
+#define L_DBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _cb) \
+ .name = _name, \
+ .in_args = _in_args, \
+ .out_args = _out_args, \
+ .cb = _cb, \
+ .flags = L_DBUS_METHOD_FLAG_NOREPLY
+
+#define L_DBUS_ASYNC_METHOD(_name, _in_args, _out_args, _cb) \
+ .name = _name, \
+ .in_args = _in_args, \
+ .out_args = _out_args, \
+ .cb = _cb, \
+ .flags = L_DBUS_METHOD_FLAG_ASYNC
+
+#define L_DBUS_DEPRECATED_ASYNC_METHOD(_name, _in_args, _out_args, _cb) \
+ .name = _name, \
+ .in_args = _in_args, \
+ .out_args = _out_args, \
+ .cb = _cb, \
+ .flags = L_DBUS_METHOD_FLAG_DEPRECATED | L_DBUS_METHOD_FLAG_ASYNC
+
+
bool l_dbus_register_interface(struct l_dbus *dbus,
const char *path, const char *interface,
+ const struct l_dbus_method *methods,
l_dbus_interface_setup_func_t setup_func,
void *user_data,
l_dbus_destroy_func_t destroy);
@@ -63,12 +114,6 @@ bool l_dbus_register_interface(struct l_dbus *dbus,
bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
const char *interface);
-bool l_dbus_interface_method(struct l_dbus_interface *interface,
- const char *name, uint32_t flags,
- l_dbus_interface_method_cb_t cb,
- const char *return_sig, const char *param_sig,
- ...);
-
bool l_dbus_interface_signal(struct l_dbus_interface *interface,
const char *name, uint32_t flags,
const char *signature, ...);
--
2.0.4
10:36 a.m.
New subject: [PATCH 05/21] unit: Apply the DBus interface methods API change to the tests
Use constant methods array instead of function calls.
---
unit/test-dbus-service.c | 38 ++++++++++++++++++++++++++------------
unit/test-kdbus.c | 12 ++++++------
2 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/unit/test-dbus-service.c b/unit/test-dbus-service.c
index 7452269..6e77e78 100644
--- a/unit/test-dbus-service.c
+++ b/unit/test-dbus-service.c
@@ -107,7 +107,7 @@ static const struct introspect_test interface_test = {
static void test_introspect_method(const void *test_data)
{
const struct introspect_test *test = test_data;
- struct _dbus_method *method;
+ const struct l_dbus_method *method;
struct l_string *buf;
char *xml;
@@ -238,8 +238,6 @@ static struct l_dbus_message *get_modems_callback(struct l_dbus
*dbus,
static void build_manager_interface(struct l_dbus_interface *iface)
{
- l_dbus_interface_method(iface, "GetModems", 0, get_modems_callback,
- "a(oa{sv})", "", "modems");
l_dbus_interface_signal(iface, "ModemAdded", 0,
"oa{sv}", "path", "properties");
l_dbus_interface_signal(iface, "ModemRemoved", 0,
@@ -268,6 +266,13 @@ static const char *ofono_manager_introspection =
"\t<node name=\"phonesim\"/>\n"
"</node>\n";
+static const struct l_dbus_method ofono_methods[] = {
+ { L_DBUS_METHOD("GetModems",
+ NULL, L_DBUS_ARGS({ "modems", "a(oa{sv})" }),
+ get_modems_callback) },
+ { },
+};
+
static void test_dbus_object_tree_introspection(const void *test_data)
{
struct _dbus_object_tree *tree;
@@ -277,7 +282,7 @@ static void test_dbus_object_tree_introspection(const void
*test_data)
tree = _dbus_object_tree_new();
_dbus_object_tree_register(tree, "/", "org.ofono.Manager",
- build_manager_interface,
+ ofono_methods, build_manager_interface,
NULL, NULL);
_dbus_object_tree_makepath(tree, "/phonesim");
@@ -299,7 +304,7 @@ static void test_dbus_object_tree_dispatch(const void *test_data)
tree = _dbus_object_tree_new();
_dbus_object_tree_register(tree, "/", "org.ofono.Manager",
- build_manager_interface,
+ ofono_methods, build_manager_interface,
dummy_data, NULL);
message = _dbus_message_new_method_call(1, "org.ofono", "/",
@@ -315,6 +320,21 @@ static void test_dbus_object_tree_dispatch(const void *test_data)
_dbus_object_tree_free(tree);
}
+static const struct l_dbus_method sample_methods[] = {
+ { L_DBUS_DEPRECATED_METHOD("Frobate",
+ L_DBUS_ARGS({ "foo", "i" }),
+ L_DBUS_ARGS({ "bar", "s"}, { "baz", "a{us}"}),
+ NULL) },
+ { L_DBUS_METHOD("Bazify",
+ L_DBUS_ARGS({ "bar", "(iiu)"}),
+ L_DBUS_ARGS({ "bar", "v"}),
+ NULL) },
+ { L_DBUS_METHOD("Mogrify",
+ L_DBUS_ARGS({ "bar", "(iiav)" }),
+ NULL, NULL) },
+ { },
+};
+
int main(int argc, char *argv[])
{
int ret;
@@ -323,13 +343,7 @@ int main(int argc, char *argv[])
l_test_init(&argc, &argv);
- l_dbus_interface_method(interface, "Frobate",
- L_DBUS_METHOD_FLAG_DEPRECATED,
- NULL, "sa{us}", "i", "bar", "baz",
"foo");
- l_dbus_interface_method(interface, "Bazify", 0, NULL, "v",
"(iiu)",
- "bar", "bar");
- l_dbus_interface_method(interface, "Mogrify", 0, NULL, "",
- "(iiav)", "bar");
+ _dbus_interface_methods(interface, sample_methods);
l_dbus_interface_signal(interface, "Changed", 0, "b",
"new_value");
diff --git a/unit/test-kdbus.c b/unit/test-kdbus.c
index 4bf41bb..817a39f 100644
--- a/unit/test-kdbus.c
+++ b/unit/test-kdbus.c
@@ -116,11 +116,11 @@ static void service_ready_callback(void *user_data)
l_dbus_send(dbus, message);
}
-static void setup_test_interface(struct l_dbus_interface *interface)
-{
- l_dbus_interface_method(interface, "TestMethod", 0,
- test_method_call, "", "");
-}
+static const struct l_dbus_method test_methods[] = {
+ { L_DBUS_METHOD("TestMethod",
+ NULL, NULL, test_method_call) },
+ { },
+};
int main(int argc, char *argv[])
{
@@ -159,7 +159,7 @@ int main(int argc, char *argv[])
service, NULL);
if (!l_dbus_register_interface(service, "/test", "org.test",
- setup_test_interface, NULL, NULL)) {
+ test_methods, NULL, NULL, NULL)) {
l_info("Unable to register interface");
goto error;
}
--
2.0.4
10:36 a.m.
New subject: [PATCH 06/21] examples: Apply the DBus interface methods API change
Use constant methods array instead of function calls.
---
examples/dbus-service.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index 40c1646..5630cad 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -189,15 +189,6 @@ static struct l_dbus_message *test_method_call(struct l_dbus *dbus,
static void setup_test_interface(struct l_dbus_interface *interface)
{
- l_dbus_interface_method(interface, "GetProperties", 0,
- test_get_properties,
- "a{sv}", "", "properties");
- l_dbus_interface_method(interface, "SetProperty", 0,
- test_set_property,
- "", "sv", "name", "value");
- l_dbus_interface_method(interface, "MethodCall", 0,
- test_method_call, "", "");
-
l_dbus_interface_signal(interface, "PropertyChanged", 0,
"sv", "name", "value");
@@ -205,6 +196,18 @@ static void setup_test_interface(struct l_dbus_interface *interface)
l_dbus_interface_rw_property(interface, "Integer", "u");
}
+static const struct l_dbus_method test_methods[] = {
+ { L_DBUS_METHOD("GetProperties",
+ L_DBUS_ARGS({ "properties", "a{sv}" }), NULL,
+ test_get_properties) },
+ { L_DBUS_METHOD("SetProperty",
+ NULL, L_DBUS_ARGS({ "name", "s" }, { "value",
"v"}),
+ test_set_property) },
+ { L_DBUS_METHOD("MethodCall",
+ NULL, NULL, test_method_call) },
+ { },
+};
+
int main(int argc, char *argv[])
{
struct l_dbus *dbus;
@@ -235,8 +238,8 @@ int main(int argc, char *argv[])
test->integer = 42;
if (!l_dbus_register_interface(dbus, "/test", "org.test",
- setup_test_interface, test,
- test_data_destroy)) {
+ test_methods, setup_test_interface,
+ test, test_data_destroy)) {
l_info("Unable to register interface");
test_data_destroy(test);
goto cleanup;
--
2.0.4
10:36 a.m.
New subject: [PATCH 07/21] dbus: Refactor the interface API so it uses constant signals array
This makes ELL DBus interface API more in line with what exists in
GDBus.
---
ell/dbus-private.h | 11 ++--
ell/dbus-service.c | 144 ++++++++++-------------------------------------------
ell/dbus-service.h | 19 +++++--
3 files changed, 48 insertions(+), 126 deletions(-)
diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 0fe5946..5248dd6 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -60,7 +60,7 @@ struct l_string;
struct l_dbus_interface;
struct l_dbus_argument;
struct l_dbus_method;
-struct _dbus_signal;
+struct l_dbus_signal;
struct _dbus_property;
struct l_dbus_message_iter;
struct l_dbus_message;
@@ -146,8 +146,8 @@ void _dbus_print_arguments(const struct l_dbus_argument *args,
const char *direction, struct l_string *buf);
void _dbus_method_introspection(const struct l_dbus_method *info,
struct l_string *buf);
-void _dbus_signal_introspection(struct _dbus_signal *info,
- struct l_string *buf);
+void _dbus_signal_introspection(const struct l_dbus_signal *info,
+ struct l_string *buf);
void _dbus_property_introspection(struct _dbus_property *info,
struct l_string *buf);
void _dbus_interface_introspection(struct l_dbus_interface *interface,
@@ -158,10 +158,12 @@ void _dbus_interface_free(struct l_dbus_interface *interface);
bool _dbus_interface_methods(struct l_dbus_interface *interface,
const struct l_dbus_method *methods);
+bool _dbus_interface_signals(struct l_dbus_interface *interface,
+ const struct l_dbus_signal *signals);
const struct l_dbus_method *_dbus_interface_find_method(struct l_dbus_interface *i,
const char *method);
-struct _dbus_signal *_dbus_interface_find_signal(struct l_dbus_interface *i,
+const struct l_dbus_signal *_dbus_interface_find_signal(struct l_dbus_interface *i,
const char *signal);
struct _dbus_property *_dbus_interface_find_property(struct l_dbus_interface *i,
const char *property);
@@ -178,6 +180,7 @@ void _dbus_object_tree_prune_node(struct object_node *node);
bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
const char *path, const char *interface,
const struct l_dbus_method *methods,
+ const struct l_dbus_signal *signals,
void (*setup_func)(struct l_dbus_interface *),
void *user_data, void (*destroy) (void *));
bool _dbus_object_tree_unregister(struct _dbus_object_tree *tree,
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 6f2eee7..8996407 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -47,12 +47,6 @@ static const char *static_introspectable =
"\t\t\t<arg name=\"xml\" type=\"s\"
direction=\"out\"/>\n"
"\t\t</method>\n\t</interface>\n";
-struct _dbus_signal {
- uint32_t flags;
- unsigned char name_len;
- char metainfo[];
-};
-
struct _dbus_property {
uint32_t flags;
unsigned char name_len;
@@ -61,7 +55,7 @@ struct _dbus_property {
struct l_dbus_interface {
const struct l_dbus_method *methods;
- struct l_queue *signals;
+ const struct l_dbus_signal *signals;
struct l_queue *properties;
char name[];
};
@@ -127,30 +121,12 @@ void _dbus_method_introspection(const struct l_dbus_method *info,
l_string_append(buf, "\t\t</method>\n");
}
-void _dbus_signal_introspection(struct _dbus_signal *info,
- struct l_string *buf)
+void _dbus_signal_introspection(const struct l_dbus_signal *info,
+ struct l_string *buf)
{
- const char *sig;
- const char *end;
- const char *pname;
- unsigned int offset = info->name_len + 1;
+ l_string_append_printf(buf, "\t\t<signal name=\"%s\">\n",
info->name);
- l_string_append_printf(buf, "\t\t<signal name=\"%s\">\n",
- info->metainfo);
-
- sig = info->metainfo + offset;
- offset += strlen(sig) + 1;
-
- for (; *sig; sig++) {
- end = _dbus_signature_end(sig);
- pname = info->metainfo + offset;
-
- l_string_append_printf(buf, "\t\t\t<arg name=\"%s\" "
- "type=\"%.*s\"/>\n",
- pname, (int) (end - sig + 1), sig);
- sig = end;
- offset += strlen(pname) + 1;
- }
+ _dbus_print_arguments(info->args, NULL, buf);
if (info->flags & L_DBUS_SIGNAL_FLAG_DEPRECATED)
l_string_append(buf, "\t\t\t<annotation name=\""
@@ -188,54 +164,21 @@ void _dbus_interface_introspection(struct l_dbus_interface
*interface,
struct l_string *buf)
{
const struct l_dbus_method *method;
+ const struct l_dbus_signal *signal;
l_string_append_printf(buf, "\t<interface name=\"%s\">\n",
interface->name);
for (method = interface->methods; method && method->name; method++)
_dbus_method_introspection(method, buf);
- l_queue_foreach(interface->signals,
- (l_queue_foreach_func_t) _dbus_signal_introspection, buf);
+ for (signal = interface->signals; signal && signal->name; signal++)
+ _dbus_signal_introspection(signal, buf);
l_queue_foreach(interface->properties,
(l_queue_foreach_func_t) _dbus_property_introspection, buf);
l_string_append(buf, "\t</interface>\n");
}
-static char *copy_params(char *dest, const char *signature, va_list args)
-{
- const char *pname;
- const char *sig;
-
- for (sig = signature; *sig; sig++) {
- sig = _dbus_signature_end(sig);
- if (!sig)
- return NULL;
-
- pname = va_arg(args, const char *);
- dest = stpcpy(dest, pname) + 1;
- }
-
- return dest;
-}
-
-static bool size_params(const char *signature, va_list args, unsigned int *len)
-{
- const char *pname;
- const char *sig;
-
- for (sig = signature; *sig; sig++) {
- sig = _dbus_signature_end(sig);
- if (!sig)
- return false;
-
- pname = va_arg(args, const char *);
- *len += strlen(pname) + 1;
- }
-
- return true;
-}
-
bool _dbus_interface_methods(struct l_dbus_interface *interface,
const struct l_dbus_method *methods)
{
@@ -250,49 +193,16 @@ bool _dbus_interface_methods(struct l_dbus_interface *interface,
return true;
}
-LIB_EXPORT bool l_dbus_interface_signal(struct l_dbus_interface *interface,
- const char *name, uint32_t flags,
- const char *signature, ...)
+bool _dbus_interface_signals(struct l_dbus_interface *interface,
+ const struct l_dbus_signal *signals)
{
- va_list args;
- unsigned int metainfo_len;
- struct _dbus_signal *info;
- char *p;
-
- if (!_dbus_valid_method(name))
- return false;
-
- if (unlikely(!signature))
- return false;
-
- if (signature[0] && !_dbus_valid_signature(signature))
+ if (unlikely(!interface))
return false;
- /* Pre-calculate the needed meta-info length */
- metainfo_len = strlen(name) + 1;
- metainfo_len += strlen(signature) + 1;
-
- va_start(args, signature);
-
- if (!size_params(signature, args, &metainfo_len)) {
- va_end(args);
+ if (unlikely(!signals))
return false;
- }
- va_end(args);
-
- info = l_malloc(sizeof(*info) + metainfo_len);
- info->flags = flags;
- info->name_len = strlen(name);
-
- p = stpcpy(info->metainfo, name) + 1;
-
- va_start(args, signature);
- p = stpcpy(p, signature) + 1;
- p = copy_params(p, signature, args);
- va_end(args);
-
- l_queue_push_tail(interface->signals, info);
+ interface->signals = signals;
return true;
}
@@ -352,7 +262,6 @@ struct l_dbus_interface *_dbus_interface_new(const char *name)
interface = l_malloc(sizeof(*interface) + strlen(name) + 1);
- interface->signals = l_queue_new();
interface->properties = l_queue_new();
strcpy(interface->name, name);
@@ -362,7 +271,6 @@ struct l_dbus_interface *_dbus_interface_new(const char *name)
void _dbus_interface_free(struct l_dbus_interface *interface)
{
- l_queue_destroy(interface->signals, l_free);
l_queue_destroy(interface->properties, l_free);
l_free(interface);
@@ -381,21 +289,17 @@ const struct l_dbus_method *_dbus_interface_find_method(struct
l_dbus_interface
return NULL;
}
-static bool match_signal(const void *a, const void *b)
+const struct l_dbus_signal *_dbus_interface_find_signal(struct l_dbus_interface *i,
+ const char *signal)
{
- const struct _dbus_signal *signal = a;
- const char *name = b;
+ const struct l_dbus_signal *s;
- if (!strcmp(signal->metainfo, name))
- return true;
-
- return false;
-}
+ for (s = i->signals; s && s->name; s++) {
+ if (!strcmp(s->name, signal))
+ return s;
+ }
-struct _dbus_signal *_dbus_interface_find_signal(struct l_dbus_interface *i,
- const char *signal)
-{
- return l_queue_find(i->signals, match_signal, (char *) signal);
+ return NULL;
}
static bool match_property(const void *a, const void *b)
@@ -586,6 +490,7 @@ void _dbus_object_tree_prune_node(struct object_node *node)
bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
const char *path, const char *interface,
const struct l_dbus_method *methods,
+ const struct l_dbus_signal *signals,
void (*setup_func)(struct l_dbus_interface *),
void *user_data, void (*destroy) (void *))
{
@@ -624,6 +529,7 @@ bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
if (!dbi) {
dbi = _dbus_interface_new(interface);
dbi->methods = methods;
+ dbi->signals = signals;
setup_func(dbi);
l_hashmap_insert(tree->interfaces, dbi->name, dbi);
}
@@ -789,6 +695,7 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
const char *path, const char *interface,
const struct l_dbus_method *methods,
+ const struct l_dbus_signal *signals,
l_dbus_interface_setup_func_t setup_func,
void *user_data,
l_dbus_destroy_func_t destroy)
@@ -802,7 +709,8 @@ LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
if (unlikely(!tree))
return false;
- return _dbus_object_tree_register(tree, path, interface, methods,
+ return _dbus_object_tree_register(tree, path, interface,
+ methods, signals,
setup_func, user_data, destroy);
}
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index f3dceae..880e27c 100644
--- a/ell/dbus-service.h
+++ b/ell/dbus-service.h
@@ -67,6 +67,12 @@ struct l_dbus_method {
const struct l_dbus_argument *out_args;
};
+struct l_dbus_signal {
+ const char *name;
+ enum l_dbus_signal_flag flags;
+ const struct l_dbus_argument *args;
+};
+
#define L_DBUS_ARGS(args...) (const struct l_dbus_argument[]) { args, { } }
#define L_DBUS_METHOD(_name, _in_args, _out_args, _cb) \
@@ -103,10 +109,19 @@ struct l_dbus_method {
.cb = _cb, \
.flags = L_DBUS_METHOD_FLAG_DEPRECATED | L_DBUS_METHOD_FLAG_ASYNC
+#define L_DBUS_SIGNAL(_name, _args) \
+ .name = _name, \
+ .args = _args
+
+#define L_DBUS_DEPRECATED_SIGNAL(_name, _args) \
+ .name = _name, \
+ .args = _args, \
+ .flags = L_DBUS_SIGNAL_FLAG_DEPRECATED
bool l_dbus_register_interface(struct l_dbus *dbus,
const char *path, const char *interface,
const struct l_dbus_method *methods,
+ const struct l_dbus_signal *signals,
l_dbus_interface_setup_func_t setup_func,
void *user_data,
l_dbus_destroy_func_t destroy);
@@ -114,10 +129,6 @@ bool l_dbus_register_interface(struct l_dbus *dbus,
bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
const char *interface);
-bool l_dbus_interface_signal(struct l_dbus_interface *interface,
- const char *name, uint32_t flags,
- const char *signature, ...);
-
bool l_dbus_interface_property(struct l_dbus_interface *interface,
const char *name, uint32_t flags,
const char *signature);
--
2.0.4
10:36 a.m.
New subject: [PATCH 08/21] unit: Apply the DBus interface signals API change to the tests
Use constant signals array instead of function calls.
---
unit/test-dbus-service.c | 37 ++++++++++++++++++++++---------------
unit/test-kdbus.c | 3 ++-
2 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/unit/test-dbus-service.c b/unit/test-dbus-service.c
index 6e77e78..25b0951 100644
--- a/unit/test-dbus-service.c
+++ b/unit/test-dbus-service.c
@@ -125,7 +125,7 @@ static void test_introspect_method(const void *test_data)
static void test_introspect_signal(const void *test_data)
{
const struct introspect_test *test = test_data;
- struct _dbus_signal *signal;
+ const struct l_dbus_signal *signal;
struct l_string *buf;
char *xml;
@@ -236,14 +236,6 @@ static struct l_dbus_message *get_modems_callback(struct l_dbus
*dbus,
return NULL;
}
-static void build_manager_interface(struct l_dbus_interface *iface)
-{
- l_dbus_interface_signal(iface, "ModemAdded", 0,
- "oa{sv}", "path", "properties");
- l_dbus_interface_signal(iface, "ModemRemoved", 0,
- "o", "path");
-}
-
static const char *ofono_manager_introspection =
"<!DOCTYPE node PUBLIC \""
"-//freedesktop//DTD D-BUS Object Introspection 1.0//EN\"\n"
@@ -261,7 +253,8 @@ static const char *ofono_manager_introspection =
"\t\t\t<arg name=\"path\" type=\"o\"/>\n"
"\t\t\t<arg name=\"properties\" type=\"a{sv}\"/>\n"
"\t\t</signal>\n"
- "\t\t<signal
name=\"ModemRemoved\">\n" "\t\t\t<arg
name=\"path\" type=\"o\"/>\n"
+ "\t\t<signal name=\"ModemRemoved\">\n"
+ "\t\t\t<arg name=\"path\" type=\"o\"/>\n"
"\t\t</signal>\n\t</interface>\n"
"\t<node name=\"phonesim\"/>\n"
"</node>\n";
@@ -273,6 +266,14 @@ static const struct l_dbus_method ofono_methods[] = {
{ },
};
+static const struct l_dbus_signal ofono_signals[] = {
+ { L_DBUS_SIGNAL("ModemAdded",
+ L_DBUS_ARGS({ "path", "o" }, { "properties",
"a{sv}" })) },
+ { L_DBUS_SIGNAL("ModemRemoved",
+ L_DBUS_ARGS({ "path", "o" })) },
+ { },
+};
+
static void test_dbus_object_tree_introspection(const void *test_data)
{
struct _dbus_object_tree *tree;
@@ -282,8 +283,8 @@ static void test_dbus_object_tree_introspection(const void
*test_data)
tree = _dbus_object_tree_new();
_dbus_object_tree_register(tree, "/", "org.ofono.Manager",
- ofono_methods, build_manager_interface,
- NULL, NULL);
+ ofono_methods, ofono_signals,
+ NULL, NULL, NULL);
_dbus_object_tree_makepath(tree, "/phonesim");
@@ -304,8 +305,8 @@ static void test_dbus_object_tree_dispatch(const void *test_data)
tree = _dbus_object_tree_new();
_dbus_object_tree_register(tree, "/", "org.ofono.Manager",
- ofono_methods, build_manager_interface,
- dummy_data, NULL);
+ ofono_methods, ofono_signals,
+ NULL, dummy_data, NULL);
message = _dbus_message_new_method_call(1, "org.ofono", "/",
"org.ofono.Manager",
@@ -335,6 +336,12 @@ static const struct l_dbus_method sample_methods[] = {
{ },
};
+static const struct l_dbus_signal sample_signals[] = {
+ { L_DBUS_SIGNAL("Changed",
+ L_DBUS_ARGS({ "new_value", "b" })) },
+ { },
+};
+
int main(int argc, char *argv[])
{
int ret;
@@ -345,7 +352,7 @@ int main(int argc, char *argv[])
_dbus_interface_methods(interface, sample_methods);
- l_dbus_interface_signal(interface, "Changed", 0, "b",
"new_value");
+ _dbus_interface_signals(interface, sample_signals);
l_dbus_interface_rw_property(interface, "Bar", "y");
diff --git a/unit/test-kdbus.c b/unit/test-kdbus.c
index 817a39f..9470f75 100644
--- a/unit/test-kdbus.c
+++ b/unit/test-kdbus.c
@@ -159,7 +159,8 @@ int main(int argc, char *argv[])
service, NULL);
if (!l_dbus_register_interface(service, "/test", "org.test",
- test_methods, NULL, NULL, NULL)) {
+ test_methods, NULL,
+ NULL, NULL, NULL)) {
l_info("Unable to register interface");
goto error;
}
--
2.0.4
10:36 a.m.
New subject: [PATCH 09/21] examples: Apply the DBus interface signals API change
Use constant signals array instead of function calls.
---
examples/dbus-service.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index 5630cad..cb9a308 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -189,9 +189,6 @@ static struct l_dbus_message *test_method_call(struct l_dbus *dbus,
static void setup_test_interface(struct l_dbus_interface *interface)
{
- l_dbus_interface_signal(interface, "PropertyChanged", 0,
- "sv", "name", "value");
-
l_dbus_interface_rw_property(interface, "String", "s");
l_dbus_interface_rw_property(interface, "Integer", "u");
}
@@ -208,6 +205,12 @@ static const struct l_dbus_method test_methods[] = {
{ },
};
+static const struct l_dbus_signal test_signals[] = {
+ { L_DBUS_SIGNAL("PropertyChanged",
+ L_DBUS_ARGS({ "name", "s" }, { "value", "v" }))
},
+ { },
+};
+
int main(int argc, char *argv[])
{
struct l_dbus *dbus;
@@ -238,7 +241,8 @@ int main(int argc, char *argv[])
test->integer = 42;
if (!l_dbus_register_interface(dbus, "/test", "org.test",
- test_methods, setup_test_interface,
+ test_methods, test_signals,
+ setup_test_interface,
test, test_data_destroy)) {
l_info("Unable to register interface");
test_data_destroy(test);
--
2.0.4
10:36 a.m.
New subject: [PATCH 10/21] dbus: Refactor the interface API so it uses constant properties array
This makes ELL DBus interface API more in line with what exists in
GDBus.
---
ell/dbus-private.h | 12 ++++---
ell/dbus-service.c | 104 ++++++++++++++---------------------------------------
ell/dbus-service.h | 39 +++++++++++++-------
3 files changed, 61 insertions(+), 94 deletions(-)
diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 5248dd6..6ead02d 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -61,7 +61,7 @@ struct l_dbus_interface;
struct l_dbus_argument;
struct l_dbus_method;
struct l_dbus_signal;
-struct _dbus_property;
+struct l_dbus_property;
struct l_dbus_message_iter;
struct l_dbus_message;
struct l_dbus;
@@ -148,7 +148,7 @@ void _dbus_method_introspection(const struct l_dbus_method *info,
struct l_string *buf);
void _dbus_signal_introspection(const struct l_dbus_signal *info,
struct l_string *buf);
-void _dbus_property_introspection(struct _dbus_property *info,
+void _dbus_property_introspection(const struct l_dbus_property *info,
struct l_string *buf);
void _dbus_interface_introspection(struct l_dbus_interface *interface,
struct l_string *buf);
@@ -160,13 +160,15 @@ bool _dbus_interface_methods(struct l_dbus_interface *interface,
const struct l_dbus_method *methods);
bool _dbus_interface_signals(struct l_dbus_interface *interface,
const struct l_dbus_signal *signals);
+bool _dbus_interface_properties(struct l_dbus_interface *interface,
+ const struct l_dbus_property *properties);
const struct l_dbus_method *_dbus_interface_find_method(struct l_dbus_interface *i,
const char *method);
const struct l_dbus_signal *_dbus_interface_find_signal(struct l_dbus_interface *i,
const char *signal);
-struct _dbus_property *_dbus_interface_find_property(struct l_dbus_interface *i,
- const char *property);
+const struct l_dbus_property *_dbus_interface_find_property(struct l_dbus_interface *i,
+ const char *property);
struct _dbus_object_tree *_dbus_object_tree_new();
void _dbus_object_tree_free(struct _dbus_object_tree *tree);
@@ -181,7 +183,7 @@ bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
const char *path, const char *interface,
const struct l_dbus_method *methods,
const struct l_dbus_signal *signals,
- void (*setup_func)(struct l_dbus_interface *),
+ const struct l_dbus_property *properties,
void *user_data, void (*destroy) (void *));
bool _dbus_object_tree_unregister(struct _dbus_object_tree *tree,
const char *path,
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 8996407..8dc394a 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -47,16 +47,10 @@ static const char *static_introspectable =
"\t\t\t<arg name=\"xml\" type=\"s\"
direction=\"out\"/>\n"
"\t\t</method>\n\t</interface>\n";
-struct _dbus_property {
- uint32_t flags;
- unsigned char name_len;
- char metainfo[];
-};
-
struct l_dbus_interface {
const struct l_dbus_method *methods;
const struct l_dbus_signal *signals;
- struct l_queue *properties;
+ const struct l_dbus_property *properties;
char name[];
};
@@ -136,21 +130,18 @@ void _dbus_signal_introspection(const struct l_dbus_signal *info,
l_string_append(buf, "\t\t</signal>\n");
}
-void _dbus_property_introspection(struct _dbus_property *info,
+void _dbus_property_introspection(const struct l_dbus_property *info,
struct l_string *buf)
{
- unsigned int offset = info->name_len + 1;
- const char *signature = info->metainfo + offset;
-
l_string_append_printf(buf, "\t\t<property name=\"%s\"
type=\"%s\" ",
- info->metainfo, signature);
+ info->name, info->type);
if (info->flags & L_DBUS_PROPERTY_FLAG_WRITABLE)
l_string_append(buf, "access=\"readwrite\"");
else
l_string_append(buf, "access=\"read\"");
- if (info->flags & L_DBUS_METHOD_FLAG_DEPRECATED) {
+ if (info->flags & L_DBUS_PROPERTY_FLAG_DEPRECATED) {
l_string_append(buf, ">\n");
l_string_append(buf, "\t\t\t<annotation name=\""
"org.freedesktop.DBus.Deprecated\" "
@@ -165,6 +156,7 @@ void _dbus_interface_introspection(struct l_dbus_interface
*interface,
{
const struct l_dbus_method *method;
const struct l_dbus_signal *signal;
+ const struct l_dbus_property *property;
l_string_append_printf(buf, "\t<interface name=\"%s\">\n",
interface->name);
@@ -173,8 +165,9 @@ void _dbus_interface_introspection(struct l_dbus_interface
*interface,
_dbus_method_introspection(method, buf);
for (signal = interface->signals; signal && signal->name; signal++)
_dbus_signal_introspection(signal, buf);
- l_queue_foreach(interface->properties,
- (l_queue_foreach_func_t) _dbus_property_introspection, buf);
+ for (property = interface->properties;
+ property && property->name; property++)
+ _dbus_property_introspection(property, buf);
l_string_append(buf, "\t</interface>\n");
}
@@ -207,63 +200,26 @@ bool _dbus_interface_signals(struct l_dbus_interface *interface,
return true;
}
-LIB_EXPORT bool l_dbus_interface_property(struct l_dbus_interface *interface,
- const char *name, uint32_t flags,
- const char *signature)
+bool _dbus_interface_properties(struct l_dbus_interface *interface,
+ const struct l_dbus_property *properties)
{
- unsigned int metainfo_len;
- struct _dbus_property *info;
- char *p;
-
- if (!_dbus_valid_method(name))
- return false;
-
- if (unlikely(!signature))
+ if (unlikely(!interface))
return false;
- if (!_dbus_valid_signature(signature))
+ if (unlikely(!properties))
return false;
- /* Pre-calculate the needed meta-info length */
- metainfo_len = strlen(name) + 1;
- metainfo_len += strlen(signature) + 1;
-
- info = l_malloc(sizeof(*info) + metainfo_len);
- info->flags = flags;
- info->name_len = strlen(name);
-
- p = stpcpy(info->metainfo, name) + 1;
- strcpy(p, signature);
-
- l_queue_push_tail(interface->properties, info);
+ interface->properties = properties;
return true;
}
-LIB_EXPORT bool l_dbus_interface_ro_property(struct l_dbus_interface *interface,
- const char *name,
- const char *signature)
-{
- return l_dbus_interface_property(interface, name, 0, signature);
-}
-
-LIB_EXPORT bool l_dbus_interface_rw_property(struct l_dbus_interface *interface,
- const char *name,
- const char *signature)
-{
- return l_dbus_interface_property(interface, name,
- L_DBUS_PROPERTY_FLAG_WRITABLE,
- signature);
-}
-
struct l_dbus_interface *_dbus_interface_new(const char *name)
{
struct l_dbus_interface *interface;
interface = l_malloc(sizeof(*interface) + strlen(name) + 1);
- interface->properties = l_queue_new();
-
strcpy(interface->name, name);
return interface;
@@ -271,8 +227,6 @@ struct l_dbus_interface *_dbus_interface_new(const char *name)
void _dbus_interface_free(struct l_dbus_interface *interface)
{
- l_queue_destroy(interface->properties, l_free);
-
l_free(interface);
}
@@ -302,21 +256,17 @@ const struct l_dbus_signal *_dbus_interface_find_signal(struct
l_dbus_interface
return NULL;
}
-static bool match_property(const void *a, const void *b)
+const struct l_dbus_property *_dbus_interface_find_property(struct l_dbus_interface *i,
+ const char *property)
{
- const struct _dbus_property *property = a;
- const char *name = b;
-
- if (!strcmp(property->metainfo, name))
- return true;
+ const struct l_dbus_property *p;
- return false;
-}
+ for (p = i->properties; p && p->name; p++) {
+ if (!strcmp(p->name, property))
+ return p;
+ }
-struct _dbus_property *_dbus_interface_find_property(struct l_dbus_interface *i,
- const char *property)
-{
- return l_queue_find(i->properties, match_property, (char *) property);
+ return NULL;
}
static void interface_instance_free(struct interface_instance *instance)
@@ -491,7 +441,7 @@ bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
const char *path, const char *interface,
const struct l_dbus_method *methods,
const struct l_dbus_signal *signals,
- void (*setup_func)(struct l_dbus_interface *),
+ const struct l_dbus_property *properties,
void *user_data, void (*destroy) (void *))
{
struct object_node *object;
@@ -530,7 +480,7 @@ bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
dbi = _dbus_interface_new(interface);
dbi->methods = methods;
dbi->signals = signals;
- setup_func(dbi);
+ dbi->properties = properties;
l_hashmap_insert(tree->interfaces, dbi->name, dbi);
}
@@ -696,7 +646,7 @@ LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
const char *path, const char *interface,
const struct l_dbus_method *methods,
const struct l_dbus_signal *signals,
- l_dbus_interface_setup_func_t setup_func,
+ const struct l_dbus_property *properties,
void *user_data,
l_dbus_destroy_func_t destroy)
{
@@ -709,9 +659,9 @@ LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
if (unlikely(!tree))
return false;
- return _dbus_object_tree_register(tree, path, interface,
- methods, signals,
- setup_func, user_data, destroy);
+ return _dbus_object_tree_register(tree, path, interface, methods,
+ signals, properties,
+ user_data, destroy);
}
LIB_EXPORT bool l_dbus_unregister_interface(struct l_dbus *dbus,
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index 880e27c..1a210f0 100644
--- a/ell/dbus-service.h
+++ b/ell/dbus-service.h
@@ -49,7 +49,6 @@ enum l_dbus_property_flag {
L_DBUS_PROPERTY_FLAG_WRITABLE = 2,
};
-typedef void (*l_dbus_interface_setup_func_t) (struct l_dbus_interface *);
typedef struct l_dbus_message *(*l_dbus_interface_method_cb_t) (struct l_dbus *,
struct l_dbus_message *message,
void *user_data);
@@ -73,6 +72,12 @@ struct l_dbus_signal {
const struct l_dbus_argument *args;
};
+struct l_dbus_property {
+ const char *name;
+ const char *type;
+ enum l_dbus_property_flag flags;
+};
+
#define L_DBUS_ARGS(args...) (const struct l_dbus_argument[]) { args, { } }
#define L_DBUS_METHOD(_name, _in_args, _out_args, _cb) \
@@ -118,27 +123,37 @@ struct l_dbus_signal {
.args = _args, \
.flags = L_DBUS_SIGNAL_FLAG_DEPRECATED
+#define L_DBUS_RO_PROPERTY(_name, _type) \
+ .name = _name, \
+ .type = _type
+
+#define L_DBUS_RW_PROPERTY(_name, _type) \
+ .name = _name, \
+ .type = _type, \
+ .flags = L_DBUS_PROPERTY_FLAG_WRITABLE
+
+#define L_DBUS_RO_DEPRECATED_PROPERTY(_name, _type) \
+ .name = _name, \
+ .type = _type, \
+ .flags = L_DBUS_PROPERTY_FLAG_DEPRECATED
+
+#define L_DBUS_RW_DEPRECATED_PROPERTY(_name, _type) \
+ .name = _name, \
+ .type = _type, \
+ .flags = L_DBUS_PROPERTY_FLAG_WRITABLE | \
+ L_DBUS_PROPERTY_FLAG_DEPRECATED
+
bool l_dbus_register_interface(struct l_dbus *dbus,
const char *path, const char *interface,
const struct l_dbus_method *methods,
const struct l_dbus_signal *signals,
- l_dbus_interface_setup_func_t setup_func,
+ const struct l_dbus_property *properties,
void *user_data,
l_dbus_destroy_func_t destroy);
bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
const char *interface);
-bool l_dbus_interface_property(struct l_dbus_interface *interface,
- const char *name, uint32_t flags,
- const char *signature);
-bool l_dbus_interface_ro_property(struct l_dbus_interface *interface,
- const char *name,
- const char *signature);
-bool l_dbus_interface_rw_property(struct l_dbus_interface *interface,
- const char *name,
- const char *signature);
-
#ifdef __cplusplus
}
#endif
--
2.0.4
10:36 a.m.
New subject: [PATCH 11/21] unit: Apply the DBus interface properties API change to the tests
Use constant properties array instead of function calls.
---
unit/test-dbus-service.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/unit/test-dbus-service.c b/unit/test-dbus-service.c
index 25b0951..825a159 100644
--- a/unit/test-dbus-service.c
+++ b/unit/test-dbus-service.c
@@ -143,7 +143,7 @@ static void test_introspect_signal(const void *test_data)
static void test_introspect_property(const void *test_data)
{
const struct introspect_test *test = test_data;
- struct _dbus_property *property;
+ const struct l_dbus_property *property;
struct l_string *buf;
char *xml;
@@ -342,6 +342,11 @@ static const struct l_dbus_signal sample_signals[] = {
{ },
};
+static const struct l_dbus_property sample_properties[] = {
+ { L_DBUS_RW_PROPERTY("Bar", "y") },
+ { },
+};
+
int main(int argc, char *argv[])
{
int ret;
@@ -354,7 +359,7 @@ int main(int argc, char *argv[])
_dbus_interface_signals(interface, sample_signals);
- l_dbus_interface_rw_property(interface, "Bar", "y");
+ _dbus_interface_properties(interface, sample_properties);
l_test_add("Test Frobate Introspection", test_introspect_method,
&frobate_test);
--
2.0.4
10:36 a.m.
New subject: [PATCH 12/21] examples: Apply the DBus interface properties API change
Use constant properties array instead of function calls.
---
examples/dbus-service.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index cb9a308..06cf1d9 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -187,12 +187,6 @@ static struct l_dbus_message *test_method_call(struct l_dbus *dbus,
return reply;
}
-static void setup_test_interface(struct l_dbus_interface *interface)
-{
- l_dbus_interface_rw_property(interface, "String", "s");
- l_dbus_interface_rw_property(interface, "Integer", "u");
-}
-
static const struct l_dbus_method test_methods[] = {
{ L_DBUS_METHOD("GetProperties",
L_DBUS_ARGS({ "properties", "a{sv}" }), NULL,
@@ -211,6 +205,12 @@ static const struct l_dbus_signal test_signals[] = {
{ },
};
+static const struct l_dbus_property test_properties[] = {
+ { L_DBUS_RW_PROPERTY("String", "s") },
+ { L_DBUS_RW_PROPERTY("Integer", "u") },
+ { },
+};
+
int main(int argc, char *argv[])
{
struct l_dbus *dbus;
@@ -242,8 +242,8 @@ int main(int argc, char *argv[])
if (!l_dbus_register_interface(dbus, "/test", "org.test",
test_methods, test_signals,
- setup_test_interface,
- test, test_data_destroy)) {
+ test_properties, test,
+ test_data_destroy)) {
l_info("Unable to register interface");
test_data_destroy(test);
goto cleanup;
--
2.0.4
10:36 a.m.
New subject: [PATCH 13/21] dbus: Add support for org.freedesktop.DBus.Properties.Get method
First part of the org.freedesktop.DBus.Properties standard interface
support.
---
ell/dbus-private.h | 5 +++
ell/dbus-service.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++-----
ell/dbus-service.h | 20 ++++++---
ell/dbus.c | 1 -
4 files changed, 126 insertions(+), 17 deletions(-)
diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 6ead02d..0694a12 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -36,6 +36,8 @@ enum dbus_container_type {
DBUS_CONTAINER_TYPE_DICT_ENTRY = 'e',
};
+#define DBUS_ERROR_INVALID_ARGS "org.freedesktop.DBus.Error.InvalidArgs"
+
#if __BYTE_ORDER == __LITTLE_ENDIAN
#define DBUS_NATIVE_ENDIAN 'l'
#elif __BYTE_ORDER == __BIG_ENDIAN
@@ -178,6 +180,9 @@ struct object_node *_dbus_object_tree_makepath(struct
_dbus_object_tree *tree,
struct object_node *_dbus_object_tree_lookup(struct _dbus_object_tree *tree,
const char *path);
void _dbus_object_tree_prune_node(struct object_node *node);
+struct interface_instance *_dbus_object_find_instance(
+ struct object_node *object,
+ const char *interface);
bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
const char *path, const char *interface,
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 8dc394a..c745972 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -37,6 +37,8 @@
#include "dbus-private.h"
#include "private.h"
+#define DBUS_INTERFACE_PROPERTIES "org.freedesktop.DBus.Properties"
+
#define XML_ID "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
#define XML_DTD "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd"
#define XML_HEAD "<!DOCTYPE node PUBLIC
\""XML_ID"\"\n\""XML_DTD"\">\n"
@@ -437,6 +439,95 @@ void _dbus_object_tree_prune_node(struct object_node *node)
}
}
+struct interface_instance *_dbus_object_find_instance(
+ struct object_node *object,
+ const char *interface)
+{
+ const struct l_queue_entry *entry;
+
+ entry = l_queue_get_entries(object->instances);
+ while (entry) {
+ struct interface_instance *instance = entry->data;
+
+ if (!strcmp(instance->interface->name, interface))
+ return instance;
+
+ entry = entry->next;
+ }
+
+ return NULL;
+}
+
+static struct l_dbus_message *properties_get(struct l_dbus *dbus,
+ struct l_dbus_message *message,
+ void *user_data)
+{
+ const char *interface;
+ const char *name;
+ const char *path;
+ struct _dbus_object_tree *tree;
+ struct object_node *object;
+ struct interface_instance *instance;
+ const struct l_dbus_property *property;
+ struct l_dbus_message *reply;
+ struct l_dbus_message_builder *builder;
+
+ if (!l_dbus_message_get_arguments(message, "ss", &interface, &name))
+ return NULL;
+
+ path = l_dbus_message_get_path(message);
+ tree = _dbus_object_tree_get_from_dbus(dbus);
+
+ object = l_hashmap_lookup(tree->objects, path);
+ if (!object)
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_ARGS,
+ "No such object '%s'", path);
+
+ instance = _dbus_object_find_instance(object, interface);
+ if (!instance)
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_ARGS,
+ "No such interface '%s'", interface);
+
+ property = _dbus_interface_find_property(instance->interface, name);
+ if (!property)
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_ARGS,
+ "No such property '%s'", name);
+ if (property->get == NULL)
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_ARGS,
+ "Property '%s' is not readable", name);
+
+ reply = l_dbus_message_new_method_return(message);
+ if (!reply)
+ return NULL;
+
+ builder = l_dbus_message_builder_new(reply);
+ l_dbus_message_builder_enter_variant(builder, property->type);
+
+ if (!property->get(property, builder, instance->user_data)) {
+ l_dbus_message_builder_destroy(builder);
+ l_dbus_message_unref(reply);
+ return NULL;
+ }
+
+ l_dbus_message_builder_leave_variant(builder);
+ l_dbus_message_builder_finalize(builder);
+ l_dbus_message_builder_destroy(builder);
+
+ return reply;
+}
+
+static const struct l_dbus_method properties_methods[] = {
+ { L_DBUS_METHOD("Get",
+ L_DBUS_ARGS({ "interface", "s" }, { "name",
"s" }),
+ L_DBUS_ARGS({ "value", "v" }),
+ properties_get) },
+ { }
+};
+
bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
const char *path, const char *interface,
const struct l_dbus_method *methods,
@@ -446,7 +537,6 @@ bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
{
struct object_node *object;
struct l_dbus_interface *dbi;
- const struct l_queue_entry *entry;
struct interface_instance *instance;
if (!_dbus_valid_interface(interface))
@@ -465,15 +555,8 @@ bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
* Check to make sure we do not have this interface already
* registered for this object
*/
- entry = l_queue_get_entries(object->instances);
- while (entry) {
- instance = entry->data;
-
- if (!strcmp(instance->interface->name, interface))
- return false;
-
- entry = entry->next;
- }
+ if (_dbus_object_find_instance(object, interface))
+ return false;
dbi = l_hashmap_lookup(tree->interfaces, interface);
if (!dbi) {
@@ -494,6 +577,13 @@ bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
l_queue_push_tail(object->instances, instance);
+ if (properties) {
+ _dbus_object_tree_register(tree, path,
+ DBUS_INTERFACE_PROPERTIES,
+ properties_methods,
+ NULL, NULL, instance, NULL);
+ }
+
return true;
}
@@ -514,8 +604,13 @@ bool _dbus_object_tree_unregister(struct _dbus_object_tree *tree,
r = instance ? true : false;
- if (instance)
+ if (instance) {
+ if (instance->interface->properties)
+ _dbus_object_tree_unregister(tree, path,
+ DBUS_INTERFACE_PROPERTIES);
+
interface_instance_free(instance);
+ }
if (l_queue_isempty(node->instances) && !node->children) {
l_hashmap_remove(tree->objects, path);
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index 1a210f0..efda8b8 100644
--- a/ell/dbus-service.h
+++ b/ell/dbus-service.h
@@ -49,9 +49,14 @@ enum l_dbus_property_flag {
L_DBUS_PROPERTY_FLAG_WRITABLE = 2,
};
+struct l_dbus_property;
+
typedef struct l_dbus_message *(*l_dbus_interface_method_cb_t) (struct l_dbus *,
struct l_dbus_message *message,
void *user_data);
+typedef bool (*l_dbus_property_getter) (const struct l_dbus_property *property,
+ struct l_dbus_message_builder *builder,
+ void *user_data);
struct l_dbus_argument {
const char *name;
@@ -75,6 +80,7 @@ struct l_dbus_signal {
struct l_dbus_property {
const char *name;
const char *type;
+ l_dbus_property_getter get;
enum l_dbus_property_flag flags;
};
@@ -123,23 +129,27 @@ struct l_dbus_property {
.args = _args, \
.flags = L_DBUS_SIGNAL_FLAG_DEPRECATED
-#define L_DBUS_RO_PROPERTY(_name, _type) \
+#define L_DBUS_RO_PROPERTY(_name, _type, _get) \
.name = _name, \
- .type = _type
+ .type = _type, \
+ .get = _get
-#define L_DBUS_RW_PROPERTY(_name, _type) \
+#define L_DBUS_RW_PROPERTY(_name, _type, _get) \
.name = _name, \
.type = _type, \
+ .get = _get, \
.flags = L_DBUS_PROPERTY_FLAG_WRITABLE
-#define L_DBUS_RO_DEPRECATED_PROPERTY(_name, _type) \
+#define L_DBUS_RO_DEPRECATED_PROPERTY(_name, _type, _get) \
.name = _name, \
.type = _type, \
+ .get = _get, \
.flags = L_DBUS_PROPERTY_FLAG_DEPRECATED
-#define L_DBUS_RW_DEPRECATED_PROPERTY(_name, _type) \
+#define L_DBUS_RW_DEPRECATED_PROPERTY(_name, _type, _get) \
.name = _name, \
.type = _type, \
+ .get = _get, \
.flags = L_DBUS_PROPERTY_FLAG_WRITABLE | \
L_DBUS_PROPERTY_FLAG_DEPRECATED
diff --git a/ell/dbus.c b/ell/dbus.c
index ff7e814..2741912 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -51,7 +51,6 @@
#define DBUS_INTERFACE_DBUS "org.freedesktop.DBus"
#define DBUS_INTERFACE_INTROSPECTABLE "org.freedesktop.DBus.Introspectable"
-#define DBUS_INTERFACE_PROPERTIES "org.freedesktop.DBus.Properties"
enum auth_state {
WAITING_FOR_OK,
--
2.0.4
10:36 a.m.
New subject: [PATCH 14/21] dbus: Add support for org.freedesktop.DBus.Properties.GetAll method
It is thus possible to get the whole subset of properties at once.
---
ell/dbus-service.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index c745972..dbb52a6 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -520,11 +520,91 @@ static struct l_dbus_message *properties_get(struct l_dbus *dbus,
return reply;
}
+static void append_property(struct interface_instance *instance,
+ const struct l_dbus_property *property,
+ struct l_dbus_message_builder *builder)
+{
+ l_dbus_message_builder_enter_dict(builder, "sv");
+ l_dbus_message_builder_append_basic(builder, 's', property->name);
+ l_dbus_message_builder_enter_variant(builder, property->type);
+
+ property->get(property, builder, instance->user_data);
+
+ l_dbus_message_builder_leave_variant(builder);
+ l_dbus_message_builder_leave_dict(builder);
+}
+
+static void append_properties(struct interface_instance *instance,
+ struct l_dbus_message_builder *builder)
+{
+ const struct l_dbus_property *p;
+
+ l_dbus_message_builder_enter_array(builder, "{sv}");
+
+ for (p = instance->interface->properties; p && p->name; p++) {
+ if (!p->get)
+ continue;
+
+ append_property(instance, p, builder);
+ }
+
+ l_dbus_message_builder_leave_array(builder);
+}
+
+static struct l_dbus_message *properties_get_all(
+ struct l_dbus *dbus,
+ struct l_dbus_message *message,
+ void *user_data)
+{
+ const char *interface;
+ const char *path;
+ struct _dbus_object_tree *tree;
+ struct object_node *object;
+ struct interface_instance *instance;
+ struct l_dbus_message *reply;
+ struct l_dbus_message_builder *builder;
+
+ if (!l_dbus_message_get_arguments(message, "s", &interface))
+ return NULL;
+
+ path = l_dbus_message_get_path(message);
+ tree = _dbus_object_tree_get_from_dbus(dbus);
+
+ object = l_hashmap_lookup(tree->objects, path);
+ if (!object)
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_ARGS,
+ "No such object '%s'", path);
+
+ instance = _dbus_object_find_instance(object, interface);
+ if (!instance)
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_ARGS,
+ "No such interface '%s'", interface);
+
+ reply = l_dbus_message_new_method_return(message);
+ if (!reply)
+ return NULL;
+
+ builder = l_dbus_message_builder_new(reply);
+
+ append_properties(instance, builder);
+
+ l_dbus_message_builder_finalize(builder);
+ l_dbus_message_builder_destroy(builder);
+
+ return reply;
+}
+
static const struct l_dbus_method properties_methods[] = {
{ L_DBUS_METHOD("Get",
L_DBUS_ARGS({ "interface", "s" }, { "name",
"s" }),
L_DBUS_ARGS({ "value", "v" }),
properties_get) },
+ { L_DBUS_METHOD("GetAll",
+ L_DBUS_ARGS({ "interface", "s" }),
+ L_DBUS_ARGS({ "properties", "a{sv}" }),
+ properties_get_all) },
{ }
};
--
2.0.4
10:36 a.m.
New subject: [PATCH 15/21] dbus: Add support for org.freedesktop.DBus.Properties.Set method
It is thus possible to set a property following this freedesktop
standard interface.
---
ell/dbus-private.h | 4 ++
ell/dbus-service.c | 166 +++++++++++++++++++++++++++++++++++++++++++++++++++++
ell/dbus-service.h | 22 ++++++-
3 files changed, 190 insertions(+), 2 deletions(-)
diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 0694a12..cbdd01f 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -37,6 +37,10 @@ enum dbus_container_type {
};
#define DBUS_ERROR_INVALID_ARGS "org.freedesktop.DBus.Error.InvalidArgs"
+#define DBUS_ERROR_UNKNOWN_PROPERTY \
+ "org.freedesktop.DBus.Error.UnknownProperty"
+#define DBUS_ERROR_INVALID_SIGNATURE \
+ "org.freedesktop.DBus.Error.InvalidSignature"
#if __BYTE_ORDER == __LITTLE_ENDIAN
#define DBUS_NATIVE_ENDIAN 'l'
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index dbb52a6..d297e68 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -78,6 +78,14 @@ struct _dbus_object_tree {
struct l_hashmap *interfaces;
struct l_hashmap *objects;
struct object_node *root;
+ uint32_t next_property_set;
+ struct l_queue *pending_data;
+};
+
+struct _dbus_pending_data {
+ uint32_t id;
+ struct l_dbus *dbus;
+ struct l_dbus_message *message;
};
void _dbus_print_arguments(const struct l_dbus_argument *args,
@@ -305,6 +313,9 @@ struct _dbus_object_tree *_dbus_object_tree_new()
tree->root = l_new(struct object_node, 1);
+ tree->next_property_set = 1;
+ tree->pending_data = l_queue_new();
+
return tree;
}
@@ -326,6 +337,17 @@ static void subtree_free(struct object_node *node)
l_free(node);
}
+static void destroy_pending_data(void *data)
+{
+ struct _dbus_pending_data *pending_data = data;
+
+ if (!pending_data)
+ return;
+
+ l_dbus_message_unref(pending_data->message);
+ l_free(pending_data);
+}
+
void _dbus_object_tree_free(struct _dbus_object_tree *tree)
{
l_hashmap_destroy(tree->interfaces,
@@ -334,6 +356,9 @@ void _dbus_object_tree_free(struct _dbus_object_tree *tree)
subtree_free(tree->root);
+ if (tree->pending_data)
+ l_queue_destroy(tree->pending_data,
+ destroy_pending_data);
l_free(tree);
}
@@ -596,6 +621,66 @@ static struct l_dbus_message *properties_get_all(
return reply;
}
+static struct l_dbus_message *properties_set(struct l_dbus *dbus,
+ struct l_dbus_message *message,
+ void *user_data)
+{
+ const char *interface;
+ const char *name;
+ struct l_dbus_message_iter variant;
+ const char *path;
+ struct _dbus_object_tree *tree;
+ struct object_node *object;
+ struct interface_instance *instance;
+ const struct l_dbus_property *property;
+ struct _dbus_pending_data *pending_data;
+
+ if (!l_dbus_message_get_arguments(message, "ssv",
+ &interface, &name, &variant))
+ return NULL;
+
+ path = l_dbus_message_get_path(message);
+ tree = _dbus_object_tree_get_from_dbus(dbus);
+
+ object = l_hashmap_lookup(tree->objects, path);
+ if (!object)
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_ARGS,
+ "No such object '%s'", path);
+
+ instance = _dbus_object_find_instance(object, interface);
+ if (!instance)
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_ARGS,
+ "No such interface '%s'", interface);
+
+ property = _dbus_interface_find_property(instance->interface, name);
+ if (!property)
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_ARGS,
+ "No such property '%s'", name);
+ if (property->set == NULL)
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_ARGS,
+ "Property '%s' is not writable", name);
+
+ if (!l_dbus_message_iter_has_signature(&variant, property->type))
+ return l_dbus_message_new_error(message,
+ DBUS_ERROR_INVALID_SIGNATURE,
+ "Invalid signature for '%s'", name);
+
+ pending_data = l_new(struct _dbus_pending_data, 1);
+ pending_data->id = tree->next_property_set++;
+ pending_data->message = l_dbus_message_ref(message);
+
+ l_queue_push_tail(tree->pending_data, pending_data);
+
+ property->set(property, dbus, &variant,
+ pending_data->id, instance->user_data);
+
+ return NULL;
+}
+
static const struct l_dbus_method properties_methods[] = {
{ L_DBUS_METHOD("Get",
L_DBUS_ARGS({ "interface", "s" }, { "name",
"s" }),
@@ -605,6 +690,11 @@ static const struct l_dbus_method properties_methods[] = {
L_DBUS_ARGS({ "interface", "s" }),
L_DBUS_ARGS({ "properties", "a{sv}" }),
properties_get_all) },
+ { L_DBUS_ASYNC_METHOD("Set",
+ L_DBUS_ARGS({ "interface", "s" },
+ { "name", "s" }, { "value", "v" }),
+ NULL,
+ properties_set) },
{ }
};
@@ -854,3 +944,79 @@ LIB_EXPORT bool l_dbus_unregister_interface(struct l_dbus *dbus,
return _dbus_object_tree_unregister(tree, path, interface);
}
+
+static bool match_pending_data_by_id(const void *data, const void *user_data)
+{
+ const struct _dbus_pending_data *pending_data = data;
+ uint32_t id = L_PTR_TO_UINT(user_data);
+
+ if (pending_data->id == id)
+ return true;
+
+ return false;
+}
+
+LIB_EXPORT void l_dbus_pending_property_success(struct l_dbus *dbus,
+ uint32_t id)
+{
+ struct _dbus_object_tree *tree = _dbus_object_tree_get_from_dbus(dbus);
+ struct _dbus_pending_data *pending_data;
+ struct l_dbus_message *reply;
+
+ if (unlikely(!dbus))
+ return;
+
+ pending_data = l_queue_remove_if(tree->pending_data,
+ match_pending_data_by_id,
+ L_UINT_TO_PTR(id));
+ if (!pending_data)
+ return;
+
+ reply = l_dbus_message_new_method_return(pending_data->message);
+ if (reply) {
+ l_dbus_message_set_arguments(reply, "");
+ l_dbus_send(dbus, reply);
+ }
+
+ destroy_pending_data(pending_data);
+}
+
+LIB_EXPORT void l_dbus_pending_property_error_valist(struct l_dbus *dbus,
+ uint32_t id,
+ const char *name,
+ const char *format,
+ va_list args)
+{
+ struct _dbus_object_tree *tree = _dbus_object_tree_get_from_dbus(dbus);
+ struct _dbus_pending_data *pending_data;
+ struct l_dbus_message *error;
+
+ if (unlikely(!dbus))
+ return;
+
+ pending_data = l_queue_remove_if(tree->pending_data,
+ match_pending_data_by_id,
+ L_UINT_TO_PTR(id));
+ if (!pending_data)
+ return;
+
+ error = l_dbus_message_new_error_valist(pending_data->message,
+ name, format, args);
+ if (error)
+ l_dbus_send(dbus, error);
+
+ destroy_pending_data(pending_data);
+}
+
+LIB_EXPORT void l_dbus_pending_property_error(struct l_dbus *dbus, uint32_t id,
+ const char *name,
+ const char *format, ...)
+{
+ va_list args;
+
+ va_start(args, format);
+
+ l_dbus_pending_property_error_valist(dbus, id, name, format, args);
+
+ va_end(args);
+}
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index efda8b8..8e344f5 100644
--- a/ell/dbus-service.h
+++ b/ell/dbus-service.h
@@ -57,6 +57,10 @@ typedef struct l_dbus_message *(*l_dbus_interface_method_cb_t) (struct
l_dbus *,
typedef bool (*l_dbus_property_getter) (const struct l_dbus_property *property,
struct l_dbus_message_builder *builder,
void *user_data);
+typedef void (*l_dbus_property_setter) (const struct l_dbus_property *property,
+ struct l_dbus *dbus,
+ struct l_dbus_message_iter *iter,
+ uint32_t id, void *user_data);
struct l_dbus_argument {
const char *name;
@@ -81,6 +85,7 @@ struct l_dbus_property {
const char *name;
const char *type;
l_dbus_property_getter get;
+ l_dbus_property_setter set;
enum l_dbus_property_flag flags;
};
@@ -134,10 +139,11 @@ struct l_dbus_property {
.type = _type, \
.get = _get
-#define L_DBUS_RW_PROPERTY(_name, _type, _get) \
+#define L_DBUS_RW_PROPERTY(_name, _type, _get, _set) \
.name = _name, \
.type = _type, \
.get = _get, \
+ .set = _set, \
.flags = L_DBUS_PROPERTY_FLAG_WRITABLE
#define L_DBUS_RO_DEPRECATED_PROPERTY(_name, _type, _get) \
@@ -146,10 +152,11 @@ struct l_dbus_property {
.get = _get, \
.flags = L_DBUS_PROPERTY_FLAG_DEPRECATED
-#define L_DBUS_RW_DEPRECATED_PROPERTY(_name, _type, _get) \
+#define L_DBUS_RW_DEPRECATED_PROPERTY(_name, _type, _get, _set) \
.name = _name, \
.type = _type, \
.get = _get, \
+ .set = _set, \
.flags = L_DBUS_PROPERTY_FLAG_WRITABLE | \
L_DBUS_PROPERTY_FLAG_DEPRECATED
@@ -164,6 +171,17 @@ bool l_dbus_register_interface(struct l_dbus *dbus,
bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
const char *interface);
+void l_dbus_pending_property_success(struct l_dbus *dbus, uint32_t id);
+
+void l_dbus_pending_property_error_valist(struct l_dbus *dbus, uint32_t id,
+ const char *name,
+ const char *format,
+ va_list args);
+
+void l_dbus_pending_property_error(struct l_dbus *dbus, uint32_t id,
+ const char *name,
+ const char *format, ...);
+
#ifdef __cplusplus
}
#endif
--
2.0.4
10:36 a.m.
New subject: [PATCH 16/21] dbus: Add support for checking if a property exists
As part of the org.freedesktop.DBus.Properties interface, it is
mandatory to provide a mean to check if a property is available or not
for the methods Get, GetAll and Set.
---
ell/dbus-service.c | 9 +++++++--
ell/dbus-service.h | 17 ++++++++++++-----
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index d297e68..b0f0063 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -516,7 +516,8 @@ static struct l_dbus_message *properties_get(struct l_dbus *dbus,
"No such interface '%s'", interface);
property = _dbus_interface_find_property(instance->interface, name);
- if (!property)
+ if (!property || (property->exists &&
+ !property->exists(property, instance->user_data)))
return l_dbus_message_new_error(message,
DBUS_ERROR_INVALID_ARGS,
"No such property '%s'", name);
@@ -570,6 +571,9 @@ static void append_properties(struct interface_instance *instance,
if (!p->get)
continue;
+ if (p->exists && !p->exists(p, instance->user_data))
+ continue;
+
append_property(instance, p, builder);
}
@@ -655,7 +659,8 @@ static struct l_dbus_message *properties_set(struct l_dbus *dbus,
"No such interface '%s'", interface);
property = _dbus_interface_find_property(instance->interface, name);
- if (!property)
+ if (!property || (property->exists &&
+ !property->exists(property, instance->user_data)))
return l_dbus_message_new_error(message,
DBUS_ERROR_INVALID_ARGS,
"No such property '%s'", name);
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index 8e344f5..d5a9a8e 100644
--- a/ell/dbus-service.h
+++ b/ell/dbus-service.h
@@ -61,6 +61,8 @@ typedef void (*l_dbus_property_setter) (const struct l_dbus_property
*property,
struct l_dbus *dbus,
struct l_dbus_message_iter *iter,
uint32_t id, void *user_data);
+typedef bool (*l_dbus_property_exists) (const struct l_dbus_property *property,
+ void *user_data);
struct l_dbus_argument {
const char *name;
@@ -86,6 +88,7 @@ struct l_dbus_property {
const char *type;
l_dbus_property_getter get;
l_dbus_property_setter set;
+ l_dbus_property_exists exists;
enum l_dbus_property_flag flags;
};
@@ -134,29 +137,33 @@ struct l_dbus_property {
.args = _args, \
.flags = L_DBUS_SIGNAL_FLAG_DEPRECATED
-#define L_DBUS_RO_PROPERTY(_name, _type, _get) \
+#define L_DBUS_RO_PROPERTY(_name, _type, _get, _exists) \
.name = _name, \
.type = _type, \
- .get = _get
+ .get = _get, \
+ .exists = _exists
-#define L_DBUS_RW_PROPERTY(_name, _type, _get, _set) \
+#define L_DBUS_RW_PROPERTY(_name, _type, _get, _set, _exists) \
.name = _name, \
.type = _type, \
.get = _get, \
.set = _set, \
+ .exists = _exists, \
.flags = L_DBUS_PROPERTY_FLAG_WRITABLE
-#define L_DBUS_RO_DEPRECATED_PROPERTY(_name, _type, _get) \
+#define L_DBUS_RO_DEPRECATED_PROPERTY(_name, _type, _get, _exists) \
.name = _name, \
.type = _type, \
.get = _get, \
+ .exists = _exists, \
.flags = L_DBUS_PROPERTY_FLAG_DEPRECATED
-#define L_DBUS_RW_DEPRECATED_PROPERTY(_name, _type, _get, _set) \
+#define L_DBUS_RW_DEPRECATED_PROPERTY(_name, _type, _get, _set, _exists) \
.name = _name, \
.type = _type, \
.get = _get, \
.set = _set, \
+ .exists = _exists, \
.flags = L_DBUS_PROPERTY_FLAG_WRITABLE | \
L_DBUS_PROPERTY_FLAG_DEPRECATED
--
2.0.4
10:36 a.m.
New subject: [PATCH 17/21] unit: Apply the changes related to interface property API
Now providing pointer for get, set and exists methods, part of
org.freedesktop.DBus.Properties interface standard support.
---
unit/test-dbus-service.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/unit/test-dbus-service.c b/unit/test-dbus-service.c
index 825a159..a4a6c85 100644
--- a/unit/test-dbus-service.c
+++ b/unit/test-dbus-service.c
@@ -343,7 +343,7 @@ static const struct l_dbus_signal sample_signals[] = {
};
static const struct l_dbus_property sample_properties[] = {
- { L_DBUS_RW_PROPERTY("Bar", "y") },
+ { L_DBUS_RW_PROPERTY("Bar", "y", NULL, NULL, NULL) },
{ },
};
--
2.0.4
10:36 a.m.
New subject: [PATCH 18/21] examples: Apply the changes related to interface property API
Now providing pointer for get, set and exists methods, part of
org.freedesktop.DBus.Properties interface standard support.
---
examples/dbus-service.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index 06cf1d9..261f311 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -206,8 +206,8 @@ static const struct l_dbus_signal test_signals[] = {
};
static const struct l_dbus_property test_properties[] = {
- { L_DBUS_RW_PROPERTY("String", "s") },
- { L_DBUS_RW_PROPERTY("Integer", "u") },
+ { L_DBUS_RW_PROPERTY("String", "s", NULL, NULL, NULL) },
+ { L_DBUS_RW_PROPERTY("Integer", "u", NULL, NULL, NULL) },
{ },
};
--
2.0.4
10:36 a.m.
New subject: [PATCH 19/21] dbus: Add org.freedesktop.DBus.Properties.PropertiesChanged signal
Finalizing the support for org.freedesktop.DBus.Properties standard
interface via supporting its PropertiesChanged signal.
---
ell/dbus-service.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
ell/dbus-service.h | 4 ++
2 files changed, 179 insertions(+), 1 deletion(-)
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index b0f0063..ab6fafe 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -32,6 +32,7 @@
#include "queue.h"
#include "string.h"
#include "hashmap.h"
+#include "idle.h"
#include "dbus.h"
#include "dbus-service.h"
#include "dbus-private.h"
@@ -86,6 +87,10 @@ struct _dbus_pending_data {
uint32_t id;
struct l_dbus *dbus;
struct l_dbus_message *message;
+ struct l_idle *idler;
+ char *path;
+ struct interface_instance *instance;
+ struct l_queue *properties_changed;
};
void _dbus_print_arguments(const struct l_dbus_argument *args,
@@ -344,6 +349,11 @@ static void destroy_pending_data(void *data)
if (!pending_data)
return;
+ if (pending_data->idler) {
+ l_idle_remove(pending_data->idler);
+ return;
+ }
+
l_dbus_message_unref(pending_data->message);
l_free(pending_data);
}
@@ -703,6 +713,14 @@ static const struct l_dbus_method properties_methods[] = {
{ }
};
+static const struct l_dbus_signal properties_signals[] = {
+ { L_DBUS_SIGNAL("PropertiesChanged",
+ L_DBUS_ARGS({ "interface", "s" },
+ { "changed_properties", "a{sv}" },
+ { "invalidated_properties", "as"})) },
+ { }
+};
+
bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
const char *path, const char *interface,
const struct l_dbus_method *methods,
@@ -756,7 +774,8 @@ bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
_dbus_object_tree_register(tree, path,
DBUS_INTERFACE_PROPERTIES,
properties_methods,
- NULL, NULL, instance, NULL);
+ properties_signals,
+ NULL, instance, NULL);
}
return true;
@@ -1025,3 +1044,158 @@ LIB_EXPORT void l_dbus_pending_property_error(struct l_dbus *dbus,
uint32_t id,
va_end(args);
}
+
+static bool match_pending_data_by_instance(const void *data,
+ const void *user_data)
+{
+ const struct _dbus_pending_data *pending_data = data;
+ const struct interface_instance *instance = user_data;
+
+ if (pending_data->instance == instance)
+ return true;
+
+ return false;
+}
+
+static void process_property_change(struct l_idle *idle, void *data)
+{
+ struct _dbus_pending_data *pending_data = data;
+ struct l_dbus_message *signal = NULL;
+ struct interface_instance *instance;
+ struct l_dbus_message_builder *builder;
+ struct l_queue *invalidated = NULL;
+ const struct l_queue_entry *entry;
+ struct _dbus_object_tree *tree;
+
+ if (l_queue_isempty(pending_data->properties_changed))
+ goto out;
+
+ signal = l_dbus_message_new_signal(pending_data->dbus,
+ pending_data->path,
+ DBUS_INTERFACE_PROPERTIES,
+ "PropertiesChanged");
+ if (!signal)
+ goto out;
+
+ instance = pending_data->instance;
+
+ builder = l_dbus_message_builder_new(signal);
+ l_dbus_message_builder_append_basic(builder, 's',
+ instance->interface->name);
+ l_dbus_message_builder_enter_array(builder, "{sv}");
+
+ entry = l_queue_get_entries(pending_data->properties_changed);
+ while (entry) {
+ const struct l_dbus_property *p = entry->data;
+
+ if (!p->get)
+ goto next;
+
+ if (p->exists && !p->exists(p, instance->user_data)) {
+ if (!invalidated)
+ invalidated = l_queue_new();
+
+ l_queue_push_tail(invalidated, (void *) p);
+ goto next;
+ }
+
+ append_property(instance, p, builder);
+ next:
+ entry = entry->next;
+ }
+
+ l_dbus_message_builder_leave_array(builder);
+ l_dbus_message_builder_enter_array(builder, "s");
+
+ entry = l_queue_get_entries(invalidated);
+ while (entry) {
+ const struct l_dbus_property *p = entry->data;
+
+ l_dbus_message_builder_append_basic(builder, 's', p->name);
+
+ entry = entry->next;
+ }
+
+ l_queue_destroy(invalidated, NULL);
+
+ l_dbus_message_builder_leave_array(builder);
+ l_dbus_message_builder_finalize(builder);
+ l_dbus_message_builder_destroy(builder);
+
+ l_dbus_send(pending_data->dbus, signal);
+out:
+ tree = _dbus_object_tree_get_from_dbus(pending_data->dbus);
+ l_queue_remove(tree->pending_data, pending_data);
+ l_idle_remove(idle);
+}
+
+static void destroy_property_idler(void *data)
+{
+ struct _dbus_pending_data *pending_data = data;
+
+ l_queue_destroy(pending_data->properties_changed, NULL);
+ l_free(pending_data->path);
+ l_free(pending_data);
+}
+
+static bool match_data(const void *data, const void *user_data)
+{
+ if (data == user_data)
+ return true;
+
+ return false;
+}
+
+LIB_EXPORT void l_dbus_emit_property_changed(struct l_dbus *dbus,
+ const char *path,
+ const char *interface,
+ const char *name)
+{
+ struct _dbus_object_tree *tree;
+ struct object_node *object;
+ struct interface_instance *instance;
+ const struct l_dbus_property *property;
+ struct _dbus_pending_data *pending_data;
+
+ if (unlikely(!dbus))
+ return;
+
+ if (!_dbus_valid_interface(interface))
+ return;
+
+ if (!_dbus_valid_object_path(path))
+ return;
+
+ tree = _dbus_object_tree_get_from_dbus(dbus);
+
+ object = l_hashmap_lookup(tree->objects, path);
+ if (!object)
+ return;
+
+ instance = _dbus_object_find_instance(object, interface);
+ if (!instance)
+ return;
+
+ property = _dbus_interface_find_property(instance->interface, name);
+ if (!property)
+ return;
+
+ pending_data = l_queue_find(tree->pending_data,
+ match_pending_data_by_instance, instance);
+ if (!pending_data) {
+ pending_data = l_new(struct _dbus_pending_data, 1);
+ pending_data->dbus = dbus;
+ pending_data->path = l_strdup(path);
+ pending_data->instance = instance;
+ pending_data->properties_changed = l_queue_new();
+
+ pending_data->idler = l_idle_create(process_property_change,
+ pending_data, destroy_property_idler);
+ l_queue_push_tail(tree->pending_data, pending_data);
+ }
+
+ if (!l_queue_find(pending_data->properties_changed,
+ match_data, (void *) property))
+ l_queue_push_tail(pending_data->properties_changed,
+ (void *) property);
+}
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index d5a9a8e..d13c9f1 100644
--- a/ell/dbus-service.h
+++ b/ell/dbus-service.h
@@ -189,6 +189,10 @@ void l_dbus_pending_property_error(struct l_dbus *dbus, uint32_t id,
const char *name,
const char *format, ...);
+void l_dbus_emit_property_changed(struct l_dbus *dbus, const char *path,
+ const char *interface,
+ const char *name);
+
#ifdef __cplusplus
}
#endif
--
2.0.4
10:36 a.m.
New subject: [PATCH 20/21] unit: Add a test for the org.freedesktop.DBus.Properties interface
This will run a properties interface, without any specific methods or
signals. Instead, it will test only the org.freedesktop.DBus.Properties
interface which is automatically created by ell. These methods will be
tested: GetAll, Set and Get. PropertiesChanged signal cannot be caught
since ell does not provide yet a mean to catch a signal.
---
Makefile.am | 3 +
unit/test-dbus-freedesktop.c | 347 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 350 insertions(+)
create mode 100644 unit/test-dbus-freedesktop.c
diff --git a/Makefile.am b/Makefile.am
index beda08b..e86b740 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -101,6 +101,7 @@ unit_tests = unit/test-unit \
unit/test-dbus-util \
unit/test-dbus-message \
unit/test-dbus-service \
+ unit/test-dbus-freedesktop \
unit/test-gvariant-util \
unit/test-gvariant-message \
unit/test-siphash \
@@ -144,6 +145,8 @@ unit_test_dbus_util_LDADD = ell/libell-private.la
unit_test_dbus_service_LDADD = ell/libell-private.la
+unit_test_dbus_freedesktop_LDADD = ell/libell-private.la
+
unit_test_gvariant_util_LDADD = ell/libell-private.la
unit_test_gvariant_message_LDADD = ell/libell-private.la
diff --git a/unit/test-dbus-freedesktop.c b/unit/test-dbus-freedesktop.c
new file mode 100644
index 0000000..385707d
--- /dev/null
+++ b/unit/test-dbus-freedesktop.c
@@ -0,0 +1,347 @@
+/*
+ *
+ * Embedded Linux library
+ *
+ * Copyright (C) 2011-2014 Intel Corporation. All rights reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <assert.h>
+#include <stdbool.h>
+
+#include <ell/ell.h>
+#include "ell/dbus-private.h"
+
+#define DBUS_TEST_PATH "/"
+#define DBUS_TEST_INTERFACE "net.test.dbus"
+#define DBUS_INTERFACE_PROPERTIES "org.freedesktop.DBus.Properties"
+
+static struct l_dbus *dbus;
+static int status = -1;
+static char *bar_value = NULL;
+
+static void dbus_method_error(struct l_dbus_message *message)
+{
+ const char *error, *text;
+
+ if (!l_dbus_message_get_error(message, &error, &text))
+ return;
+
+ l_error("error=%s", error);
+ l_error("message=%s", text);
+ l_main_quit();
+}
+
+static void get_cb(struct l_dbus_message *message, void *user_data)
+{
+ struct l_dbus_message_iter variant;
+ const char *sval;
+
+ l_info("Get result");
+
+ dbus_method_error(message);
+
+ l_dbus_message_get_arguments(message, "v", &variant);
+ l_dbus_message_iter_get_variant(&variant, "s", &sval);
+
+ if (!strcmp(sval, "done"))
+ status = 0;
+
+ l_main_quit();
+}
+
+static void build_get_call(struct l_dbus_message *message, void *user_data)
+{
+ const char *interface = DBUS_TEST_INTERFACE;
+ const char *name = "bar";
+
+ l_dbus_message_set_arguments(message, "ss", interface, name);
+}
+
+static void test_get_bar(void *user_data)
+{
+ l_dbus_method_call(dbus, DBUS_TEST_INTERFACE,
+ DBUS_TEST_PATH,
+ DBUS_INTERFACE_PROPERTIES,
+ "Get", build_get_call,
+ get_cb, NULL, NULL);
+}
+
+static void set_cb(struct l_dbus_message *message, void *user_data)
+{
+ l_info("Set result");
+
+ dbus_method_error(message);
+
+ l_idle_oneshot(test_get_bar, NULL, NULL);
+}
+
+static void build_set_call(struct l_dbus_message *message, void *user_data)
+{
+ const char *interface = DBUS_TEST_INTERFACE;
+ const char *name = "bar";
+ const char *value = "done";
+
+ l_dbus_message_set_arguments(message, "ssv",
+ interface, name, "s", value);
+}
+
+static void test_set_bar(void *user_data)
+{
+ l_dbus_method_call(dbus, DBUS_TEST_INTERFACE,
+ DBUS_TEST_PATH,
+ DBUS_INTERFACE_PROPERTIES,
+ "Set", build_set_call,
+ set_cb, NULL, NULL);
+}
+
+static void get_all_cb(struct l_dbus_message *message, void *user_data)
+{
+ struct l_dbus_message_iter dict, variant;
+ const char *name, *sval;
+ int ival;
+
+ l_info("GetAll result");
+
+ dbus_method_error(message);
+
+ if (!l_dbus_message_get_arguments(message, "a{sv}", &dict))
+ goto error;
+
+ while (l_dbus_message_iter_next_entry(&dict, &name, &variant)) {
+ if (!strcmp(name, "foo")) {
+ l_dbus_message_iter_get_variant(&variant, "i", &ival);
+ if (ival != 7)
+ goto error;
+ } else if (!strcmp(name, "bar")) {
+ l_dbus_message_iter_get_variant(&variant, "s", &sval);
+ if (strcmp(sval, "bapapa"))
+ goto error;
+ } else if (!strcmp(name, "not")) {
+ l_dbus_message_iter_get_variant(&variant, "i", &ival);
+ if (ival != 42)
+ goto error;
+ } else
+ goto error;
+ }
+
+ l_idle_oneshot(test_set_bar, NULL, NULL);
+
+ return;
+error:
+ l_main_quit();
+}
+
+static void build_get_all_call(struct l_dbus_message *message, void *user_data)
+{
+ const char *interface = DBUS_TEST_INTERFACE;
+
+ l_dbus_message_set_arguments(message, "s", interface);
+}
+
+static void test_get_all(void *user_data)
+{
+ l_dbus_method_call(dbus, DBUS_TEST_INTERFACE,
+ DBUS_TEST_PATH,
+ DBUS_INTERFACE_PROPERTIES,
+ "GetAll", build_get_all_call,
+ get_all_cb, NULL, NULL);
+}
+
+static void request_name_setup(struct l_dbus_message *message, void *user_data)
+{
+ const char *name = DBUS_TEST_INTERFACE;
+
+ l_dbus_message_set_arguments(message, "su", name, 0);
+}
+
+static void request_name_callback(struct l_dbus_message *message,
+ void *user_data)
+{
+ uint32_t result;
+
+ dbus_method_error(message);
+
+ if (!l_dbus_message_get_arguments(message, "u", &result))
+ return;
+
+ l_info("request name result=%d", result);
+ l_idle_oneshot(test_get_all, NULL, NULL);
+}
+
+static void ready_callback(void *user_data)
+{
+ l_info("DBus is ready");
+}
+
+static void disconnect_callback(void *user_data)
+{
+ l_main_quit();
+}
+
+static bool get_foo(const struct l_dbus_property *property,
+ struct l_dbus_message_builder *builder,
+ void *user_data)
+{
+ int value = 7;
+
+ l_dbus_message_builder_append_basic(builder, 'i', &value);
+
+ return true;
+}
+
+static bool get_bar(const struct l_dbus_property *property,
+ struct l_dbus_message_builder *builder,
+ void *user_data)
+{
+ const char *value = bar_value;
+
+ if (!value)
+ value = "bapapa";
+
+ l_dbus_message_builder_append_basic(builder, 's', value);
+
+ return true;
+}
+
+static bool get_not(const struct l_dbus_property *property,
+ struct l_dbus_message_builder *builder,
+ void *user_data)
+{
+ int value = 42;
+
+ l_dbus_message_builder_append_basic(builder, 'i', &value);
+
+ return true;
+}
+
+static void set_bar(const struct l_dbus_property *property,
+ struct l_dbus *dbus,
+ struct l_dbus_message_iter *iter,
+ uint32_t id, void *user_data)
+{
+ const char *error_str = "Invalid arguments";
+ const char *value;
+
+ if (strcmp(property->name, "bar"))
+ goto invalid;
+
+ if (!l_dbus_message_iter_get_variant(iter, "s", &value)) {
+ error_str = "Could not get the value";
+ goto invalid;
+ }
+
+ if (bar_value)
+ l_free(bar_value);
+
+ bar_value = l_strdup(value);
+
+ l_dbus_pending_property_success(dbus, id);
+ l_dbus_emit_property_changed(dbus, DBUS_TEST_PATH,
+ DBUS_TEST_INTERFACE, property->name);
+ return;
+invalid:
+ l_dbus_pending_property_error(dbus, id, "org.test.InvalidArguments",
+ "%s", error_str);
+}
+
+static bool property_exists(const struct l_dbus_property *property,
+ void *user_data)
+{
+ if (!strcmp(property->name, "foo") ||
+ !strcmp(property->name, "bar") ||
+ !strcmp(property->name, "not") ||
+ !strcmp(property->name, "nOne"))
+ return true;
+ return false;
+}
+
+static void do_debug(const char *str, void *user_data)
+{
+ const char *prefix = user_data;
+
+ l_info("%s%s", prefix, str);
+}
+
+static void signal_handler(struct l_signal *signal, uint32_t signo,
+ void *user_data)
+{
+ switch (signo) {
+ case SIGINT:
+ case SIGTERM:
+ l_info("Terminate");
+ l_main_quit();
+ break;
+ }
+}
+
+static struct l_dbus_property test_properties[] = {
+ { L_DBUS_RO_PROPERTY("foo", "i", get_foo, property_exists) },
+ { L_DBUS_RW_PROPERTY("bar", "s", get_bar, set_bar, property_exists)
},
+ { L_DBUS_RO_PROPERTY("nOne", "s", NULL, property_exists) },
+ { L_DBUS_RO_PROPERTY("nope", "s", NULL, NULL) },
+ { L_DBUS_RW_PROPERTY("not", "i", get_not, NULL, property_exists) },
+ { }
+};
+
+int main(int argc, char *argv[])
+{
+ struct l_signal *signal;
+ sigset_t mask;
+
+ sigemptyset(&mask);
+ sigaddset(&mask, SIGINT);
+ sigaddset(&mask, SIGTERM);
+ signal = l_signal_create(&mask, signal_handler, NULL, NULL);
+
+ l_log_set_stderr();
+
+ dbus = l_dbus_new_default(L_DBUS_SESSION_BUS);
+
+ l_dbus_set_debug(dbus, do_debug, "[DBUS] ", NULL);
+ l_dbus_set_ready_handler(dbus, ready_callback, dbus, NULL);
+ l_dbus_set_disconnect_handler(dbus, disconnect_callback, NULL, NULL);
+
+ l_dbus_method_call(dbus, "org.freedesktop.DBus",
+ "/org/freedesktop/DBus",
+ "org.freedesktop.DBus", "RequestName",
+ request_name_setup,
+ request_name_callback, NULL, NULL);
+
+ if (!l_dbus_register_interface(dbus, DBUS_TEST_PATH,
+ DBUS_TEST_INTERFACE, NULL, NULL,
+ test_properties, NULL, NULL))
+ goto cleanup;
+
+ l_main_run();
+
+ if (bar_value)
+ l_free(bar_value);
+
+ if (status < 0)
+ l_info("Test failed");
+
+cleanup:
+ l_dbus_destroy(dbus);
+ l_signal_remove(signal);
+
+ return status;
+}
--
2.0.4
10:36 a.m.
New subject: [PATCH 21/21] git: Ignoring the org.freedesktop.DBus.Properties interface test binary
---
.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/.gitignore b/.gitignore
index 1047a87..9448673 100644
--- a/.gitignore
+++ b/.gitignore
@@ -35,6 +35,7 @@ unit/test-dbus
unit/test-dbus-message
unit/test-dbus-util
unit/test-dbus-service
+unit/test-dbus-freedesktop
unit/test-gvariant-util
unit/test-gvariant-message
unit/test-siphash
--
2.0.4
3:39 p.m.
Hi Tomasz,
Just some preliminary comments:
There is already a dbus service API in ell. I wanted to keep it,
wrapping the
gdbus-like API within the existing functions. However that has proven to be
impossible unless I would modify the existing API. Thus my conclusion: if I
have to break the existing API, let's do it all. So I finnally got rid of the
existing API.
I find table/macro based approach unreadable and that is why I did not
use the gdbus approach in ell. I'm actually quite happy with the
current API, it fits in with ell's design philosophy much better. If
someone wants to re-use the API from gdbus I'm good with that, but
please don't throw stuff out in the process.
Right now we are only focused on iwd, and this is our opportunity to try
new things. I wouldn't rush to try and replicate old ways of doing things.
To support DBus.Properties some modifications would absolutely be
required to the l_dbus_interface_property method. What other changes do
you feel are necessary?
Also, patch 4, due to the port of the constants structure, it looses
the
pre-built signature, thus adding the necessity to add a compare_signature()
function as in GDBus. It was nice to get this full pre-built signature that
could be compared at once. If you have ideas how to get it without changing the
API... Like changing the macro L_DBUS_ARGS() or L_DBUS_METHOD, so it would set
a const char * in_signature/out_signature in struct l_dbus_method for instance?
Macro magic won't really help here. However, you can do this as part of
the l_dbus_register_interface call. It would be no different from the
current setup, just the information is coming from a static array
instead of vargs.
About other improvements:
- patch 1 is a tiny helper I required in patch 15, since ell checks the
signature of the variant we try to set, before calling the set function.
- patch 2 and 3 are there to separate the code properly. It hads thus a getter
function for the object tree, but there is no shared structure informations
between dbus.c and dbus-service.c. I figured out it was cleaner that way imo.
I'm okay with these
Minor issues:
- I put the error message macros in dbus-private.h, figuring that some other
part of the code could need it. But maybe it's better no to share those?
Always err on the side of opaqueness. If someone needs it later, it is
simple to move the #define appropriately. If no one needs it, you just
cluttered up the namespace.
- When it's required to send an empty reply, it would be good to have a helper
function I think, which wraps l_dbus_message_new_method_return() +
l_dbus_message_set_arguments + l_dbus_send together (see patch 15 in
function l_dbus_pending_property_success).
We probably need a simple wrapper for l_dbus_message_new_method_return
that takes varargs. Similar to l_dbus_message_new_error(). The send
step should probably not be combined in the ell API directly.
Regards,
-Denis
10:34 a.m.
Hi Denis,
> There is already a dbus service API in ell. I wanted to keep it,
> wrapping the
> gdbus-like API within the existing functions. However that has proven
> to be
> impossible unless I would modify the existing API. Thus my
> conclusion: if I
> have to break the existing API, let's do it all. So I finnally got
> rid of the
> existing API.
I find table/macro based approach unreadable and that is why I did not
use the gdbus approach in ell. I'm actually quite happy with the
current API, it fits in with ell's design philosophy much better. If
someone wants to re-use the API from gdbus I'm good with that, but
please don't throw stuff out in the process.
Right now we are only focused on iwd, and this is our opportunity to
try new things. I wouldn't rush to try and replicate old ways of
doing things.
I am fine with trying new things as long as we identify problems to get
fixed in the old way of doing things.
Readability is pretty subjective, I did not consider it. (I am ok with
the table/macro approach)
The good part with tables over function calls is that it does all at once.
Technically also, it uses less memory, does way less function calls,
probably less context switches etc...
That's also why I kept most of underlying implementation, since it
technically brings some nice stuff.
The other motivation was of course moving from gdbus to ell in existing
projects could be much harder if the API is that different.
> - When it's required to send an empty reply, it would be good
to have
> a helper
> function I think, which wraps l_dbus_message_new_method_return() +
> l_dbus_message_set_arguments + l_dbus_send together (see patch 15 in
> function l_dbus_pending_property_success).
>
We probably need a simple wrapper for l_dbus_message_new_method_return
that takes varargs. Similar to l_dbus_message_new_error(). The send
step should probably not be combined in the ell API directly.
Better approach yes, will prepare a patch for it.
Tomasz
4:24 p.m.
Hi Tomasz,
On 11/17/2014 04:34 AM, Tomasz Bursztyka wrote:
Hi Denis,
>> There is already a dbus service API in ell. I wanted to keep it,
>> wrapping the
>> gdbus-like API within the existing functions. However that has proven
>> to be
>> impossible unless I would modify the existing API. Thus my
>> conclusion: if I
>> have to break the existing API, let's do it all. So I finnally got
>> rid of the
>> existing API.
>
> I find table/macro based approach unreadable and that is why I did not
> use the gdbus approach in ell. I'm actually quite happy with the
> current API, it fits in with ell's design philosophy much better. If
> someone wants to re-use the API from gdbus I'm good with that, but
> please don't throw stuff out in the process.
>
> Right now we are only focused on iwd, and this is our opportunity to
> try new things. I wouldn't rush to try and replicate old ways of
> doing things.
I am fine with trying new things as long as we identify problems to get
fixed in the old way of doing things.
Readability is pretty subjective, I did not consider it. (I am ok with
the table/macro approach)
To me API readability is the most important aspect. The function based
approach also allows way more flexibility. For example, in GDBUS today,
if we want to implement the non-ObjectManager based APIs, we have omit
the Property definitions (and thus introspection data).
The good part with tables over function calls is that it does all at
once.
Does what all at once? You keep a bunch of static memory around until
someone calls register_interface.
Technically also, it uses less memory, does way less function calls,
probably less context switches etc...
Memory? Maybe, or maybe not. The metadata is pretty compact, what
wastes most is the linked list overhead. But then we are able to play
some pretty neat tricks in order to optimize the introspection
generation and call dispatch.
However, I digress. This is all a drop in the bucket in the context of
the larger program execution. Trying to optimize any of this, while
interesting, is not that productive.
That's also why I kept most of underlying implementation, since
it
technically brings some nice stuff.
The other motivation was of course moving from gdbus to ell in existing
projects could be much harder if the API is that different.
Except you decided to diverge from GDBUS APIs as well for some reason by
introducing L_DBUS_RW_PROPERTY, L_DBUS_RO_PROPERTY, etc ;)
So here's what I propose. If you are dead set on adding
DBus.Properties, then lets do this on top of the existing code. The
GDBUS compatibility layer can be bolted on top of that easily enough.
At the end of the day it is just some extra struct definitions and an
alternative call to l_dbus_register_interface. Can probably be done in
less than a day.
For adding DBus.Properties handlers, we can add something like:
l_dbus_interface_property_add_handlers(interface, property, get_func,
set_func, exists_func);
The first time the above is called, we automagically add DBus.Properties
Interface introspection and capability to the object in question. If it
isn't called, then we don't include DBus.Properties at all.
Regards,
-Denis
2296
days inactive
2300
days old
24 comments
2 participants
participants (2)
-
Denis Kenzior -
Tomasz Bursztyka