On (12/05/14 23:27), Jakub Hrozek wrote:
Hi,
the attached patchset implement the
org.freedesktop.DBus.Properties.Get() interface for properties of
primitive types and arrays of primitive types (except for array of bools
as DBus bool is of different size than C-sized bool).
Getters are always synchronous, mostly to allow sane GetAll processing.
The getters also never error out, but rather return a default value.
The first two patches add utility functions, one by me and one by Pavel
that make it easier to return a primitive type or an array in a variant.
I wish the DBus library itself had a helper like this...
Patch 3/6 just allows the messages to be passed through to the IFP
responder.
The whole patchset is based on Stef's WIP branch and expands on it, that's
patch 4/6. There is a setter stub that, while not finished, is not
harmful either and can be expanded on later.
Patch 5/6 is the meat of the whole patchset. It implements the sbus
codegen to generate getter functions based on the XML description of the
interface and adds unit test for all the supported types and all
supported arrays. Pavel was kind enough to contribute several bugfixes
for this patch -- thank you!
Patch 6/6 is a bugfix by Pavel, one that I agree with, you can consider
the patch acked by me.
The next patch I'm going to send will implement a 'raw property getter'
that would allow the interface developer to return complex values for a
property value.
From f5c17fe154cc396f7a497dcc389c8a2fd5037a7d Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Mon, 12 May 2014 18:59:35 +0200
Subject: [PATCH 1/6] SBUS: Utility function sbus_request_return_as_variant
Adds a utility function that returns a single value with a given type in
a variant. This utility function will be used by the Get property call.
---
src/sbus/sssd_dbus.h | 4 ++++
src/sbus/sssd_dbus_request.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h
index 15ca521184f82c7424549907b7afa912f891c3ad..efb0296813e9946f2b2fa1fd867592315cc4ee4f
100644
--- a/src/sbus/sssd_dbus.h
+++ b/src/sbus/sssd_dbus.h
@@ -246,6 +246,10 @@ int sbus_request_return_and_finish(struct sbus_request *dbus_req,
int first_arg_type,
...);
+int sbus_request_return_as_variant(struct sbus_request *dbus_req,
+ int type,
+ const void *value);
+
/*
* Return an error for a DBus method call request. The @error is a normal
* DBusError.
diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c
index 2d23931aa07a83c15c30b31615d5db7cfee11a0a..af439b284024b2b335adc0119f131dd6046c61bc
100644
--- a/src/sbus/sssd_dbus_request.c
+++ b/src/sbus/sssd_dbus_request.c
@@ -125,6 +125,63 @@ int sbus_request_return_and_finish(struct sbus_request *dbus_req,
return ret;
}
+int sbus_request_return_as_variant(struct sbus_request *dbus_req,
+ int type,
+ const void *value)
+{
+ DBusMessage *reply;
+ dbus_bool_t dbret;
+ DBusMessageIter iter;
+ DBusMessageIter valiter;
+ char strtype[2];
+ int ret;
+
+ snprintf(strtype, 2, "%c", type);
+
+ reply = dbus_message_new_method_return(dbus_req->message);
+ if (reply == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory allocating DBus message\n");
+ sbus_request_finish(dbus_req, NULL);
+ return ENOMEM;
+ }
+
+ dbus_message_iter_init_append(reply, &iter);
+ dbret = dbus_message_iter_open_container(&iter, DBUS_TYPE_VARIANT,
+ strtype, &valiter);
+ if (!dbret) {
+ sbus_request_finish(dbus_req, NULL);
+ ret = EINVAL;
+ goto done;
+ }
+
+ dbret = dbus_message_iter_append_basic(&valiter, type, value);
+ if (!dbret) {
+ sbus_request_finish(dbus_req, NULL);
+ ret = EINVAL;
+ goto done;
+ }
+
+ dbret = dbus_message_iter_close_container(&iter, &valiter);
+ if (!dbret) {
+ sbus_request_finish(dbus_req, NULL);
+ ret = EINVAL;
+ goto done;
+ }
// dbret will be 0 after this line
+
+ if (dbret) {
+ ret = sbus_request_finish(dbus_req, reply);
+ } else {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Couldn't build DBus message\n");
+ sbus_request_finish(dbus_req, NULL);
+ ret = EINVAL;
// this branch is dead code
+ }
+
+done:
+ dbus_message_unref(reply);
+ return ret;
+}
+
+
int sbus_request_fail_and_finish(struct sbus_request *dbus_req,
const DBusError *error)
{
--
1.9.0
From a0fe9eb748301ed5df475ff55e375ba2bcd75f31 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Mon, 12 May 2014 19:00:36 +0200
Subject: [PATCH 2/6] SBUS: Utility function
sbus_request_return_array_as_variant
Adds a utility function that returns an array of types values, each of a
given size, with a given type in a variant. This utility function will be
used by the GetAll property call.
---
src/sbus/sssd_dbus.h | 6 +++
src/sbus/sssd_dbus_request.c | 103 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 109 insertions(+)
diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c
index af439b284024b2b335adc0119f131dd6046c61bc..f2870077d6dc96b94c083ccdc9fc9a4c00c0428d
100644
--- a/src/sbus/sssd_dbus_request.c
+++ b/src/sbus/sssd_dbus_request.c
@@ -182,6 +182,109 @@ done:
}
+int sbus_request_return_array_as_variant(struct sbus_request *dbus_req,
+ int type,
+ uint8_t *values,
+ const int len,
+ const size_t item_size)
+{
// snip
+
+ dbret = dbus_message_iter_close_container(&iter, &variant_iter);
+ if (!dbret) {
+ sbus_request_finish(dbus_req, NULL);
+ ret = EINVAL;
+ goto done;
+ }
// dbret will be 0 after this line
+
+ if (dbret) {
+ ret = sbus_request_finish(dbus_req, reply);
+ } else {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Couldn't build DBus message\n");
+ sbus_request_finish(dbus_req, NULL);
+ ret = EINVAL;
// the same problem as in 1st patch.
+ }
+
+done:
+ if (reply != NULL) {
+ dbus_message_unref(reply);
+ }
+
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
int sbus_request_fail_and_finish(struct sbus_request *dbus_req,
const DBusError *error)
{
--
1.9.0
From 5733ceac904342a7a979d0efe066afd9f093ebe9 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Tue, 22 Apr 2014 21:50:28 +0200
Subject: [PATCH 3/6] IFP: Allow Set, Get and GetAll from DBus.Properties
The InfoPipe will support all three of:
DBus.Properties.Get
DBus.Properties.GetAll
DBus.Properties.Set
Hence it must allow these calls to be received.
---
src/responder/ifp/org.freedesktop.sssd.infopipe.conf | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/responder/ifp/org.freedesktop.sssd.infopipe.conf
b/src/responder/ifp/org.freedesktop.sssd.infopipe.conf
index 56e460aa11c2534e95092fb10216b05c50c04d59..4f2a906ba812d46b6bca401d3fc14baa5c18de50
100644
--- a/src/responder/ifp/org.freedesktop.sssd.infopipe.conf
+++ b/src/responder/ifp/org.freedesktop.sssd.infopipe.conf
@@ -19,6 +19,16 @@
<allow send_destination="org.freedesktop.sssd.infopipe"
send_interface="org.freedesktop.DBus.Introspectable"/>
+ <allow send_destination="org.freedesktop.sssd.infopipe"
+ send_interface="org.freedesktop.DBus.Properties"
+ send_member="GetAll"/>
+ <allow send_destination="org.freedesktop.sssd.infopipe"
+ send_interface="org.freedesktop.DBus.Properties"
+ send_member="Get"/>
+ <allow send_destination="org.freedesktop.sssd.infopipe"
+ send_interface="org.freedesktop.DBus.Properties"
+ send_member="Set"/>
+
<allow send_interface="org.freedesktop.sssd.infopipe"/>
</policy>
--
1.9.0
From d23886c484bc955bf4bc03998601aeec1650ce02 Mon Sep 17 00:00:00
2001
From: Stef Walter <stefw(a)redhat.com>
Date: Tue, 25 Feb 2014 18:31:03 +0100
Subject: [PATCH 4/6] WIP properties
I knot that is is a Stef's patch, but it
does not look professional to have
WIP patch in master.
From 796d392b6cb33d6ded6ae58e1abcca84cf6a9b25 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Wed, 23 Apr 2014 03:01:21 +0200
Subject: [PATCH 5/6] SBUS: Implement org.freedesktop.DBus.Properties.Get for
primitive types
This patch implements type-safe getters for primitive types and their
arrays. The patch includes unit tests of all supported types and arrays
of these types.
All getter are synchronous. The getters never fail, instead, they return
a default or 'not defined' value. Making the getters synchronous and
always returning a value will make it significantly easier to implement
the GetAll method.
---
src/sbus/sbus_codegen | 114 ++++---
src/sbus/sssd_dbus.h | 6 +
src/sbus/sssd_dbus_meta.h | 2 +
src/sbus/sssd_dbus_properties.c | 75 +++++
src/tests/sbus_codegen_tests.c | 490 ++++++++++++++++++++++++++++-
src/tests/sbus_codegen_tests.xml | 26 ++
src/tests/sbus_codegen_tests_generated.c | 522 +++++++++++++++++++++++++++++++
src/tests/sbus_codegen_tests_generated.h | 44 +++
8 files changed, 1235 insertions(+), 44 deletions(-)
//snip
diff --git a/src/sbus/sssd_dbus_properties.c
b/src/sbus/sssd_dbus_properties.c
index 835b3078b5abd124ab98265401aa48533a306ee3..bdd5b432fe57ca66a4d06f98720578b3a175a65c
100644
--- a/src/sbus/sssd_dbus_properties.c
+++ b/src/sbus/sssd_dbus_properties.c
@@ -105,6 +105,77 @@ dispatch_properties_set(struct sbus_connection *conn,
return EOK;
}
+static int
+dispatch_properties_get(struct sbus_connection *conn,
+ struct sbus_interface *intf,
+ DBusMessage *message)
+{
+ struct sbus_request *req;
+ const char *signature;
+ const struct sbus_interface_meta *meta;
+ DBusMessageIter iter;
+ sbus_msg_handler_fn handler_fn;
+ const struct sbus_property_meta *property;
+ const char *interface_name;
+ const char *property_name;
+
+ req = sbus_new_request(conn, intf, message);
+ if (req == NULL) {
+ return ENOMEM;
+ }
+
+ meta = intf->vtable->meta;
+
+ signature = dbus_message_get_signature(message);
+ /* Interface name, property name */
+ if (strcmp(signature, "ss") != 0) {
+ return sbus_request_fail_and_finish(req,
+ sbus_error_new(req,
+ DBUS_ERROR_INVALID_ARGS,
+ "Invalid argument types passed to Get
method"));
+ }
+
+ dbus_message_iter_init(message, &iter);
+ dbus_message_iter_get_basic(&iter, &interface_name);
+ dbus_message_iter_next(&iter);
+ dbus_message_iter_get_basic(&iter, &property_name);
+
+ if (strcmp(interface_name, meta->name) != 0) {
+ return sbus_request_fail_and_finish(req,
+ sbus_error_new(req,
+ DBUS_ERROR_UNKNOWN_INTERFACE,
+ "No such interface"));
+ }
+
+ property = sbus_meta_find_property(intf->vtable->meta, property_name);
+ if (property == NULL) {
+ return sbus_request_fail_and_finish(req,
+ sbus_error_new(req,
+ DBUS_ERROR_UNKNOWN_PROPERTY,
+ "No such property"));
+ }
+
+ if (!(property->flags & SBUS_PROPERTY_READABLE)) {
+ return sbus_request_fail_and_finish(req,
+ sbus_error_new(req,
+ DBUS_ERROR_ACCESS_DENIED,
+ "Property is not readable"));
+ }
+
+ handler_fn = VTABLE_FUNC(intf->vtable, property->vtable_offset_get);
+ if (!handler_fn) {
+ return sbus_request_fail_and_finish(req,
+ sbus_error_new(req,
+ DBUS_ERROR_NOT_SUPPORTED,
+ "Not implemented"));
+ }
+
+ sbus_request_invoke_or_finish(req, handler_fn,
+ intf->instance_data,
+ property->invoker_get);
+ return EOK;
+}
+
int sbus_properties_dispatch(struct sbus_request *dbus_req)
{
const char *member;
@@ -116,6 +187,10 @@ int sbus_properties_dispatch(struct sbus_request *dbus_req)
return dispatch_properties_set(dbus_req->conn,
dbus_req->intf,
dbus_req->message);
+ } else if (strcmp (member, "Get") == 0) {
+ return dispatch_properties_get(dbus_req->conn,
+ dbus_req->intf,
+ dbus_req->message);
}
The last two patches break build on rhel6.
src/sbus/sssd_dbus_properties.c:66: error: 'DBUS_ERROR_UNKNOWN_INTERFACE'
undeclared (first use in this function)
src/sbus/sssd_dbus_properties.c:66: error: (Each undeclared identifier is reported only
once
src/sbus/sssd_dbus_properties.c:66: error: for each function it appears in.)
src/sbus/sssd_dbus_properties.c:74: error: 'DBUS_ERROR_UNKNOWN_PROPERTY'
undeclared (first use in this function)
src/sbus/sssd_dbus_properties.c:81: error: 'DBUS_ERROR_PROPERTY_READ_ONLY'
undeclared (first use in this function)
src/sbus/sssd_dbus_properties.c: In function 'dispatch_properties_get':
src/sbus/sssd_dbus_properties.c:146: error: 'DBUS_ERROR_UNKNOWN_INTERFACE'
undeclared (first use in this function)
src/sbus/sssd_dbus_properties.c:154: error: 'DBUS_ERROR_UNKNOWN_PROPERTY'
undeclared (first use in this function)
make[2]: *** [src/sbus/libsss_util_la-sssd_dbus_properties.lo] Error 1
LS