[SSSD] [PATCH] Implement type-safe getters for primitive types and their arrays

Stef Walter stefw at redhat.com
Tue May 13 17:04:22 UTC 2014


On 12.05.2014 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).

This looks nice to me. A few comments.

+    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;
+    }

You probably want some diagnostics here, as this is where you'll end up
if you pass in an invalid @type.

+    dbus_message_iter_init_append(reply, &iter);
+    dbret = dbus_message_iter_open_container(&iter, DBUS_TYPE_VARIANT,
+                                             variant_type, &variant_iter);
+    if (!dbret) {
+        sbus_request_finish(dbus_req, NULL);
+        ret = EINVAL;
+        goto done;
+    }
+
+    /* Open container for values */
+    dbret = dbus_message_iter_open_container(&variant_iter,
DBUS_TYPE_ARRAY,
+                                             array_type, &array_iter);
+    if (!dbret) {
+        sbus_request_finish(dbus_req, NULL);
+        ret = EINVAL;
+        goto done;
+    }

Same here and here.

> +    for (i = 0; i < len; i++) {
> +        addr = values + i * item_size;
> +        dbret = dbus_message_iter_append_basic(&array_iter, type, addr);
> +        if (!dbret) {
> +            sbus_request_finish(dbus_req, NULL);
> +            ret = ENOMEM;
> +            goto done;
> +        }
> +    }

Are you planning on using this for byte arrays and such? If so it may be
worth looking into special casing arrays of bytes/ints with
dbus_message_iter_append_fixed_array()

http://dbus.freedesktop.org/doc/api/html/group__DBusMessage.html#ga6e2d1e936c3c61fe00d80a3f22fd5e76

But maybe that's not needed in this initial patchset?

> From d23886c484bc955bf4bc03998601aeec1650ce02 Mon Sep 17 00:00:00 2001
> From: Stef Walter <stefw at redhat.com>
> Date: Tue, 25 Feb 2014 18:31:03 +0100
> Subject: [PATCH 4/6] WIP properties

Should I give this a better commit message? :)

> +    if prop.type == "s":
> +        out("    out_val = prop_val == NULL ? \"\" : prop_val;")

You'll probably also want to special case object paths, returning "/" in
those cases.

Stef



More information about the sssd-devel mailing list