From: Lennart Poettering Date: Tue, 3 Jul 2012 14:09:36 +0000 (+0200) Subject: load-fragment: a few modernizations X-Git-Tag: v186~3 X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=9946996cda11a18b44d82344676e5a0e96339408 load-fragment: a few modernizations --- diff --git a/TODO b/TODO index 7cc177f9f..b42f2ff70 100644 --- a/TODO +++ b/TODO @@ -22,6 +22,9 @@ Bugfixes: Features: +* load-fragment: when loading a unit file via a chain of symlinks + verify that it isn't masked via any of the names traversed. + * journald: _BOOT_ID triggers too many collisions. * journald: we currently rotate only after MaxUse+MaxFilesize has been reached. diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 1ca0dece6..e0e42acbb 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2047,21 +2047,24 @@ static int open_follow(char **filename, FILE **_f, Set *names, char **_final) { } /* Try to open the file name, but don't if its a symlink */ - if ((fd = open(*filename, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW)) >= 0) + fd = open(*filename, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW); + if (fd >= 0) break; if (errno != ELOOP) return -errno; /* Hmm, so this is a symlink. Let's read the name, and follow it manually */ - if ((r = readlink_and_make_absolute(*filename, &target)) < 0) + r = readlink_and_make_absolute(*filename, &target); + if (r < 0) return r; free(*filename); *filename = target; } - if (!(f = fdopen(fd, "re"))) { + f = fdopen(fd, "re"); + if (!f) { r = -errno; close_nointr_nofail(fd); return r; @@ -2085,7 +2088,8 @@ static int merge_by_names(Unit **u, Set *names, const char *id) { /* First try to merge in the other name into our * unit */ - if ((r = unit_merge_by_name(*u, k)) < 0) { + r = unit_merge_by_name(*u, k); + if (r < 0) { Unit *other; /* Hmm, we couldn't merge the other unit into @@ -2095,11 +2099,13 @@ static int merge_by_names(Unit **u, Set *names, const char *id) { other = manager_get_unit((*u)->manager, k); free(k); - if (other) - if ((r = unit_merge(other, *u)) >= 0) { + if (other) { + r = unit_merge(other, *u); + if (r >= 0) { *u = other; return merge_by_names(u, names, NULL); } + } return r; } @@ -2130,12 +2136,14 @@ static int load_from_path(Unit *u, const char *path) { if (path_is_absolute(path)) { - if (!(filename = strdup(path))) { + filename = strdup(path); + if (!filename) { r = -ENOMEM; goto finish; } - if ((r = open_follow(&filename, &f, symlink_names, &id)) < 0) { + r = open_follow(&filename, &f, symlink_names, &id); + if (r < 0) { free(filename); filename = NULL; @@ -2151,7 +2159,8 @@ static int load_from_path(Unit *u, const char *path) { /* Instead of opening the path right away, we manually * follow all symlinks and add their name to our unit * name set while doing so */ - if (!(filename = path_make_absolute(path, *p))) { + filename = path_make_absolute(path, *p); + if (!filename) { r = -ENOMEM; goto finish; } @@ -2163,8 +2172,6 @@ static int load_from_path(Unit *u, const char *path) { r = open_follow(&filename, &f, symlink_names, &id); if (r < 0) { - char *sn; - free(filename); filename = NULL; @@ -2172,9 +2179,7 @@ static int load_from_path(Unit *u, const char *path) { goto finish; /* Empty the symlink names for the next run */ - while ((sn = set_steal_first(symlink_names))) - free(sn); - + set_clear_free(symlink_names); continue; } @@ -2189,7 +2194,8 @@ static int load_from_path(Unit *u, const char *path) { } merged = u; - if ((r = merge_by_names(&merged, symlink_names, id)) < 0) + r = merge_by_names(&merged, symlink_names, id); + if (r < 0) goto finish; if (merged != u) { @@ -2252,7 +2258,8 @@ int unit_load_fragment(Unit *u) { /* First, try to find the unit under its id. We always look * for unit files in the default directories, to make it easy * to override things by placing things in /etc/systemd/system */ - if ((r = load_from_path(u, u->id)) < 0) + r = load_from_path(u, u->id); + if (r < 0) return r; /* Try to find an alias we can load this with */ @@ -2262,7 +2269,8 @@ int unit_load_fragment(Unit *u) { if (t == u->id) continue; - if ((r = load_from_path(u, t)) < 0) + r = load_from_path(u, t); + if (r < 0) return r; if (u->load_state != UNIT_STUB) @@ -2272,7 +2280,8 @@ int unit_load_fragment(Unit *u) { /* And now, try looking for it under the suggested (originally linked) path */ if (u->load_state == UNIT_STUB && u->fragment_path) { - if ((r = load_from_path(u, u->fragment_path)) < 0) + r = load_from_path(u, u->fragment_path); + if (r < 0) return r; if (u->load_state == UNIT_STUB) { @@ -2288,7 +2297,8 @@ int unit_load_fragment(Unit *u) { if (u->load_state == UNIT_STUB && u->instance) { char *k; - if (!(k = unit_name_template(u->id))) + k = unit_name_template(u->id); + if (!k) return -ENOMEM; r = load_from_path(u, k); @@ -2303,7 +2313,8 @@ int unit_load_fragment(Unit *u) { if (t == u->id) continue; - if (!(k = unit_name_template(t))) + k = unit_name_template(t); + if (!k) return -ENOMEM; r = load_from_path(u, k); diff --git a/src/shared/hashmap.c b/src/shared/hashmap.c index 5d1e63208..ab0095730 100644 --- a/src/shared/hashmap.c +++ b/src/shared/hashmap.c @@ -277,11 +277,7 @@ void hashmap_free(Hashmap*h) { } void hashmap_free_free(Hashmap *h) { - void *p; - - while ((p = hashmap_steal_first(h))) - free(p); - + hashmap_clear_free(h); hashmap_free(h); } @@ -293,6 +289,15 @@ void hashmap_clear(Hashmap *h) { remove_entry(h, h->iterate_list_head); } +void hashmap_clear_free(Hashmap *h) { + void *p; + + assert(h); + + while ((p = hashmap_steal_first(h))) + free(p); +} + static struct hashmap_entry *hash_scan(Hashmap *h, unsigned hash, const void *key) { struct hashmap_entry *e; assert(h); diff --git a/src/shared/hashmap.h b/src/shared/hashmap.h index fcf2cb1c8..deefd9a80 100644 --- a/src/shared/hashmap.h +++ b/src/shared/hashmap.h @@ -71,6 +71,8 @@ void *hashmap_iterate_backwards(Hashmap *h, Iterator *i, const void **key); void *hashmap_iterate_skip(Hashmap *h, const void *key, Iterator *i); void hashmap_clear(Hashmap *h); +void hashmap_clear_free(Hashmap *h); + void *hashmap_steal_first(Hashmap *h); void *hashmap_steal_first_key(Hashmap *h); void* hashmap_first(Hashmap *h); diff --git a/src/shared/set.c b/src/shared/set.c index 20e457eda..f5c7c37ca 100644 --- a/src/shared/set.c +++ b/src/shared/set.c @@ -116,3 +116,7 @@ Set* set_copy(Set *s) { void set_clear(Set *s) { hashmap_clear(MAKE_HASHMAP(s)); } + +void set_clear_free(Set *s) { + hashmap_clear_free(MAKE_HASHMAP(s)); +} diff --git a/src/shared/set.h b/src/shared/set.h index a4653b3a4..18921df42 100644 --- a/src/shared/set.h +++ b/src/shared/set.h @@ -56,6 +56,8 @@ void *set_iterate_backwards(Set *s, Iterator *i); void *set_iterate_skip(Set *s, void *value, Iterator *i); void set_clear(Set *s); +void set_clear_free(Set *s); + void *set_steal_first(Set *s); void* set_first(Set *s); void* set_last(Set *s);