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.

diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c
index ce2cb94..7053471 100644 (file)
--- 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 b278094..582b050 100644 (file)
--- 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);