From 856ad2a86bd9b3e264a090fcf4b0d05bfaa91030 Mon Sep 17 00:00:00 2001 From: Giacinto Cifelli Date: Tue, 8 Jan 2019 12:14:37 +0100 Subject: [PATCH] sd-bus: add methods and signals parameter names. Fixes: #1564 --- src/libsystemd/sd-bus/bus-introspect.c | 25 ++++- src/libsystemd/sd-bus/bus-objects.c | 120 +++++++++++++++++++++++- src/libsystemd/sd-bus/bus-objects.h | 2 + src/libsystemd/sd-bus/bus-slot.c | 2 +- src/libsystemd/sd-bus/test-bus-vtable.c | 65 +++++++++++++ src/systemd/sd-bus-vtable.h | 27 +++++- 6 files changed, 226 insertions(+), 15 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-introspect.c b/src/libsystemd/sd-bus/bus-introspect.c index f623dd9ce0..bde97483e8 100644 --- a/src/libsystemd/sd-bus/bus-introspect.c +++ b/src/libsystemd/sd-bus/bus-introspect.c @@ -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(" \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, " 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, " \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(" \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, " \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(" \n", i->f); break; diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index d0538104ae..7ff4c3f935 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -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; diff --git a/src/libsystemd/sd-bus/bus-objects.h b/src/libsystemd/sd-bus/bus-objects.h index a119ff95c0..b45fe6323e 100644 --- a/src/libsystemd/sd-bus/bus-objects.h +++ b/src/libsystemd/sd-bus/bus-objects.h @@ -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); diff --git a/src/libsystemd/sd-bus/bus-slot.c b/src/libsystemd/sd-bus/bus-slot.c index c9aca07f90..f90a7f05cc 100644 --- a/src/libsystemd/sd-bus/bus-slot.c +++ b/src/libsystemd/sd-bus/bus-slot.c @@ -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) { diff --git a/src/libsystemd/sd-bus/test-bus-vtable.c b/src/libsystemd/sd-bus/test-bus-vtable.c index fd9ad81217..b278094fad 100644 --- a/src/libsystemd/sd-bus/test-bus-vtable.c +++ b/src/libsystemd/sd-bus/test-bus-vtable.c @@ -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); diff --git a/src/systemd/sd-bus-vtable.h b/src/systemd/sd-bus-vtable.h index 4350a8c705..8a73ef0503 100644 --- a/src/systemd/sd-bus-vtable.h +++ b/src/systemd/sd-bus-vtable.h @@ -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) \ { \