]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
sd-bus: add methods and signals parameter names. Fixes: #1564 11357/head
authorGiacinto Cifelli <gciofono@gmail.com>
Tue, 8 Jan 2019 11:14:37 +0000 (12:14 +0100)
committerGiacinto Cifelli <gciofono@gmail.com>
Tue, 26 Feb 2019 11:55:02 +0000 (12:55 +0100)
src/libsystemd/sd-bus/bus-introspect.c
src/libsystemd/sd-bus/bus-objects.c
src/libsystemd/sd-bus/bus-objects.h
src/libsystemd/sd-bus/bus-slot.c
src/libsystemd/sd-bus/test-bus-vtable.c
src/systemd/sd-bus-vtable.h

index f623dd9ce00425d079c54dff3098d84d918637c6..bde97483e83caa30f86e543902516eb88cabfad4 100644 (file)
@@ -4,6 +4,7 @@
 
 #include "bus-internal.h"
 #include "bus-introspect.h"
+#include "bus-objects.h"
 #include "bus-protocol.h"
 #include "bus-signature.h"
 #include "fd-util.h"
@@ -86,7 +87,9 @@ static void introspect_write_flags(struct introspect *i, int type, uint64_t flag
                 fputs("   <annotation name=\"org.freedesktop.systemd1.Privileged\" value=\"true\"/>\n", i->f);
 }
 
-static int introspect_write_arguments(struct introspect *i, const char *signature, const char *direction) {
+/* Note that "names" is both an input and an output parameter. It initially points to the first argument name in a
+   NULL-separated list of strings, and is then advanced with each argument, and the resulting pointer is returned. */
+static int introspect_write_arguments(struct introspect *i, const char *signature, const char **names, const char *direction) {
         int r;
 
         for (;;) {
@@ -101,6 +104,11 @@ static int introspect_write_arguments(struct introspect *i, const char *signatur
 
                 fprintf(i->f, "   <arg type=\"%.*s\"", (int) l, signature);
 
+                if (**names != '\0') {
+                        fprintf(i->f, " name=\"%s\"", *names);
+                        *names += strlen(*names) + 1;
+                }
+
                 if (direction)
                         fprintf(i->f, " direction=\"%s\"/>\n", direction);
                 else
@@ -111,10 +119,13 @@ static int introspect_write_arguments(struct introspect *i, const char *signatur
 }
 
 int introspect_write_interface(struct introspect *i, const sd_bus_vtable *v) {
+        const sd_bus_vtable *vtable = v;
+        const char *names = "";
+
         assert(i);
         assert(v);
 
-        for (; v->type != _SD_BUS_VTABLE_END; v++) {
+        for (; v->type != _SD_BUS_VTABLE_END; v = bus_vtable_next(vtable, v)) {
 
                 /* Ignore methods, signals and properties that are
                  * marked "hidden", but do show the interface
@@ -132,8 +143,10 @@ int introspect_write_interface(struct introspect *i, const sd_bus_vtable *v) {
 
                 case _SD_BUS_VTABLE_METHOD:
                         fprintf(i->f, "  <method name=\"%s\">\n", v->x.method.member);
-                        introspect_write_arguments(i, strempty(v->x.method.signature), "in");
-                        introspect_write_arguments(i, strempty(v->x.method.result), "out");
+                        if (bus_vtable_has_names(vtable))
+                                names = strempty(v->x.method.names);
+                        introspect_write_arguments(i, strempty(v->x.method.signature), &names, "in");
+                        introspect_write_arguments(i, strempty(v->x.method.result), &names, "out");
                         introspect_write_flags(i, v->type, v->flags);
                         fputs("  </method>\n", i->f);
                         break;
@@ -150,7 +163,9 @@ int introspect_write_interface(struct introspect *i, const sd_bus_vtable *v) {
 
                 case _SD_BUS_VTABLE_SIGNAL:
                         fprintf(i->f, "  <signal name=\"%s\">\n", v->x.signal.member);
-                        introspect_write_arguments(i, strempty(v->x.signal.signature), NULL);
+                        if (bus_vtable_has_names(vtable))
+                                names = strempty(v->x.method.names);
+                        introspect_write_arguments(i, strempty(v->x.signal.signature), &names, NULL);
                         introspect_write_flags(i, v->type, v->flags);
                         fputs("  </signal>\n", i->f);
                         break;
index d0538104ae251100f2e1ffd1f366040d646d6bfc..7ff4c3f93593cd6b150e4ce1d59ea400f8f3d09f 100644 (file)
@@ -736,7 +736,8 @@ static int vtable_append_all_properties(
         if (c->vtable[0].flags & SD_BUS_VTABLE_HIDDEN)
                 return 1;
 
-        for (v = c->vtable+1; v->type != _SD_BUS_VTABLE_END; v++) {
+        v = c->vtable;
+        for (v = bus_vtable_next(c->vtable, v); v->type != _SD_BUS_VTABLE_END; v = bus_vtable_next(c->vtable, v)) {
                 if (!IN_SET(v->type, _SD_BUS_VTABLE_PROPERTY, _SD_BUS_VTABLE_WRITABLE_PROPERTY))
                         continue;
 
@@ -1610,6 +1611,99 @@ static int vtable_member_compare_func(const struct vtable_member *x, const struc
 
 DEFINE_PRIVATE_HASH_OPS(vtable_member_hash_ops, struct vtable_member, vtable_member_hash_func, vtable_member_compare_func);
 
+typedef enum {
+        NAMES_FIRST_PART        = 1 << 0, /* first part of argument name list (input names). It is reset by names_are_valid() */
+        NAMES_PRESENT           = 1 << 1, /* at least one argument name is present, so the names will checked.
+                                             This flag is set and used internally by names_are_valid(), but needs to be stored across calls for 2-parts list  */
+        NAMES_SINGLE_PART       = 1 << 2, /* argument name list consisting of a single part */
+} names_flags;
+
+static bool names_are_valid(const char *signature, const char **names, names_flags *flags) {
+        int r;
+
+        if ((*flags & NAMES_FIRST_PART || *flags & NAMES_SINGLE_PART) && **names != '\0')
+                *flags |= NAMES_PRESENT;
+
+        for (;*flags & NAMES_PRESENT;) {
+                size_t l;
+
+                if (!*signature)
+                        break;
+
+                r = signature_element_length(signature, &l);
+                if (r < 0)
+                        return false;
+
+                if (**names != '\0') {
+                        if (!member_name_is_valid(*names))
+                                return false;
+                        *names += strlen(*names) + 1;
+                } else if (*flags & NAMES_PRESENT)
+                        return false;
+
+                signature += l;
+        }
+        /* let's check if there are more argument names specified than the signature allows */
+        if (*flags & NAMES_PRESENT && **names != '\0' && !(*flags & NAMES_FIRST_PART))
+                return false;
+        *flags &= ~NAMES_FIRST_PART;
+        return true;
+}
+
+/* the current version of this struct is defined in sd-bus-vtable.h, but we need to list here the historical versions
+   to make sure the calling code is compatible with one of these */
+struct sd_bus_vtable_original {
+        uint8_t type:8;
+        uint64_t flags:56;
+        union {
+                struct {
+                        size_t element_size;
+                } start;
+                struct {
+                        const char *member;
+                        const char *signature;
+                        const char *result;
+                        sd_bus_message_handler_t handler;
+                        size_t offset;
+                } method;
+                struct {
+                        const char *member;
+                        const char *signature;
+                } signal;
+                struct {
+                        const char *member;
+                        const char *signature;
+                        sd_bus_property_get_t get;
+                        sd_bus_property_set_t set;
+                        size_t offset;
+                } property;
+        } x;
+};
+/* Structure size up to v241 */
+#define VTABLE_ELEMENT_SIZE_ORIGINAL sizeof(struct sd_bus_vtable_original)
+/* Current structure size */
+#define VTABLE_ELEMENT_SIZE sizeof(struct sd_bus_vtable)
+
+static int vtable_features(const sd_bus_vtable *vtable) {
+        if (vtable[0].x.start.element_size == VTABLE_ELEMENT_SIZE_ORIGINAL)
+                return 0;
+        return vtable[0].x.start.features;
+}
+
+bool bus_vtable_has_names(const sd_bus_vtable *vtable) {
+        return vtable_features(vtable) & _SD_BUS_VTABLE_PARAM_NAMES;
+}
+
+const sd_bus_vtable* bus_vtable_next(const sd_bus_vtable *vtable, const sd_bus_vtable *v) {
+        if (vtable[0].x.start.element_size == VTABLE_ELEMENT_SIZE_ORIGINAL) {
+                const struct sd_bus_vtable_original *v2 = (const struct sd_bus_vtable_original *)v;
+                v2++;
+                v = (const sd_bus_vtable*)v2;
+        } else /* current version */
+                v++;
+        return v;
+}
+
 static int add_object_vtable_internal(
                 sd_bus *bus,
                 sd_bus_slot **slot,
@@ -1625,6 +1719,8 @@ static int add_object_vtable_internal(
         const sd_bus_vtable *v;
         struct node *n;
         int r;
+        const char *names = "";
+        names_flags nf;
 
         assert_return(bus, -EINVAL);
         assert_return(bus = bus_resolve(bus), -ENOPKG);
@@ -1632,7 +1728,7 @@ static int add_object_vtable_internal(
         assert_return(interface_name_is_valid(interface), -EINVAL);
         assert_return(vtable, -EINVAL);
         assert_return(vtable[0].type == _SD_BUS_VTABLE_START, -EINVAL);
-        assert_return(vtable[0].x.start.element_size == sizeof(struct sd_bus_vtable), -EINVAL);
+        assert_return(IN_SET(vtable[0].x.start.element_size, VTABLE_ELEMENT_SIZE_ORIGINAL, VTABLE_ELEMENT_SIZE), -EINVAL);
         assert_return(!bus_pid_changed(bus), -ECHILD);
         assert_return(!streq(interface, "org.freedesktop.DBus.Properties") &&
                       !streq(interface, "org.freedesktop.DBus.Introspectable") &&
@@ -1684,16 +1780,23 @@ static int add_object_vtable_internal(
                 goto fail;
         }
 
-        for (v = s->node_vtable.vtable+1; v->type != _SD_BUS_VTABLE_END; v++) {
+        v = s->node_vtable.vtable;
+        for (v = bus_vtable_next(vtable, v); v->type != _SD_BUS_VTABLE_END; v = bus_vtable_next(vtable, v)) {
 
                 switch (v->type) {
 
                 case _SD_BUS_VTABLE_METHOD: {
                         struct vtable_member *m;
+                        nf = NAMES_FIRST_PART;
+
+                        if (bus_vtable_has_names(vtable))
+                                names = strempty(v->x.method.names);
 
                         if (!member_name_is_valid(v->x.method.member) ||
                             !signature_is_valid(strempty(v->x.method.signature), false) ||
                             !signature_is_valid(strempty(v->x.method.result), false) ||
+                            !names_are_valid(strempty(v->x.method.signature), &names, &nf) ||
+                            !names_are_valid(strempty(v->x.method.result), &names, &nf) ||
                             !(v->x.method.handler || (isempty(v->x.method.signature) && isempty(v->x.method.result))) ||
                             v->flags & (SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE|SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION)) {
                                 r = -EINVAL;
@@ -1770,9 +1873,14 @@ static int add_object_vtable_internal(
                 }
 
                 case _SD_BUS_VTABLE_SIGNAL:
+                        nf = NAMES_SINGLE_PART;
+
+                        if (bus_vtable_has_names(vtable))
+                                names = strempty(v->x.signal.names);
 
                         if (!member_name_is_valid(v->x.signal.member) ||
                             !signature_is_valid(strempty(v->x.signal.signature), false) ||
+                            !names_are_valid(strempty(v->x.signal.signature), &names, &nf) ||
                             v->flags & SD_BUS_VTABLE_UNPRIVILEGED) {
                                 r = -EINVAL;
                                 goto fail;
@@ -1977,7 +2085,8 @@ static int emit_properties_changed_on_interface(
                          * we include all properties that are marked
                          * as changing in the message. */
 
-                        for (v = c->vtable+1; v->type != _SD_BUS_VTABLE_END; v++) {
+                        v = c->vtable;
+                        for (v = bus_vtable_next(c->vtable, v); v->type != _SD_BUS_VTABLE_END; v = bus_vtable_next(c->vtable, v)) {
                                 if (!IN_SET(v->type, _SD_BUS_VTABLE_PROPERTY, _SD_BUS_VTABLE_WRITABLE_PROPERTY))
                                         continue;
 
@@ -2048,7 +2157,8 @@ static int emit_properties_changed_on_interface(
                         } else {
                                 const sd_bus_vtable *v;
 
-                                for (v = c->vtable+1; v->type != _SD_BUS_VTABLE_END; v++) {
+                                v = c->vtable;
+                                for (v = bus_vtable_next(c->vtable, v); v->type != _SD_BUS_VTABLE_END; v = bus_vtable_next(c->vtable, v)) {
                                         if (!IN_SET(v->type, _SD_BUS_VTABLE_PROPERTY, _SD_BUS_VTABLE_WRITABLE_PROPERTY))
                                                 continue;
 
index a119ff95c09701d80445a83fffca1ca361ea77d8..b45fe6323efc08e66472ee7417ac91b9a8d5c5d2 100644 (file)
@@ -3,5 +3,7 @@
 
 #include "bus-internal.h"
 
+const sd_bus_vtable* bus_vtable_next(const sd_bus_vtable *vtable, const sd_bus_vtable *v);
+bool bus_vtable_has_names(const sd_bus_vtable *vtable);
 int bus_process_object(sd_bus *bus, sd_bus_message *m);
 void bus_node_gc(sd_bus *b, struct node *n);
index c9aca07f9005b6086a044726d5b66e70b7fc0fa8..f90a7f05ccf7fb0d9a94af914b3e62d0ba23c977 100644 (file)
@@ -117,7 +117,7 @@ void bus_slot_disconnect(sd_bus_slot *slot, bool unref) {
                 if (slot->node_vtable.node && slot->node_vtable.interface && slot->node_vtable.vtable) {
                         const sd_bus_vtable *v;
 
-                        for (v = slot->node_vtable.vtable; v->type != _SD_BUS_VTABLE_END; v++) {
+                        for (v = slot->node_vtable.vtable; v->type != _SD_BUS_VTABLE_END; v = bus_vtable_next(slot->node_vtable.vtable, v)) {
                                 struct vtable_member *x = NULL;
 
                                 switch (v->type) {
index fd9ad8121742faf55c97eb31d239e7fe4969005b..b278094fadd0fc4be19c15e26164fad4fb9e38b1 100644 (file)
@@ -39,6 +39,10 @@ static const sd_bus_vtable vtable[] = {
         SD_BUS_METHOD("Exit", "", "", handler, 0),
         SD_BUS_METHOD_WITH_OFFSET("AlterSomething2", "s", "s", handler, 200, 0),
         SD_BUS_METHOD_WITH_OFFSET("Exit2", "", "", handler, 200, 0),
+        SD_BUS_METHOD_WITH_NAMES_OFFSET("AlterSomething3", "so", SD_BUS_PARAM(string) SD_BUS_PARAM(path),
+                        "s", SD_BUS_PARAM(returnstring), handler, 200, 0),
+        SD_BUS_METHOD_WITH_NAMES("Exit3", "bx", SD_BUS_PARAM(with_confirmation) SD_BUS_PARAM(after_msec),
+                        "bb", SD_BUS_PARAM(accepted) SD_BUS_PARAM(scheduled), handler, 0),
         SD_BUS_PROPERTY("Value", "s", value_handler, 10, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
         SD_BUS_PROPERTY("Value2", "s", value_handler, 10, SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION),
         SD_BUS_PROPERTY("Value3", "s", value_handler, 10, SD_BUS_VTABLE_PROPERTY_CONST),
@@ -53,9 +57,68 @@ static const sd_bus_vtable vtable[] = {
         SD_BUS_METHOD("NoOperation", NULL, NULL, NULL, 0),
         SD_BUS_SIGNAL("DummySignal", "b", 0),
         SD_BUS_SIGNAL("DummySignal2", "so", 0),
+        SD_BUS_SIGNAL_WITH_NAMES("DummySignal3", "so", SD_BUS_PARAM(string) SD_BUS_PARAM(path), 0),
         SD_BUS_VTABLE_END
 };
 
+struct sd_bus_vtable_original {
+        uint8_t type:8;
+        uint64_t flags:56;
+        union {
+                struct {
+                        size_t element_size;
+                } start;
+                struct {
+                        const char *member;
+                        const char *signature;
+                        const char *result;
+                        sd_bus_message_handler_t handler;
+                        size_t offset;
+                } method;
+                struct {
+                        const char *member;
+                        const char *signature;
+                } signal;
+                struct {
+                        const char *member;
+                        const char *signature;
+                        sd_bus_property_get_t get;
+                        sd_bus_property_set_t set;
+                        size_t offset;
+                } property;
+        } x;
+};
+
+static const struct sd_bus_vtable_original vtable_format_original[] = {
+        {
+                .type = _SD_BUS_VTABLE_START,
+                .flags = 0,
+                .x = {
+                        .start = {
+                                .element_size = sizeof(struct sd_bus_vtable_original)
+                        },
+                },
+        },
+        {
+                .type = _SD_BUS_VTABLE_METHOD,
+                .flags = 0,
+                .x = {
+                        .method = {
+                                .member = "Exit",
+                                .signature = "",
+                                .result = "",
+                                .handler = handler,
+                                .offset = 0,
+                        },
+                },
+        },
+        {
+                .type = _SD_BUS_VTABLE_END,
+                .flags = 0,
+                .x = { { 0 } },
+        }
+};
+
 static void test_vtable(void) {
         sd_bus *bus = NULL;
         struct context c = {};
@@ -65,6 +128,8 @@ static void test_vtable(void) {
 
         assert(sd_bus_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.testVtable", vtable, &c) >= 0);
         assert(sd_bus_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.testVtable2", vtable, &c) >= 0);
+        /* the cast on the line below is needed to test with the old version of the table */
+        assert(sd_bus_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.testVtableOriginal", (const sd_bus_vtable *)vtable_format_original, &c) >= 0);
 
         assert(sd_bus_set_address(bus, DEFAULT_BUS_PATH) >= 0);
         r = sd_bus_start(bus);
index 4350a8c7057155c0c3da55d3050bac4f73f01eb3..8a73ef0503cc97454be577e408908d7bdcd2758a 100644 (file)
@@ -48,6 +48,10 @@ enum {
 
 #define SD_BUS_VTABLE_CAPABILITY(x) ((uint64_t) (((x)+1) & 0xFFFF) << 40)
 
+enum {
+        _SD_BUS_VTABLE_PARAM_NAMES     = 1 << 0,
+};
+
 struct sd_bus_vtable {
         /* Please do not initialize this structure directly, use the
          * macros below instead */
@@ -57,6 +61,7 @@ struct sd_bus_vtable {
         union {
                 struct {
                         size_t element_size;
+                        uint64_t features;
                 } start;
                 struct {
                         const char *member;
@@ -64,10 +69,12 @@ struct sd_bus_vtable {
                         const char *result;
                         sd_bus_message_handler_t handler;
                         size_t offset;
+                        const char *names;
                 } method;
                 struct {
                         const char *member;
                         const char *signature;
+                        const char *names;
                 } signal;
                 struct {
                         const char *member;
@@ -85,12 +92,16 @@ struct sd_bus_vtable {
                 .flags = _flags,                                        \
                 .x = {                                                  \
                     .start = {                                          \
-                        .element_size = sizeof(sd_bus_vtable)           \
+                        .element_size = sizeof(sd_bus_vtable),          \
+                        .features = _SD_BUS_VTABLE_PARAM_NAMES          \
                     },                                                  \
                 },                                                      \
         }
 
-#define SD_BUS_METHOD_WITH_OFFSET(_member, _signature, _result, _handler, _offset, _flags)   \
+/* helper macro to format method and signal parameters, one at a time */
+#define SD_BUS_PARAM(x) #x "\0"
+
+#define SD_BUS_METHOD_WITH_NAMES_OFFSET(_member, _signature, _in_names, _result, _out_names, _handler, _offset, _flags)  \
         {                                                               \
                 .type = _SD_BUS_VTABLE_METHOD,                          \
                 .flags = _flags,                                        \
@@ -101,13 +112,18 @@ struct sd_bus_vtable {
                         .result = _result,                              \
                         .handler = _handler,                            \
                         .offset = _offset,                              \
+                        .names = _in_names _out_names,                  \
                     },                                                  \
                 },                                                      \
         }
+#define SD_BUS_METHOD_WITH_OFFSET(_member, _signature, _result, _handler, _offset, _flags)   \
+        SD_BUS_METHOD_WITH_NAMES_OFFSET(_member, _signature, "", _result, "", _handler, _offset, _flags)
+#define SD_BUS_METHOD_WITH_NAMES(_member, _signature, _in_names, _result, _out_names, _handler, _flags)   \
+        SD_BUS_METHOD_WITH_NAMES_OFFSET(_member, _signature, _in_names, _result, _out_names, _handler, 0, _flags)
 #define SD_BUS_METHOD(_member, _signature, _result, _handler, _flags)   \
-        SD_BUS_METHOD_WITH_OFFSET(_member, _signature, _result, _handler, 0, _flags)
+        SD_BUS_METHOD_WITH_NAMES_OFFSET(_member, _signature, "", _result, "", _handler, 0, _flags)
 
-#define SD_BUS_SIGNAL(_member, _signature, _flags)                      \
+#define SD_BUS_SIGNAL_WITH_NAMES(_member, _signature, _out_names, _flags)                      \
         {                                                               \
                 .type = _SD_BUS_VTABLE_SIGNAL,                          \
                 .flags = _flags,                                        \
@@ -115,9 +131,12 @@ struct sd_bus_vtable {
                     .signal = {                                         \
                         .member = _member,                              \
                         .signature = _signature,                        \
+                        .names = _out_names,                            \
                     },                                                  \
                 },                                                      \
         }
+#define SD_BUS_SIGNAL(_member, _signature, _flags)   \
+        SD_BUS_SIGNAL_WITH_NAMES(_member, _signature, "", _flags)
 
 #define SD_BUS_PROPERTY(_member, _signature, _get, _offset, _flags)     \
         {                                                               \