From a7f241db3f1ae96ab2708092e1b31d2feb989947 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 21 Jul 2010 05:00:29 +0200 Subject: [PATCH] unit: deduce following unit value dynamically instead of statically, to avoid dangling pointers --- src/dbus-manager.c | 5 +++- src/dbus-unit.c | 5 ++-- src/device.c | 60 +++++++++++++++++++++++++++++++--------------- src/unit.c | 14 +++++++++-- src/unit.h | 8 ++++--- 5 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/dbus-manager.c b/src/dbus-manager.c index 63e80838..c1238f00 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 bb254180..d25f3250 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 526e7143..46b3ed3d 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 50f3b8fa..840c1d1f 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 f1171270..34e86d10 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); -- 2.39.5