unit: deduce following unit value dynamically instead of statically, to avoid dangling pointers

5 files changed, 65 insertions, 27 deletions

diff --git a/src/dbus-manager.c b/src/dbus-manager.c
index 63e8083805..c1238f0066 100644
--- a/src/dbus-manager.c
+++ b/src/dbus-manager.c
@@ -413,6 +413,7 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
                         const char *description, *load_state, *active_state, *sub_state, *sjob_type, *following;
                         DBusMessageIter sub2;
                         uint32_t job_id;
+                        Unit *f;
 
                         if (k != u->meta.id)
                                 continue;
@@ -424,7 +425,9 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
                         load_state = unit_load_state_to_string(u->meta.load_state);
                         active_state = unit_active_state_to_string(unit_active_state(u));
                         sub_state = unit_sub_state_to_string(u);
-                        following = u->meta.following ? u->meta.following->meta.id : "";
+
+                        f = unit_following(u);
+                        following = f ? f->meta.id : "";
 
                         if (!(u_path = unit_dbus_path(u)))
                                 goto oom;
diff --git a/src/dbus-unit.c b/src/dbus-unit.c
index bb2541800a..d25f325013 100644
--- a/src/dbus-unit.c
+++ b/src/dbus-unit.c
@@ -48,7 +48,7 @@ int bus_unit_append_names(Manager *m, DBusMessageIter *i, const char *property,
 }
 
 int bus_unit_append_following(Manager *m, DBusMessageIter *i, const char *property, void *data) {
-        Unit *u = data;
+        Unit *u = data, *f;
         const char *d;
 
         assert(m);
@@ -56,7 +56,8 @@ int bus_unit_append_following(Manager *m, DBusMessageIter *i, const char *proper
         assert(property);
         assert(u);
 
-        d = u->meta.following ? u->meta.following->meta.id : "";
+        f = unit_following(u);
+        d = f ? f->meta.id : "";
 
         if (!dbus_message_iter_append_basic(i, DBUS_TYPE_STRING, &d))
                 return -ENOMEM;
diff --git a/src/device.c b/src/device.c
index 526e714378..46b3ed3dff 100644
--- a/src/device.c
+++ b/src/device.c
@@ -40,22 +40,21 @@ static void device_unset_sysfs(Device *d) {
 
         assert(d);
 
-        if (d->sysfs) {
-                /* Remove this unit from the chain of devices which share the
-                 * same sysfs path. */
-                first = hashmap_get(d->meta.manager->devices_by_sysfs, d->sysfs);
-                LIST_REMOVE(Device, same_sysfs, first, d);
-
-                if (first)
-                        hashmap_remove_and_replace(d->meta.manager->devices_by_sysfs, d->sysfs, first->sysfs, first);
-                else
-                        hashmap_remove(d->meta.manager->devices_by_sysfs, d->sysfs);
-
-                free(d->sysfs);
-                d->sysfs = NULL;
-        }
+        if (!d->sysfs)
+                return;
+
+        /* Remove this unit from the chain of devices which share the
+         * same sysfs path. */
+        first = hashmap_get(d->meta.manager->devices_by_sysfs, d->sysfs);
+        LIST_REMOVE(Device, same_sysfs, first, d);
 
-        d->meta.following = NULL;
+        if (first)
+                hashmap_remove_and_replace(d->meta.manager->devices_by_sysfs, d->sysfs, first->sysfs, first);
+        else
+                hashmap_remove(d->meta.manager->devices_by_sysfs, d->sysfs);
+
+        free(d->sysfs);
+        d->sysfs = NULL;
 }
 
 static void device_init(Unit *u) {
@@ -268,10 +267,7 @@ static int device_update_unit(Manager *m, struct udev_device *dev, const char *p
                                         goto fail;
                         }
                 }
-
-                u->meta.following = NULL;
-        } else
-                device_find_escape_name(m, sysfs, &u->meta.following);
+        }
 
         unit_add_to_dbus_queue(u);
         return 0;
@@ -365,6 +361,30 @@ static int device_process_removed_device(Manager *m, struct udev_device *dev) {
         return 0;
 }
 
+static Unit *device_following(Unit *u) {
+        Device *d = DEVICE(u);
+        Device *other, *first = NULL;
+
+        assert(d);
+
+        if (startswith(u->meta.id, "sys-"))
+                return NULL;
+
+        /* Make everybody follow the unit that's named after the sysfs path */
+        for (other = d->same_sysfs_next; other; other = other->same_sysfs_next)
+                if (startswith(other->meta.id, "sys-"))
+                        return UNIT(other);
+
+        for (other = d->same_sysfs_prev; other; other = other->same_sysfs_prev) {
+                if (startswith(other->meta.id, "sys-"))
+                        return UNIT(other);
+
+                first = other;
+        }
+
+        return UNIT(first);
+}
+
 static void device_shutdown(Manager *m) {
         assert(m);
 
@@ -512,6 +532,8 @@ const UnitVTable device_vtable = {
 
         .bus_message_handler = bus_device_message_handler,
 
+        .following = device_following,
+
         .enumerate = device_enumerate,
         .shutdown = device_shutdown
 };
diff --git a/src/unit.c b/src/unit.c
index 50f3b8fabd..840c1d1fc4 100644
--- a/src/unit.c
+++ b/src/unit.c
@@ -589,6 +589,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) {
                 timestamp3[FORMAT_TIMESTAMP_MAX],
                 timestamp4[FORMAT_TIMESTAMP_MAX],
                 timespan[FORMAT_TIMESPAN_MAX];
+        Unit *following;
 
         assert(u);
         assert(u->meta.type >= 0);
@@ -625,8 +626,8 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) {
         SET_FOREACH(t, u->meta.names, i)
                 fprintf(f, "%s\tName: %s\n", prefix, t);
 
-        if (u->meta.following)
-                fprintf(f, "%s\tFollowing: %s\n", prefix, u->meta.following->meta.id);
+        if ((following = unit_following(u)))
+                fprintf(f, "%s\tFollowing: %s\n", prefix, following->meta.id);
 
         if (u->meta.fragment_path)
                 fprintf(f, "%s\tFragment Path: %s\n", prefix, u->meta.fragment_path);
@@ -2081,6 +2082,15 @@ void unit_reset_maintenance(Unit *u) {
                 UNIT_VTABLE(u)->reset_maintenance(u);
 }
 
+Unit *unit_following(Unit *u) {
+        assert(u);
+
+        if (UNIT_VTABLE(u)->following)
+                return UNIT_VTABLE(u)->following(u);
+
+        return NULL;
+}
+
 static const char* const unit_type_table[_UNIT_TYPE_MAX] = {
         [UNIT_SERVICE] = "service",
         [UNIT_TIMER] = "timer",
diff --git a/src/unit.h b/src/unit.h
index f1171270f8..34e86d1086 100644
--- a/src/unit.h
+++ b/src/unit.h
@@ -176,9 +176,6 @@ struct Meta {
         /* GC queue */
         LIST_FIELDS(Meta, gc_queue);
 
-        /* This follows another unit in state */
-        Unit *following;
-
         /* Used during GC sweeps */
         unsigned gc_marker;
 
@@ -313,6 +310,9 @@ struct UnitVTable {
         /* Called for each message received on the bus */
         DBusHandlerResult (*bus_message_handler)(Unit *u, DBusConnection *c, DBusMessage *message);
 
+        /* Return the unit this unit is following */
+        Unit *(*following)(Unit *u);
+
         /* This is called for each unit type and should be used to
          * enumerate existing devices and load them. However,
          * everything that is loaded here should still stay in
@@ -475,6 +475,8 @@ bool unit_need_daemon_reload(Unit *u);
 
 void unit_reset_maintenance(Unit *u);
 
+Unit *unit_following(Unit *u);
+
 const char *unit_type_to_string(UnitType i);
 UnitType unit_type_from_string(const char *s);