From 8dd8a286d1d2770a22116d4eb730b2c3f5fa28a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 18 Apr 2019 13:06:41 +0200 Subject: [PATCH 1/9] sd-bus: add symbol to tell linker that new vtable functions are used In 856ad2a86bd9b3e264a090fcf4b0d05bfaa91030 sd_bus_add_object_vtable() and sd_bus_add_fallback_vtable() were changed to take an updated sd_bus_vtable[] array with additional 'features' and 'names' fields in the union. The commit tried to check whether the old or the new table format is used, by looking at the vtable[0].x.start.element_size field, on the assumption that the added fields caused the structure size to grow. Unfortunately, this assumption was false, and on arm32 (at least), the structure size is unchanged. In libsystemd we use symbol versioning and a major.minor.patch semantic versioning of the library name (major equals the number in the so-name). When systemd-242 was released, the minor number was (correctly) bumped, but this is not enough, because no new symbols were added or symbol versions changed. This means that programs compiled with the new systemd headers and library could be successfully linked to older versions of the library. For example rpm only looks at the so-name and the list of versioned symbols, completely ignoring the major.minor numbers in the library name. But the older library does not understand the new vtable format, and would return -EINVAL after failing the size check (on those architectures where the structure size did change, i.e. all 64 bit architectures). To force new libsystemd (with the functions that take the updated sd_bus_vtable[] format) to be used, let's pull in a dummy symbol from the table definition. This is a bit wasteful, because a dummy pointer has to be stored, but the effect is negligible. In particular, the pointer doesn't even change the size of the structure because if fits in an unused area in the union. The number stored in the new unsigned integer is not checked anywhere. If the symbol exists, we already know we have the new version of the library, so an additional check would not tell us anything. An alternative would be to make sd_bus_add_{object,fallback}_vtable() versioned symbols, using .symver linker annotations. We would provide sd_bus_add_{object,fallback}_vtable@LIBSYSTEMD_221 (for backwards compatibility) and e.g. sd_bus_add_{object,fallback}_vtable@@LIBSYSTEMD_242 (the default) with the new implementation. This would work too, but is more work. We would have to version at least those two functions. And it turns out that the .symver linker instructions have to located in the same compilation unit as the function being annotated. We first compile libsystemd.a, and then link it into libsystemd.so and various other targets, including libsystemd-shared.so, and the nss modules. If the .symver annotations were placed next to the function definitions (in bus-object.c), they would influence all targets that link libsystemd.a, and cause problems, because those functions should not be exported there. To export them only in libsystemd.so, compilation would have to be rearranged, so that the functions exported in libsystemd.so would not be present in libsystemd.a, but a separate compilation unit containg them and the .symver annotations would be linked solely into libsystemd.so. This is certainly possible, but more work than the approach in this patch. 856ad2a86bd9b3e264a090fcf4b0d05bfaa91030 has one more issue: it relies on the undefined fields in sd_bus_vtable[] array to be zeros. But the structure contains a union, and fields of the union do not have to be zero-initalized by the compiler. This means that potentially, we could have garbarge values there, for example when reading the old vtable format definition from the new function implementation. In practice this should not be an issue at all, because vtable definitions are static data and are placed in the ro-data section, which is fully initalized, so we know that those undefined areas will be zero. Things would be different if somebody defined the vtable array on the heap or on the stack. Let's just document that they should zero-intialize the unused areas in this case. The symbol checking code had to be updated because otherwise gcc warns about a cast from unsigned to a pointer. --- src/libsystemd/libsystemd.sym | 5 +++++ src/libsystemd/sd-bus/bus-objects.c | 6 +++++- src/systemd/sd-bus-vtable.h | 13 ++++++++++++- src/test/generate-sym-test.py | 12 ++++++++---- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index a6748ceb20..a9ab0605ce 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -676,3 +676,8 @@ LIBSYSTEMD_241 { global: sd_bus_close_unref; } LIBSYSTEMD_240; + +LIBSYSTEMD_243 { +global: + sd_bus_object_vtable_format; +} LIBSYSTEMD_241; diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index d9fc25605a..ce2cb94bde 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -1701,7 +1701,8 @@ struct sd_bus_vtable_original { #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) + if (vtable[0].x.start.element_size < VTABLE_ELEMENT_SIZE || + !vtable[0].x.start.vtable_format_reference) return 0; return vtable[0].x.start.features; } @@ -1928,6 +1929,9 @@ fail: return r; } +/* This symbol exists solely to tell the linker that the "new" vtable format is used. */ +_public_ const unsigned sd_bus_object_vtable_format = 242; + _public_ int sd_bus_add_object_vtable( sd_bus *bus, sd_bus_slot **slot, diff --git a/src/systemd/sd-bus-vtable.h b/src/systemd/sd-bus-vtable.h index 8a73ef0503..e3804e203c 100644 --- a/src/systemd/sd-bus-vtable.h +++ b/src/systemd/sd-bus-vtable.h @@ -52,6 +52,15 @@ enum { _SD_BUS_VTABLE_PARAM_NAMES = 1 << 0, }; +extern const unsigned sd_bus_object_vtable_format; + +/* Note: unused areas in the sd_bus_vtable[] array must be initalized to 0. The stucture contains an embedded + * union, and the compiler is NOT required to initalize the unused areas of the union when the rest of the + * structure is initalized. Normally the array is defined as read-only data, in which case the linker places + * it in the BSS section, which is always fully initalized, so this is not a concern. But if the array is + * created on the stack or on the heap, care must be taken to initalize the unused areas, for examply by + * first memsetting the whole region to zero before filling the data in. */ + struct sd_bus_vtable { /* Please do not initialize this structure directly, use the * macros below instead */ @@ -62,6 +71,7 @@ struct sd_bus_vtable { struct { size_t element_size; uint64_t features; + const unsigned *vtable_format_reference; } start; struct { const char *member; @@ -93,7 +103,8 @@ struct sd_bus_vtable { .x = { \ .start = { \ .element_size = sizeof(sd_bus_vtable), \ - .features = _SD_BUS_VTABLE_PARAM_NAMES \ + .features = _SD_BUS_VTABLE_PARAM_NAMES, \ + .vtable_format_reference = &sd_bus_object_vtable_format, \ }, \ }, \ } diff --git a/src/test/generate-sym-test.py b/src/test/generate-sym-test.py index 357cce8e44..2510809452 100755 --- a/src/test/generate-sym-test.py +++ b/src/test/generate-sym-test.py @@ -6,18 +6,22 @@ for header in sys.argv[2:]: print('#include "{}"'.format(header.split('/')[-1])) print(''' -void* functions[] = {''') +const void* symbols[] = {''') for line in open(sys.argv[1]): match = re.search('^ +([a-zA-Z0-9_]+);', line) if match: - print(' {},'.format(match.group(1))) + s = match.group(1) + if s == 'sd_bus_object_vtable_format': + print(f' &{s},') + else: + print(f' {s},') print('''}; int main(void) { unsigned i; - for (i = 0; i < sizeof(functions)/sizeof(void*); i++) - printf("%p\\n", functions[i]); + for (i = 0; i < sizeof(symbols)/sizeof(void*); i++) + printf("%p\\n", symbols[i]); return 0; }''') From 2caef9fba40069374f7312a7861f2f4ca04edd0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 18 Apr 2019 13:42:25 +0200 Subject: [PATCH 2/9] sd-bus: allow vtable format structure to grow in the future We would check the size of sd_bus_vtable entries, requring one of the two known sizes. But we should be able to extend the structure in the future, by adding new fields, without breaking backwards compatiblity. Incidentally, this check was what caused -EINVAL failures before, when programs were compiled with systemd-242 and run with older libsystemd. --- src/libsystemd/sd-bus/bus-objects.c | 25 +++++++++++-------------- src/libsystemd/sd-bus/test-bus-vtable.c | 8 ++++---- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index ce2cb94bde..7053471d6d 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -1668,7 +1668,7 @@ static bool names_are_valid(const char *signature, const char **names, names_fla /* 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 { +struct sd_bus_vtable_221 { uint8_t type:8; uint64_t flags:56; union { @@ -1696,12 +1696,15 @@ struct sd_bus_vtable_original { } 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) +#define VTABLE_ELEMENT_SIZE_221 sizeof(struct sd_bus_vtable_221) + +/* Size of the structure when "features" field was added. If the structure definition is augmented, a copy of + * the structure definition will need to be made (similarly to the sd_bus_vtable_221 above), and this + * definition updated to refer to it. */ +#define VTABLE_ELEMENT_SIZE_242 sizeof(struct sd_bus_vtable) static int vtable_features(const sd_bus_vtable *vtable) { - if (vtable[0].x.start.element_size < VTABLE_ELEMENT_SIZE || + if (vtable[0].x.start.element_size < VTABLE_ELEMENT_SIZE_242 || !vtable[0].x.start.vtable_format_reference) return 0; return vtable[0].x.start.features; @@ -1712,13 +1715,7 @@ bool bus_vtable_has_names(const sd_bus_vtable *vtable) { } 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; + return (const sd_bus_vtable*) ((char*) v + vtable[0].x.start.element_size); } static int add_object_vtable_internal( @@ -1745,8 +1742,8 @@ 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 == VTABLE_ELEMENT_SIZE_ORIGINAL || - vtable[0].x.start.element_size == VTABLE_ELEMENT_SIZE, + assert_return(vtable[0].x.start.element_size == VTABLE_ELEMENT_SIZE_221 || + vtable[0].x.start.element_size >= VTABLE_ELEMENT_SIZE_242, -EINVAL); assert_return(!bus_pid_changed(bus), -ECHILD); assert_return(!streq(interface, "org.freedesktop.DBus.Properties") && diff --git a/src/libsystemd/sd-bus/test-bus-vtable.c b/src/libsystemd/sd-bus/test-bus-vtable.c index b278094fad..582b050fea 100644 --- a/src/libsystemd/sd-bus/test-bus-vtable.c +++ b/src/libsystemd/sd-bus/test-bus-vtable.c @@ -61,7 +61,7 @@ static const sd_bus_vtable vtable[] = { SD_BUS_VTABLE_END }; -struct sd_bus_vtable_original { +struct sd_bus_vtable_221 { uint8_t type:8; uint64_t flags:56; union { @@ -89,13 +89,13 @@ struct sd_bus_vtable_original { } x; }; -static const struct sd_bus_vtable_original vtable_format_original[] = { +static const struct sd_bus_vtable_221 vtable_format_221[] = { { .type = _SD_BUS_VTABLE_START, .flags = 0, .x = { .start = { - .element_size = sizeof(struct sd_bus_vtable_original) + .element_size = sizeof(struct sd_bus_vtable_221) }, }, }, @@ -129,7 +129,7 @@ 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_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.testVtable221", (const sd_bus_vtable *)vtable_format_221, &c) >= 0); assert(sd_bus_set_address(bus, DEFAULT_BUS_PATH) >= 0); r = sd_bus_start(bus); From 2abda6d1e4a4f18a6033f865cf83d914a3f27e9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Apr 2019 11:28:36 +0200 Subject: [PATCH 3/9] sd-bus: use _cleanup_ for struct introspect --- src/libsystemd/sd-bus/bus-objects.c | 41 +++++++-------------- src/libsystemd/sd-bus/test-bus-introspect.c | 4 +- 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index 7053471d6d..650cee63af 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -894,7 +894,7 @@ static int process_introspect( _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_set_free_free_ Set *s = NULL; const char *previous_interface = NULL; - struct introspect intro; + _cleanup_(introspect_free) struct introspect intro = {}; struct node_vtable *c; bool empty; int r; @@ -925,14 +925,10 @@ static int process_introspect( continue; r = node_vtable_get_userdata(bus, m->path, c, NULL, &error); - if (r < 0) { - r = bus_maybe_reply_error(m, r, &error); - goto finish; - } - if (bus->nodes_modified) { - r = 0; - goto finish; - } + if (r < 0) + return bus_maybe_reply_error(m, r, &error); + if (bus->nodes_modified) + return 0; if (r == 0) continue; @@ -942,7 +938,6 @@ static int process_introspect( continue; if (!streq_ptr(previous_interface, c->interface)) { - if (previous_interface) fputs(" \n", intro.f); @@ -951,7 +946,7 @@ static int process_introspect( r = introspect_write_interface(&intro, c->vtable); if (r < 0) - goto finish; + return r; previous_interface = c->interface; } @@ -963,35 +958,27 @@ static int process_introspect( /* Nothing?, let's see if we exist at all, and if not * refuse to do anything */ r = bus_node_exists(bus, n, m->path, require_fallback); - if (r <= 0) { - r = bus_maybe_reply_error(m, r, &error); - goto finish; - } - if (bus->nodes_modified) { - r = 0; - goto finish; - } + if (r <= 0) + return bus_maybe_reply_error(m, r, &error); + if (bus->nodes_modified) + return 0; } *found_object = true; r = introspect_write_child_nodes(&intro, s, m->path); if (r < 0) - goto finish; + return r; r = introspect_finish(&intro, bus, m, &reply); if (r < 0) - goto finish; + return r; r = sd_bus_send(bus, reply, NULL); if (r < 0) - goto finish; + return r; - r = 1; - -finish: - introspect_free(&intro); - return r; + return 1; } static int object_manager_serialize_path( diff --git a/src/libsystemd/sd-bus/test-bus-introspect.c b/src/libsystemd/sd-bus/test-bus-introspect.c index 9c8e93e897..968de40e5a 100644 --- a/src/libsystemd/sd-bus/test-bus-introspect.c +++ b/src/libsystemd/sd-bus/test-bus-introspect.c @@ -28,7 +28,7 @@ static const sd_bus_vtable vtable[] = { }; int main(int argc, char *argv[]) { - struct introspect intro; + _cleanup_(introspect_free) struct introspect intro = {}; test_setup_logging(LOG_DEBUG); @@ -41,7 +41,5 @@ int main(int argc, char *argv[]) { fflush(intro.f); fputs(intro.introspection, stdout); - introspect_free(&intro); - return 0; } From dff9e25a76debcc63a70c9d02bcf77485da6b305 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Apr 2019 12:14:35 +0200 Subject: [PATCH 4/9] sd-bus: split introspection into the content creation and reply creation parts Just moving code around, in preparation to allow the content creation part to be used in other places. On the surface of things, introspect_path() should be in bus-introspect.c, but introspect_path() uses many static helper functions in bus-objects.c, so moving it would require all of them to be exposed, which is too much trouble. test-bus-introspect is updated to actually write the closing bracket. --- src/libsystemd/sd-bus/bus-introspect.c | 21 ++----- src/libsystemd/sd-bus/bus-introspect.h | 2 +- src/libsystemd/sd-bus/bus-objects.c | 66 +++++++++++++++------ src/libsystemd/sd-bus/bus-objects.h | 10 ++++ src/libsystemd/sd-bus/test-bus-introspect.c | 17 ++++-- 5 files changed, 75 insertions(+), 41 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-introspect.c b/src/libsystemd/sd-bus/bus-introspect.c index 022eddb10f..beab80687d 100644 --- a/src/libsystemd/sd-bus/bus-introspect.c +++ b/src/libsystemd/sd-bus/bus-introspect.c @@ -172,13 +172,10 @@ int introspect_write_interface(struct introspect *i, const sd_bus_vtable *v) { return 0; } -int introspect_finish(struct introspect *i, sd_bus *bus, sd_bus_message *m, sd_bus_message **reply) { - sd_bus_message *q; +int introspect_finish(struct introspect *i, char **ret) { int r; assert(i); - assert(m); - assert(reply); fputs("\n", i->f); @@ -186,25 +183,17 @@ int introspect_finish(struct introspect *i, sd_bus *bus, sd_bus_message *m, sd_b if (r < 0) return r; - r = sd_bus_message_new_method_return(m, &q); - if (r < 0) - return r; + i->f = safe_fclose(i->f); + *ret = TAKE_PTR(i->introspection); - r = sd_bus_message_append(q, "s", i->introspection); - if (r < 0) { - sd_bus_message_unref(q); - return r; - } - - *reply = q; return 0; } void introspect_free(struct introspect *i) { assert(i); - safe_fclose(i->f); + /* Normally introspect_finish() does all the work, this is just a backup for error paths */ + safe_fclose(i->f); free(i->introspection); - zero(*i); } diff --git a/src/libsystemd/sd-bus/bus-introspect.h b/src/libsystemd/sd-bus/bus-introspect.h index bb2dd7ef7b..ccbb543d0c 100644 --- a/src/libsystemd/sd-bus/bus-introspect.h +++ b/src/libsystemd/sd-bus/bus-introspect.h @@ -18,5 +18,5 @@ int introspect_begin(struct introspect *i, bool trusted); int introspect_write_default_interfaces(struct introspect *i, bool object_manager); int introspect_write_child_nodes(struct introspect *i, Set *s, const char *prefix); int introspect_write_interface(struct introspect *i, const sd_bus_vtable *v); -int introspect_finish(struct introspect *i, sd_bus *bus, sd_bus_message *m, sd_bus_message **reply); +int introspect_finish(struct introspect *i, char **ret); void introspect_free(struct introspect *i); diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index 650cee63af..22db75479c 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -883,15 +883,15 @@ static int bus_node_exists( return 0; } -static int process_introspect( +int introspect_path( sd_bus *bus, - sd_bus_message *m, + const char *path, struct node *n, bool require_fallback, - bool *found_object) { + bool *found_object, + char **ret, + sd_bus_error *error) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_set_free_free_ Set *s = NULL; const char *previous_interface = NULL; _cleanup_(introspect_free) struct introspect intro = {}; @@ -899,14 +899,9 @@ static int process_introspect( bool empty; int r; - assert(bus); - assert(m); - assert(n); - assert(found_object); - - r = get_child_nodes(bus, m->path, n, 0, &s, &error); + r = get_child_nodes(bus, path, n, 0, &s, error); if (r < 0) - return bus_maybe_reply_error(m, r, &error); + return r; if (bus->nodes_modified) return 0; @@ -924,9 +919,9 @@ static int process_introspect( if (require_fallback && !c->is_fallback) continue; - r = node_vtable_get_userdata(bus, m->path, c, NULL, &error); + r = node_vtable_get_userdata(bus, path, c, NULL, error); if (r < 0) - return bus_maybe_reply_error(m, r, &error); + return r; if (bus->nodes_modified) return 0; if (r == 0) @@ -957,20 +952,55 @@ static int process_introspect( if (empty) { /* Nothing?, let's see if we exist at all, and if not * refuse to do anything */ - r = bus_node_exists(bus, n, m->path, require_fallback); + r = bus_node_exists(bus, n, path, require_fallback); if (r <= 0) - return bus_maybe_reply_error(m, r, &error); + return r; if (bus->nodes_modified) return 0; } *found_object = true; - r = introspect_write_child_nodes(&intro, s, m->path); + r = introspect_write_child_nodes(&intro, s, path); if (r < 0) return r; - r = introspect_finish(&intro, bus, m, &reply); + r = introspect_finish(&intro, ret); + if (r < 0) + return r; + + return 1; +} + +static int process_introspect( + sd_bus *bus, + sd_bus_message *m, + struct node *n, + bool require_fallback, + bool *found_object) { + + _cleanup_free_ char *s = NULL; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + int r; + + assert(bus); + assert(m); + assert(n); + assert(found_object); + + r = introspect_path(bus, m->path, n, require_fallback, found_object, &s, &error); + if (r < 0) + return bus_maybe_reply_error(m, r, &error); + if (r == 0) + /* nodes_modified == true */ + return 0; + + r = sd_bus_message_new_method_return(m, &reply); + if (r < 0) + return r; + + r = sd_bus_message_append(reply, "s", s); if (r < 0) return r; diff --git a/src/libsystemd/sd-bus/bus-objects.h b/src/libsystemd/sd-bus/bus-objects.h index b45fe6323e..7f38853da1 100644 --- a/src/libsystemd/sd-bus/bus-objects.h +++ b/src/libsystemd/sd-bus/bus-objects.h @@ -2,8 +2,18 @@ #pragma once #include "bus-internal.h" +#include "bus-introspect.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); + +int introspect_path( + sd_bus *bus, + const char *path, + struct node *n, + bool require_fallback, + bool *found_object, + char **ret, + sd_bus_error *error); diff --git a/src/libsystemd/sd-bus/test-bus-introspect.c b/src/libsystemd/sd-bus/test-bus-introspect.c index 968de40e5a..797b19f9f1 100644 --- a/src/libsystemd/sd-bus/test-bus-introspect.c +++ b/src/libsystemd/sd-bus/test-bus-introspect.c @@ -27,10 +27,9 @@ static const sd_bus_vtable vtable[] = { SD_BUS_VTABLE_END }; -int main(int argc, char *argv[]) { - _cleanup_(introspect_free) struct introspect intro = {}; - - test_setup_logging(LOG_DEBUG); +static void test_manual_introspection(void) { + struct introspect intro = {}; + _cleanup_free_ char *s = NULL; assert_se(introspect_begin(&intro, false) >= 0); @@ -38,8 +37,14 @@ int main(int argc, char *argv[]) { assert_se(introspect_write_interface(&intro, vtable) >= 0); fputs(" \n", intro.f); - fflush(intro.f); - fputs(intro.introspection, stdout); + assert_se(introspect_finish(&intro, &s) == 0); + fputs(s, stdout); +} + +int main(int argc, char *argv[]) { + test_setup_logging(LOG_DEBUG); + + test_manual_introspection(); return 0; } From d603324b4b28cfdced6677fa8eb70f0a95b4af4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Apr 2019 13:30:09 +0200 Subject: [PATCH 5/9] test-bus-{vtable,introspect}: share data and test introspect_path() test-bus-introspect is also applied to the tables from test-bus-vtable.c. test-bus-vtable.c is also used as C++ sources to produce test-bus-vtable-cc, and our hashmap headers are not C++ compatible. So let's do the introspection part only in the C version. --- src/libsystemd/sd-bus/bus-objects.c | 18 ++- src/libsystemd/sd-bus/bus-objects.h | 1 + src/libsystemd/sd-bus/test-bus-introspect.c | 33 ++--- src/libsystemd/sd-bus/test-bus-vtable.c | 132 +++----------------- src/libsystemd/sd-bus/test-vtable-data.h | 132 ++++++++++++++++++++ src/test/meson.build | 6 +- 6 files changed, 179 insertions(+), 143 deletions(-) create mode 100644 src/libsystemd/sd-bus/test-vtable-data.h diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index 22db75479c..6ed7cc4860 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -888,6 +888,7 @@ int introspect_path( const char *path, struct node *n, bool require_fallback, + bool ignore_nodes_modified, bool *found_object, char **ret, sd_bus_error *error) { @@ -899,10 +900,16 @@ int introspect_path( bool empty; int r; + if (!n) { + n = hashmap_get(bus->nodes, path); + if (!n) + return -ENOENT; + } + r = get_child_nodes(bus, path, n, 0, &s, error); if (r < 0) return r; - if (bus->nodes_modified) + if (bus->nodes_modified && !ignore_nodes_modified) return 0; r = introspect_begin(&intro, bus->trusted); @@ -922,7 +929,7 @@ int introspect_path( r = node_vtable_get_userdata(bus, path, c, NULL, error); if (r < 0) return r; - if (bus->nodes_modified) + if (bus->nodes_modified && !ignore_nodes_modified) return 0; if (r == 0) continue; @@ -955,11 +962,12 @@ int introspect_path( r = bus_node_exists(bus, n, path, require_fallback); if (r <= 0) return r; - if (bus->nodes_modified) + if (bus->nodes_modified && !ignore_nodes_modified) return 0; } - *found_object = true; + if (found_object) + *found_object = true; r = introspect_write_child_nodes(&intro, s, path); if (r < 0) @@ -989,7 +997,7 @@ static int process_introspect( assert(n); assert(found_object); - r = introspect_path(bus, m->path, n, require_fallback, found_object, &s, &error); + r = introspect_path(bus, m->path, n, require_fallback, false, found_object, &s, &error); if (r < 0) return bus_maybe_reply_error(m, r, &error); if (r == 0) diff --git a/src/libsystemd/sd-bus/bus-objects.h b/src/libsystemd/sd-bus/bus-objects.h index 7f38853da1..f650196a54 100644 --- a/src/libsystemd/sd-bus/bus-objects.h +++ b/src/libsystemd/sd-bus/bus-objects.h @@ -14,6 +14,7 @@ int introspect_path( const char *path, struct node *n, bool require_fallback, + bool ignore_nodes_modified, bool *found_object, char **ret, sd_bus_error *error); diff --git a/src/libsystemd/sd-bus/test-bus-introspect.c b/src/libsystemd/sd-bus/test-bus-introspect.c index 797b19f9f1..9c8d1434b1 100644 --- a/src/libsystemd/sd-bus/test-bus-introspect.c +++ b/src/libsystemd/sd-bus/test-bus-introspect.c @@ -4,33 +4,14 @@ #include "log.h" #include "tests.h" -static int prop_get(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error) { - return -EINVAL; -} +#include "test-vtable-data.h" -static int prop_set(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error) { - return -EINVAL; -} - -static const sd_bus_vtable vtable[] = { - SD_BUS_VTABLE_START(0), - SD_BUS_METHOD("Hello", "ssas", "a(uu)", NULL, 0), - SD_BUS_METHOD("DeprecatedHello", "", "", NULL, SD_BUS_VTABLE_DEPRECATED), - SD_BUS_METHOD("DeprecatedHelloNoReply", "", "", NULL, SD_BUS_VTABLE_DEPRECATED|SD_BUS_VTABLE_METHOD_NO_REPLY), - SD_BUS_SIGNAL("Wowza", "sss", 0), - SD_BUS_SIGNAL("DeprecatedWowza", "ut", SD_BUS_VTABLE_DEPRECATED), - SD_BUS_WRITABLE_PROPERTY("AProperty", "s", prop_get, prop_set, 0, 0), - SD_BUS_PROPERTY("AReadOnlyDeprecatedProperty", "(ut)", prop_get, 0, SD_BUS_VTABLE_DEPRECATED), - SD_BUS_PROPERTY("ChangingProperty", "t", prop_get, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), - SD_BUS_PROPERTY("Invalidating", "t", prop_get, 0, SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), - SD_BUS_PROPERTY("Constant", "t", prop_get, 0, SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_PROPERTY_EXPLICIT), - SD_BUS_VTABLE_END -}; - -static void test_manual_introspection(void) { +static void test_manual_introspection(const sd_bus_vtable vtable[]) { struct introspect intro = {}; _cleanup_free_ char *s = NULL; + log_info("/* %s */", __func__); + assert_se(introspect_begin(&intro, false) >= 0); fprintf(intro.f, " \n"); @@ -39,12 +20,16 @@ static void test_manual_introspection(void) { assert_se(introspect_finish(&intro, &s) == 0); fputs(s, stdout); + fputs("\n", stdout); } int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); - test_manual_introspection(); + test_manual_introspection(test_vtable_1); + test_manual_introspection(test_vtable_2); + test_manual_introspection(test_vtable_deprecated); + test_manual_introspection((const sd_bus_vtable *) vtable_format_221); return 0; } diff --git a/src/libsystemd/sd-bus/test-bus-vtable.c b/src/libsystemd/sd-bus/test-bus-vtable.c index 582b050fea..d69ca6ac97 100644 --- a/src/libsystemd/sd-bus/test-bus-vtable.c +++ b/src/libsystemd/sd-bus/test-bus-vtable.c @@ -1,3 +1,5 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + #include #include @@ -5,120 +7,18 @@ #undef NDEBUG #include #include +#include #include "sd-bus-vtable.h" +#ifndef __cplusplus +# include "bus-objects.h" +#endif + +#include "test-vtable-data.h" + #define DEFAULT_BUS_PATH "unix:path=/run/dbus/system_bus_socket" -struct context { - bool quit; - char *something; - char *automatic_string_property; - uint32_t automatic_integer_property; -}; - -static int handler(sd_bus_message *m, void *userdata, sd_bus_error *error) { - return 1; -} - -static int value_handler(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error) { - return 1; -} - -static int get_handler(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error) { - return 1; -} - -static int set_handler(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *value, void *userdata, sd_bus_error *error) { - return 1; -} - -static const sd_bus_vtable vtable[] = { - SD_BUS_VTABLE_START(0), - SD_BUS_METHOD("AlterSomething", "s", "s", handler, 0), - 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), - SD_BUS_PROPERTY("Value4", "s", value_handler, 10, 0), - SD_BUS_PROPERTY("AnExplicitProperty", "s", NULL, offsetof(struct context, something), - SD_BUS_VTABLE_PROPERTY_EXPLICIT|SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), - SD_BUS_WRITABLE_PROPERTY("Something", "s", get_handler, set_handler, 0, 0), - SD_BUS_WRITABLE_PROPERTY("AutomaticStringProperty", "s", NULL, NULL, - offsetof(struct context, automatic_string_property), 0), - SD_BUS_WRITABLE_PROPERTY("AutomaticIntegerProperty", "u", NULL, NULL, - offsetof(struct context, automatic_integer_property), 0), - 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_221 { - 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_221 vtable_format_221[] = { - { - .type = _SD_BUS_VTABLE_START, - .flags = 0, - .x = { - .start = { - .element_size = sizeof(struct sd_bus_vtable_221) - }, - }, - }, - { - .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 = {}; @@ -126,16 +26,24 @@ static void test_vtable(void) { assert(sd_bus_new(&bus) >= 0); - 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); + assert(sd_bus_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.testVtable", test_vtable_2, &c) >= 0); + assert(sd_bus_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.testVtable2", test_vtable_2, &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.testVtable221", (const sd_bus_vtable *)vtable_format_221, &c) >= 0); + assert(sd_bus_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.testVtable221", + (const sd_bus_vtable *)vtable_format_221, &c) >= 0); assert(sd_bus_set_address(bus, DEFAULT_BUS_PATH) >= 0); r = sd_bus_start(bus); assert(r == 0 || /* success */ r == -ENOENT /* dbus is inactive */ ); +#ifndef __cplusplus + _cleanup_free_ char *s = NULL; + + assert_se(introspect_path(bus, "/foo", NULL, false, true, NULL, &s, NULL) == 1); + fputs(s, stdout); +#endif + sd_bus_unref(bus); } diff --git a/src/libsystemd/sd-bus/test-vtable-data.h b/src/libsystemd/sd-bus/test-vtable-data.h new file mode 100644 index 0000000000..333dbd5b12 --- /dev/null +++ b/src/libsystemd/sd-bus/test-vtable-data.h @@ -0,0 +1,132 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +/* This is meant to be included in other files, hence no headers */ + +struct context { + bool quit; + char *something; + char *automatic_string_property; + uint32_t automatic_integer_property; +}; + +static int handler(sd_bus_message *m, void *userdata, sd_bus_error *error) { + return 1; +} + +static int value_handler(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error) { + return 1; +} + +static int get_handler(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error) { + return 1; +} + +static int set_handler(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *value, void *userdata, sd_bus_error *error) { + return 1; +} + +static const sd_bus_vtable test_vtable_1[] = { + SD_BUS_VTABLE_START(0), + SD_BUS_METHOD("Hello", "ssas", "a(uu)", NULL, 0), + SD_BUS_METHOD("DeprecatedHello", "", "", NULL, SD_BUS_VTABLE_DEPRECATED), + SD_BUS_METHOD("DeprecatedHelloNoReply", "", "", NULL, SD_BUS_VTABLE_DEPRECATED|SD_BUS_VTABLE_METHOD_NO_REPLY), + SD_BUS_SIGNAL("Wowza", "sss", 0), + SD_BUS_SIGNAL("DeprecatedWowza", "ut", SD_BUS_VTABLE_DEPRECATED), + SD_BUS_WRITABLE_PROPERTY("AProperty", "s", get_handler, set_handler, 0, 0), + SD_BUS_PROPERTY("AReadOnlyDeprecatedProperty", "(ut)", get_handler, 0, SD_BUS_VTABLE_DEPRECATED), + SD_BUS_PROPERTY("ChangingProperty", "t", get_handler, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("Invalidating", "t", get_handler, 0, SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), + SD_BUS_PROPERTY("Constant", "t", get_handler, 0, SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_PROPERTY_EXPLICIT), + SD_BUS_VTABLE_END +}; + +static const sd_bus_vtable test_vtable_2[] = { + SD_BUS_VTABLE_START(0), + SD_BUS_METHOD("AlterSomething", "s", "s", handler, 0), + 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), + SD_BUS_PROPERTY("Value4", "s", value_handler, 10, 0), + SD_BUS_PROPERTY("AnExplicitProperty", "s", NULL, offsetof(struct context, something), + SD_BUS_VTABLE_PROPERTY_EXPLICIT|SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), + SD_BUS_WRITABLE_PROPERTY("Something", "s", get_handler, set_handler, 0, 0), + SD_BUS_WRITABLE_PROPERTY("AutomaticStringProperty", "s", NULL, NULL, + offsetof(struct context, automatic_string_property), 0), + SD_BUS_WRITABLE_PROPERTY("AutomaticIntegerProperty", "u", NULL, NULL, + offsetof(struct context, automatic_integer_property), 0), + 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 +}; + +static const sd_bus_vtable test_vtable_deprecated[] = { + SD_BUS_VTABLE_START(SD_BUS_VTABLE_DEPRECATED), + SD_BUS_VTABLE_END +}; + +struct sd_bus_vtable_221 { + 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_221 vtable_format_221[] = { + { + .type = _SD_BUS_VTABLE_START, + .flags = 0, + .x = { + .start = { + .element_size = sizeof(struct sd_bus_vtable_221) + }, + }, + }, + { + .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 } }, + } +}; diff --git a/src/test/meson.build b/src/test/meson.build index 521985b927..e58e1cc73d 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -911,7 +911,8 @@ tests += [ [], [threads]], - [['src/libsystemd/sd-bus/test-bus-vtable.c'], + [['src/libsystemd/sd-bus/test-bus-vtable.c', + 'src/libsystemd/sd-bus/test-vtable-data.h'], [], []], @@ -934,7 +935,8 @@ tests += [ [threads], '', 'manual'], - [['src/libsystemd/sd-bus/test-bus-introspect.c'], + [['src/libsystemd/sd-bus/test-bus-introspect.c', + 'src/libsystemd/sd-bus/test-vtable-data.h'], [], []], From d5c8d8233c2efbbd1e600d1a683ea04cd36cbac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 21 Apr 2019 22:23:45 +0200 Subject: [PATCH 6/9] busctl: add introspect --xml-interface This wraps the call to org.freedesktop.DBus.Introspectable.Introspect. Using "busctl call" directly is inconvenient because busctl escapes the string before printing. Example: $ busctl introspect --xml org.freedesktop.systemd1 /org/freedesktop/systemd1 | pygmentize -lxml | less -RF --- man/busctl.xml | 10 ++++++++++ src/busctl/busctl.c | 13 +++++++++++++ 2 files changed, 23 insertions(+) diff --git a/man/busctl.xml b/man/busctl.xml index e4c7fcb283..328c101622 100644 --- a/man/busctl.xml +++ b/man/busctl.xml @@ -140,6 +140,16 @@ + + + + + When used with the introspect call, dump the XML description received from + the D-Bus org.freedesktop.DBus.Introspectable.Introspect call instead of the + normal output. + + + MODE diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 02f12dc701..86efc02bd8 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -50,6 +50,7 @@ static size_t arg_snaplen = 4096; static bool arg_list = false; static bool arg_quiet = false; static bool arg_verbose = false; +static bool arg_xml_interface = false; static bool arg_expect_reply = true; static bool arg_auto_start = true; static bool arg_allow_interactive_authorization = true; @@ -948,6 +949,12 @@ static int introspect(int argc, char **argv, void *userdata) { if (r < 0) return bus_log_parse_error(r); + if (arg_xml_interface) { + /* Just dump the received XML and finish */ + puts(xml); + return 0; + } + /* First, get list of all properties */ r = parse_xml_introspect(argv[2], xml, &ops, members); if (r < 0) @@ -2255,6 +2262,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_SIZE, ARG_LIST, ARG_VERBOSE, + ARG_XML_INTERFACE, ARG_EXPECT_REPLY, ARG_AUTO_START, ARG_ALLOW_INTERACTIVE_AUTHORIZATION, @@ -2284,6 +2292,7 @@ static int parse_argv(int argc, char *argv[]) { { "list", no_argument, NULL, ARG_LIST }, { "quiet", no_argument, NULL, 'q' }, { "verbose", no_argument, NULL, ARG_VERBOSE }, + { "xml-interface", no_argument, NULL, ARG_XML_INTERFACE }, { "expect-reply", required_argument, NULL, ARG_EXPECT_REPLY }, { "auto-start", required_argument, NULL, ARG_AUTO_START }, { "allow-interactive-authorization", required_argument, NULL, ARG_ALLOW_INTERACTIVE_AUTHORIZATION }, @@ -2388,6 +2397,10 @@ static int parse_argv(int argc, char *argv[]) { arg_verbose = true; break; + case ARG_XML_INTERFACE: + arg_xml_interface = true; + break; + case ARG_EXPECT_REPLY: r = parse_boolean(optarg); if (r < 0) From bf135d288d11c1da6b053dfe26ffa708031d544f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 21 Apr 2019 22:25:03 +0200 Subject: [PATCH 7/9] sd-bus: when running user find function don't trust the value to be initialized The find function is externally provided, and we shouldn't trust that the authors remember to set the output parameter in all cases. --- src/libsystemd/sd-bus/bus-objects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index 6ed7cc4860..fe3b9feea6 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -22,7 +22,7 @@ static int node_vtable_get_userdata( sd_bus_error *error) { sd_bus_slot *s; - void *u, *found_u; + void *u, *found_u = NULL; int r; assert(bus); From afb9c0c95817e687d27b9fa21d4e7db6075c583d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 21 Apr 2019 22:39:30 +0200 Subject: [PATCH 8/9] man: document sd_bus_add_{object,fallback}_vtable The interface provided by those two functions is huge, so this text could probably be made two or three times as long if all details were described. But I think it's a good start. --- man/rules/meson.build | 15 + man/sd-bus.xml | 1 + man/sd_bus_add_object_vtable.xml | 473 +++++++++++++++++++++++++++++++ man/vtable-example.c | 70 +++++ man/vtable-example.xml | 54 ++++ 5 files changed, 613 insertions(+) create mode 100644 man/sd_bus_add_object_vtable.xml create mode 100644 man/vtable-example.c create mode 100644 man/vtable-example.xml diff --git a/man/rules/meson.build b/man/rules/meson.build index 9674cbb30b..6894158466 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -115,6 +115,21 @@ manpages = [ 'sd_bus_match_signal', 'sd_bus_match_signal_async'], ''], + ['sd_bus_add_object_vtable', + '3', + ['SD_BUS_METHOD', + 'SD_BUS_METHOD_WITH_NAMES', + 'SD_BUS_METHOD_WITH_NAMES_OFFSET', + 'SD_BUS_METHOD_WITH_OFFSET', + 'SD_BUS_PARAM', + 'SD_BUS_PROPERTY', + 'SD_BUS_SIGNAL', + 'SD_BUS_SIGNAL_WITH_NAMES', + 'SD_BUS_VTABLE_END', + 'SD_BUS_VTABLE_START', + 'SD_BUS_WRITABLE_PROPERTY', + 'sd_bus_add_fallback_vtable'], + ''], ['sd_bus_attach_event', '3', ['sd_bus_detach_event', 'sd_bus_get_event'], ''], ['sd_bus_close', '3', ['sd_bus_flush'], ''], ['sd_bus_creds_get_pid', diff --git a/man/sd-bus.xml b/man/sd-bus.xml index 6c925e3161..e9a66d87dd 100644 --- a/man/sd-bus.xml +++ b/man/sd-bus.xml @@ -41,6 +41,7 @@ See sd_bus_add_match3, +sd_bus_add_object_vtable3, sd_bus_attach_event3, sd_bus_creds_get_pid3, sd_bus_creds_new_from_pid3, diff --git a/man/sd_bus_add_object_vtable.xml b/man/sd_bus_add_object_vtable.xml new file mode 100644 index 0000000000..92be236afd --- /dev/null +++ b/man/sd_bus_add_object_vtable.xml @@ -0,0 +1,473 @@ + + + + + + + + sd_bus_add_object_vtable + systemd + + + + sd_bus_add_object_vtable + 3 + + + + sd_bus_add_object_vtable + sd_bus_add_fallback_vtable + SD_BUS_VTABLE_START + SD_BUS_VTABLE_END + SD_BUS_METHOD_WITH_NAMES_OFFSET + SD_BUS_METHOD_WITH_NAMES + SD_BUS_METHOD_WITH_OFFSET + SD_BUS_METHOD + SD_BUS_SIGNAL_WITH_NAMES + SD_BUS_SIGNAL + SD_BUS_WRITABLE_PROPERTY + SD_BUS_PROPERTY + SD_BUS_PARAM + + Declare properties and methods for a D-Bus path + + + + + #include <systemd/sd-bus-vtable.h> + + + typedef int (*sd_bus_message_handler_t) + sd_bus_message *m + void *userdata + sd_bus_error *ret_error + + + + typedef int (*sd_bus_property_get_t) + sd_bus *bus + const char *path + const char *interface + const char *property + sd_bus_message *reply + void *userdata + sd_bus_error *ret_error + + + + typedef int (*sd_bus_property_set_t) + sd_bus *bus + const char *path + const char *interface + const char *property + sd_bus_message *value + void *userdata + sd_bus_error *ret_error + + + + typedef int (*sd_bus_object_find_t) + const char *path + const char *interface + void *userdata + void **ret_found + sd_bus_error *ret_error + + + + int sd_bus_add_object_vtable + sd_bus *bus + sd_bus_slot **slot + const char *path + const char *interface + const sd_bus_vtable *vtable + void *userdata + + + + int sd_bus_add_fallback_vtable + sd_bus *bus + sd_bus_slot **slot + const char *prefix + const char *interface + const sd_bus_vtable *vtable + sd_bus_object_find_t find + void *userdata + + + + SD_BUS_VTABLE_START(flags) + + + SD_BUS_VTABLE_END + + + SD_BUS_METHOD_WITH_NAMES_OFFSET( + member, + signature, + in_names, + result, + out_names, + handler, + offset, + flags) + + + + SD_BUS_METHOD_WITH_NAMES( + member, + signature, + in_names, + result, + out_names, + handler, + flags) + + + + SD_BUS_METHOD_WITH_OFFSET( + member, + signature, + result, + handler, + offset, + flags) + + + + SD_BUS_METHOD( + member, + signature, + result, + handler, + flags) + + + + SD_BUS_SIGNAL_WITH_NAMES( + member, + signature, + names, + flags) + + + + SD_BUS_SIGNAL( + member, + signature, + flags) + + + + SD_BUS_WRITABLE_PROPERTY( + member, + signature, + get, + set, + offset, + flags) + + + + SD_BUS_PROPERTY( + member, + signature, + get, + offset, + flags) + + + + SD_BUS_PARAM(name) + + + + + + Description + + sd_bus_add_object_vtable() is used to declare attributes for the path object + path path connected to the bus connection bus under the + interface interface. The table vtable may contain property + declarations using SD_BUS_PROPERTY() or + SD_BUS_WRITABLE_PROPERTY(), method declarations using + SD_BUS_METHOD(), SD_BUS_METHOD_WITH_NAMES(), + SD_BUS_METHOD_WITH_OFFSET(), or + SD_BUS_METHOD_WITH_NAMES_OFFSET(), and signal declarations using + SD_BUS_SIGNAL_WITH_NAMES() or SD_BUS_SIGNAL(), see below. The + userdata parameter contains a pointer that will be passed to various callback + functions. It may be specified as NULL if no value is necessary. + + sd_bus_add_object_vtable() is similar to + sd_bus_add_object_vtable(), but is used to register "fallback" attributes. When + looking for an attribute declaration, bus object paths registered with + sd_bus_add_object_vtable() are checked first. If no match is found, the fallback + vtables are checked for each prefix of the bus object path, i.e. with the last slash-separated components + successively removed. This allows the vtable to be used for an arbitrary number of dynamically created + objects. + + Parameter find is a function which is used to locate the target object + based on the bus object path path. It must return 1 and + set the ret_found output parameter if the object is found, return + 0 if the object was not found, and return a negative errno-style error code or + initialize the error structure ret_error on error. The pointer passed in + ret_found will be used as the userdata parameter for the + callback functions (offset by the offset offsets as specified in the vtable + entries). + + For both functions, a match slot is created internally. If the output parameter + slot is NULL, a "floating" slot object is created, see + sd_bus_slot_set_floating3. + Otherwise, a pointer to the slot object is returned. In that case, the reference to the slot object + should be dropped when the vtable is not needed anymore, see + sd_bus_slot_unref3. + + + + The <structname>sd_bus_vtable</structname> array + + The array consists of the structures of type sd_bus_vtable, but it + should never be filled in manually, but through one of the following macros: + + + + SD_BUS_VTABLE_START() + SD_BUS_VTABLE_END + + Those must always be the first and last element. + + + + SD_BUS_METHOD_WITH_NAMES_OFFSET() + SD_BUS_METHOD_WITH_NAMES() + SD_BUS_METHOD_WITH_OFFSET() + SD_BUS_METHOD() + + Declare a D-Bus method with the name member, parameter + signature signature, result signature result. + Parameters in_names and out_names specify the + argument names of the input and output arguments in the function signature. The handler function + handler must be of type sd_bus_message_handler_t. + It will be called to handle the incoming messages that call this method. It receives a pointer that + is the userdata parameter passed to the registration function offset by + offset bytes. This may be used to pass pointers to different fields in + the same data structure to different methods in the same + vtable. in_names and out_names should be + created using the SD_BUS_PARAM() macro, see below. Parameter + flags is a combination of flags, see below. + + SD_BUS_METHOD_WITH_NAMES(), + SD_BUS_METHOD_WITH_OFFSET(), and SD_BUS_METHOD() are + variants which specify zero offset (userdata parameter is passed with + no change), leave the names unset (i.e. no parameter names), or both. + + + + + SD_BUS_SIGNAL_WITH_NAMES() + SD_BUS_SIGNAL() + + Declare a D-Bus signal with the name member, + parameter signature signature, and argument names + names. names should be + created using the SD_BUS_PARAM() macro, see below. + Parameter flags is a combination of flags, see below. + + + Equivalent to SD_BUS_SIGNAL_WITH_NAMES() with the + names paramater unset (i.e. no parameter names). + + + + + SD_BUS_WRITABLE_PROPERTY() + SD_BUS_PROPERTY() + + Declare a D-Bus property with the name member and value + signature signature. Parameters get and + set are the getter and setter methods. They are called with a pointer + that is the userdata parameter passed to the registration function + offset by offset bytes. This may be used pass pointers to different + fields in the same data structure to different setters and getters in the same vtable. Parameter + flags is a combination of flags, see below. + + The setter and getter methods may be omitted (specified as NULL), if the + property has one of the basic types or as in case of read-only properties. In + those cases, the userdata and offset + parameters must together point to valid variable of the corresponding type. A default setter and + getters will be provided, which simply copy the argument between this variable and the message. + + + SD_BUS_PROPERTY() is used to define a read-only property. + + + + + SD_BUS_PARAM() + Parameter names should be wrapped in this macro, see the example below. + + + + + + + Flags + + The flags parameter is used to specify a combination of + D-Bus annotations. + + + + + SD_BUS_VTABLE_DEPRECATED + + Mark this vtable entry as deprecated using the + org.freedesktop.DBus.Deprecated annotation in introspection data. If + specified for SD_BUS_VTABLE_START(), the annotation is applied to the + enclosing interface. + + + + SD_BUS_VTABLE_HIDDEN + + Make this vtable entry hidden. It will not be shown in introspection data. If + specified for SD_BUS_VTABLE_START(), all entries in the array are hidden. + + + + + + SD_BUS_VTABLE_UNPRIVILEGED + + Mark this vtable entry as unprivileged. If not specified, the + org.freedesktop.systemd1.Privileged annotation with value + true will be shown in introspection data. + + + + + SD_BUS_VTABLE_METHOD_NO_REPLY + + Mark his vtable entry as a method that will not return a reply using the + org.freedesktop.DBus.Method.NoReply annotation in introspection data. + + + + + SD_BUS_VTABLE_CONST + SD_BUS_VTABLE_EMITS_CHANGE + SD_BUS_VTABLE_EMITS_INVALIDATION + + Those three flags correspond to different values of the + org.freedesktop.DBus.Property.EmitsChangedSignal annotation, which specifies + whether the org.freedesktop.DBus.Properties.PropertiesChanged signal is + emitted whenever the property changes. SD_BUS_VTABLE_CONST corresponds to + const and means that the property never changes during the lifetime of the + object it belongs to, so no signal needs to be emitted. + SD_BUS_VTABLE_EMITS_CHANGE corresponds to true and means + that the signal is emitted. SD_BUS_VTABLE_EMITS_INVALIDATION corresponds to + invalides and means that the signal is emitted, but the value is not included + in the signal. + + + + + SD_BUS_VTABLE_PROPERTY_EXPLICIT + + Mark this vtable property entry as requiring explicit request to for the value to + be shown (generally because the value is large or slow to calculate). This entry cannot be combined + with SD_BUS_VTABLE_EMITS_CHANGE, and will not be shown in property listings by + default (e.g. busctl introspect). This corresponds to the + org.freedesktop.systemd1.Explicit annotation in introspection data. + + + + + + + + Examples + + + Create a simple listener on the bus + + + + This creates a simple client on the bus (the user bus, when run as normal user). + We may use the D-Bus org.freedesktop.DBus.Introspectable.Introspect + call to acquire the XML description of the interface: + + + + + + + Return Value + + On success, sd_bus_add_object_vtable and + sd_bus_add_fallback_vtable calls return 0 or a positive integer. On failure, they + return a negative errno-style error code. + + + Errors + + Returned errors may indicate the following problems: + + + + -EINVAL + + One of the required parameters is NULL or invalid. A reserved + D-Bus interface was passed as the interface parameter. + + + + -ENOPKG + + The bus cannot be resolved. + + + + -ECHILD + + The bus was created in a different process. + + + + -ENOMEM + + Memory allocation failed. + + + + -EPROTOTYPE + + sd_bus_add_object_vtable and + sd_bus_add_fallback_vtable have been both called + for the same bus object path, which is not allowed. + + + + -EEXIST + + This vtable has already been registered for this + interface and path. + + + + + + + + + + See Also + + + sd-bus3, + busctl1 + + + diff --git a/man/vtable-example.c b/man/vtable-example.c new file mode 100644 index 0000000000..a2a6cd18d7 --- /dev/null +++ b/man/vtable-example.c @@ -0,0 +1,70 @@ +#include +#include +#include +#include +#include +#include + +#define _cleanup_(f) __attribute__((cleanup(f))) + +typedef struct object { + char *name; + uint32_t number; +} object; + +static int method(sd_bus_message *m, void *userdata, sd_bus_error *error) { + printf("Got called with userdata=%p\n", userdata); + return 1; +} + +static const sd_bus_vtable vtable[] = { + SD_BUS_VTABLE_START(0), + SD_BUS_METHOD( + "Method1", "s", "s", method, 0), + SD_BUS_METHOD_WITH_NAMES_OFFSET( + "Method2", + "so", SD_BUS_PARAM(string) SD_BUS_PARAM(path), + "s", SD_BUS_PARAM(returnstring), + method, offsetof(object, number), + SD_BUS_VTABLE_DEPRECATED), + SD_BUS_WRITABLE_PROPERTY( + "AutomaticStringProperty", "s", NULL, NULL, + offsetof(object, name), + SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_WRITABLE_PROPERTY( + "AutomaticIntegerProperty", "u", NULL, NULL, + offsetof(object, number), + SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), + SD_BUS_VTABLE_END +}; + +#define check(x) ({ \ + int r = x; \ + errno = r < 0 ? -r : 0; \ + printf(#x ": %m\n"); \ + if (r < 0) \ + return EXIT_FAILURE; \ + }) + +int main(int argc, char **argv) { + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + + sd_bus_default(&bus); + + object object = { .number = 666 }; + check((object.name = strdup("name")) != NULL); + + check(sd_bus_add_object_vtable(bus, NULL, "/object", + "org.freedesktop.systemd.VtableExample", + vtable, + &object)); + + while (true) { + check(sd_bus_wait(bus, UINT64_MAX)); + check(sd_bus_process(bus, NULL)); + } + + free(object.name); + + return 0; +} diff --git a/man/vtable-example.xml b/man/vtable-example.xml new file mode 100644 index 0000000000..a3cdeae704 --- /dev/null +++ b/man/vtable-example.xml @@ -0,0 +1,54 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 155dc16168fd98a8bfddcf7fd4ceba472d44db51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 23 Apr 2019 14:09:18 +0200 Subject: [PATCH 9/9] meson: do not use f-strings Our travis CI still uses python3.5. I'm making this into a separate commit to make it easy to revert later. --- src/test/generate-sym-test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/generate-sym-test.py b/src/test/generate-sym-test.py index 2510809452..4d358b8e34 100755 --- a/src/test/generate-sym-test.py +++ b/src/test/generate-sym-test.py @@ -13,9 +13,9 @@ for line in open(sys.argv[1]): if match: s = match.group(1) if s == 'sd_bus_object_vtable_format': - print(f' &{s},') + print(' &{},'.format(s)) else: - print(f' {s},') + print(' {},'.format(s)) print('''};