From bea1a01310efdf51b8c609a300d49bf5c25509c3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 31 Oct 2018 17:03:50 +0100 Subject: strv: wrap strv_new() in a macro so that NULL sentinel is implicit --- src/basic/strv.c | 4 +-- src/basic/strv.h | 3 +- src/basic/time-util.c | 2 +- src/core/service.c | 2 +- src/core/slice.c | 4 +-- src/libsystemd/sd-bus/test-bus-objects.c | 4 +-- src/machine/machine-dbus.c | 2 +- src/shared/ask-password-api.c | 2 +- src/shared/conf-parser.c | 2 +- src/shared/install.c | 2 +- src/shared/path-lookup.c | 15 +++------ src/shared/sleep-config.c | 10 +++--- src/systemctl/systemctl.c | 2 +- src/sysv-generator/sysv-generator.c | 2 +- src/test/test-env-util.c | 17 +++++----- src/test/test-exec-util.c | 3 +- src/test/test-nss.c | 7 ++--- src/test/test-path-util.c | 2 +- src/test/test-serialize.c | 5 ++- src/test/test-sleep.c | 16 +++++----- src/test/test-strv.c | 53 ++++++++++++++++---------------- 21 files changed, 76 insertions(+), 83 deletions(-) (limited to 'src') diff --git a/src/basic/strv.c b/src/basic/strv.c index 1bab723e88..3a62f25ded 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -169,7 +169,7 @@ char **strv_new_ap(const char *x, va_list ap) { return TAKE_PTR(a); } -char **strv_new(const char *x, ...) { +char **strv_new_internal(const char *x, ...) { char **r; va_list ap; @@ -658,7 +658,7 @@ char **strv_split_nulstr(const char *s) { } if (!r) - return strv_new(NULL, NULL); + return strv_new(NULL); return r; } diff --git a/src/basic/strv.h b/src/basic/strv.h index e9e6063f58..5f1803d87d 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -54,8 +54,9 @@ bool strv_equal(char **a, char **b); #define strv_contains(l, s) (!!strv_find((l), (s))) -char **strv_new(const char *x, ...) _sentinel_; +char **strv_new_internal(const char *x, ...) _sentinel_; char **strv_new_ap(const char *x, va_list ap); +#define strv_new(...) strv_new_internal(__VA_ARGS__, NULL) #define STRV_IGNORE ((const char *) -1) diff --git a/src/basic/time-util.c b/src/basic/time-util.c index e351684219..141b91f637 100644 --- a/src/basic/time-util.c +++ b/src/basic/time-util.c @@ -1190,7 +1190,7 @@ int get_timezones(char ***ret) { assert(ret); - zones = strv_new("UTC", NULL); + zones = strv_new("UTC"); if (!zones) return -ENOMEM; diff --git a/src/core/service.c b/src/core/service.c index 2a36cc03f3..f6f3ecc0e6 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1225,7 +1225,7 @@ static int service_collect_fds( return -ENOMEM; rfds[0] = s->socket_fd; - rfd_names = strv_new("connection", NULL); + rfd_names = strv_new("connection"); if (!rfd_names) return -ENOMEM; diff --git a/src/core/slice.c b/src/core/slice.c index a8bdbebe4b..74d056f7bc 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -124,7 +124,7 @@ static int slice_load_root_slice(Unit *u) { if (!u->description) u->description = strdup("Root Slice"); if (!u->documentation) - u->documentation = strv_new("man:systemd.special(7)", NULL); + u->documentation = strv_new("man:systemd.special(7)"); return 1; } @@ -147,7 +147,7 @@ static int slice_load_system_slice(Unit *u) { if (!u->description) u->description = strdup("System Slice"); if (!u->documentation) - u->documentation = strv_new("man:systemd.special(7)", NULL); + u->documentation = strv_new("man:systemd.special(7)"); return 1; } diff --git a/src/libsystemd/sd-bus/test-bus-objects.c b/src/libsystemd/sd-bus/test-bus-objects.c index c12795b033..3c5bb88f4e 100644 --- a/src/libsystemd/sd-bus/test-bus-objects.c +++ b/src/libsystemd/sd-bus/test-bus-objects.c @@ -207,7 +207,7 @@ static const sd_bus_vtable vtable2[] = { static int enumerator_callback(sd_bus *bus, const char *path, void *userdata, char ***nodes, sd_bus_error *error) { if (object_path_startswith("/value", path)) - assert_se(*nodes = strv_new("/value/a", "/value/b", "/value/c", NULL)); + assert_se(*nodes = strv_new("/value/a", "/value/b", "/value/c")); return 1; } @@ -215,7 +215,7 @@ static int enumerator_callback(sd_bus *bus, const char *path, void *userdata, ch static int enumerator2_callback(sd_bus *bus, const char *path, void *userdata, char ***nodes, sd_bus_error *error) { if (object_path_startswith("/value/a", path)) - assert_se(*nodes = strv_new("/value/a/x", "/value/a/y", "/value/a/z", NULL)); + assert_se(*nodes = strv_new("/value/a/x", "/value/a/y", "/value/a/z")); return 1; } diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index 77e8b161b0..4f4d780db0 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -605,7 +605,7 @@ int bus_machine_method_open_shell(sd_bus_message *message, void *userdata, sd_bu if (strv_isempty(args)) { args = strv_free(args); - args = strv_new(path, NULL); + args = strv_new(path); if (!args) return -ENOMEM; } diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 5f1c34c841..ae3dd33b7a 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -678,7 +678,7 @@ int ask_password_agent( if (passphrase[0] == '+') { /* An empty message refers to the empty password */ if (n == 1) - l = strv_new("", NULL); + l = strv_new(""); else l = strv_parse_nulstr(passphrase+1, n-1); explicit_bzero_safe(passphrase, n); diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index b21ecee129..b9c5b412f2 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -1113,7 +1113,7 @@ int config_parse_join_controllers( controllers = new(char**, 2); if (!controllers) return log_oom(); - controllers[0] = strv_new(NULL, NULL); + controllers[0] = strv_new(NULL); if (!controllers[0]) return log_oom(); controllers[1] = NULL; diff --git a/src/shared/install.c b/src/shared/install.c index d806bc69ff..d08d0af7b5 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -2446,7 +2446,7 @@ int unit_file_add_dependency( l = &i->required_by; strv_free(*l); - *l = strv_new(target_info->name, NULL); + *l = strv_new(target_info->name); if (!*l) return -ENOMEM; } diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index 075f4d1e6c..dc06502c81 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -137,8 +137,7 @@ int xdg_user_dirs(char ***ret_config_dirs, char ***ret_data_dirs) { data_dirs = strv_split(e, ":"); else data_dirs = strv_new("/usr/local/share", - "/usr/share", - NULL); + "/usr/share"); if (!data_dirs) return -ENOMEM; @@ -616,8 +615,7 @@ int lookup_paths_init( SYSTEM_DATA_UNIT_PATH, "/usr/lib/systemd/system", STRV_IFNOTNULL(flags & LOOKUP_PATHS_SPLIT_USR ? "/lib/systemd/system" : NULL), - STRV_IFNOTNULL(generator_late), - NULL); + STRV_IFNOTNULL(generator_late)); break; case UNIT_FILE_GLOBAL: @@ -640,8 +638,7 @@ int lookup_paths_init( "/usr/local/lib/systemd/user", USER_DATA_UNIT_PATH, "/usr/lib/systemd/user", - STRV_IFNOTNULL(generator_late), - NULL); + STRV_IFNOTNULL(generator_late)); break; case UNIT_FILE_USER: @@ -891,16 +888,14 @@ char **generator_binary_paths(UnitFileScope scope) { return strv_new("/run/systemd/system-generators", "/etc/systemd/system-generators", "/usr/local/lib/systemd/system-generators", - SYSTEM_GENERATOR_PATH, - NULL); + SYSTEM_GENERATOR_PATH); case UNIT_FILE_GLOBAL: case UNIT_FILE_USER: return strv_new("/run/systemd/user-generators", "/etc/systemd/user-generators", "/usr/local/lib/systemd/user-generators", - USER_GENERATOR_PATH, - NULL); + USER_GENERATOR_PATH); default: assert_not_reached("Hmm, unexpected scope."); diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 6778399188..3270ea3c4a 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -73,7 +73,7 @@ int parse_sleep_config(const char *verb, bool *ret_allow, char ***ret_modes, cha if (suspend_state) states = TAKE_PTR(suspend_state); else - states = strv_new("mem", "standby", "freeze", NULL); + states = strv_new("mem", "standby", "freeze"); } else if (streq(verb, "hibernate")) { allow = allow_hibernate != 0; @@ -81,12 +81,12 @@ int parse_sleep_config(const char *verb, bool *ret_allow, char ***ret_modes, cha if (hibernate_mode) modes = TAKE_PTR(hibernate_mode); else - modes = strv_new("platform", "shutdown", NULL); + modes = strv_new("platform", "shutdown"); if (hibernate_state) states = TAKE_PTR(hibernate_state); else - states = strv_new("disk", NULL); + states = strv_new("disk"); } else if (streq(verb, "hybrid-sleep")) { allow = allow_hybrid_sleep > 0 || @@ -95,12 +95,12 @@ int parse_sleep_config(const char *verb, bool *ret_allow, char ***ret_modes, cha if (hybrid_mode) modes = TAKE_PTR(hybrid_mode); else - modes = strv_new("suspend", "platform", "shutdown", NULL); + modes = strv_new("suspend", "platform", "shutdown"); if (hybrid_state) states = TAKE_PTR(hybrid_state); else - states = strv_new("disk", NULL); + states = strv_new("disk"); } else if (streq(verb, "suspend-then-hibernate")) { allow = allow_s2h > 0 || diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d446a2c588..9d0c40b93c 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3071,7 +3071,7 @@ static int start_unit(int argc, char *argv[], void *userdata) { } if (one_name) { - names = strv_new(one_name, NULL); + names = strv_new(one_name); if (!names) return log_oom(); } else { diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index 65313d4416..1566895ba3 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -717,7 +717,7 @@ static int acquire_search_path(const char *def, const char *envvar, char ***ret) if (strv_isempty(l)) { strv_free(l); - l = strv_new(def, NULL); + l = strv_new(def); if (!l) return log_oom(); } diff --git a/src/test/test-env-util.c b/src/test/test-env-util.c index ee9734fd66..4c33c7c13c 100644 --- a/src/test/test-env-util.c +++ b/src/test/test-env-util.c @@ -15,13 +15,13 @@ static void test_strv_env_delete(void) { _cleanup_strv_free_ char **a = NULL, **b = NULL, **c = NULL, **d = NULL; - a = strv_new("FOO=BAR", "WALDO=WALDO", "WALDO=", "PIEP", "SCHLUMPF=SMURF", NULL); + a = strv_new("FOO=BAR", "WALDO=WALDO", "WALDO=", "PIEP", "SCHLUMPF=SMURF"); assert_se(a); - b = strv_new("PIEP", "FOO", NULL); + b = strv_new("PIEP", "FOO"); assert_se(b); - c = strv_new("SCHLUMPF", NULL); + c = strv_new("SCHLUMPF"); assert_se(c); d = strv_env_delete(a, 2, b, c); @@ -45,7 +45,7 @@ static void test_strv_env_get(void) { static void test_strv_env_unset(void) { _cleanup_strv_free_ char **l = NULL; - l = strv_new("PIEP", "SCHLUMPF=SMURFF", "NANANANA=YES", NULL); + l = strv_new("PIEP", "SCHLUMPF=SMURFF", "NANANANA=YES"); assert_se(l); assert_se(strv_env_unset(l, "SCHLUMPF") == l); @@ -58,7 +58,7 @@ static void test_strv_env_unset(void) { static void test_strv_env_set(void) { _cleanup_strv_free_ char **l = NULL, **r = NULL; - l = strv_new("PIEP", "SCHLUMPF=SMURFF", "NANANANA=YES", NULL); + l = strv_new("PIEP", "SCHLUMPF=SMURFF", "NANANANA=YES"); assert_se(l); r = strv_env_set(l, "WALDO=WALDO"); @@ -74,10 +74,10 @@ static void test_strv_env_set(void) { static void test_strv_env_merge(void) { _cleanup_strv_free_ char **a = NULL, **b = NULL, **r = NULL; - a = strv_new("FOO=BAR", "WALDO=WALDO", "WALDO=", "PIEP", "SCHLUMPF=SMURF", NULL); + a = strv_new("FOO=BAR", "WALDO=WALDO", "WALDO=", "PIEP", "SCHLUMPF=SMURF"); assert_se(a); - b = strv_new("FOO=KKK", "FOO=", "PIEP=", "SCHLUMPF=SMURFF", "NANANANA=YES", NULL); + b = strv_new("FOO=KKK", "FOO=", "PIEP=", "SCHLUMPF=SMURFF", "NANANANA=YES"); assert_se(b); r = strv_env_merge(2, a, b); @@ -250,8 +250,7 @@ static void test_env_clean(void) { "xyz\n=xyz", "xyz=xyz\n", "another=one", - "another=final one", - NULL); + "another=final one"); assert_se(e); assert_se(!strv_env_is_valid(e)); assert_se(strv_env_clean(e) == e); diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c index e74b95231d..21a4538d74 100644 --- a/src/test/test-exec-util.c +++ b/src/test/test-exec-util.c @@ -346,7 +346,8 @@ static void test_environment_gathering(void) { /* now retest with "default" path passed in, as created by * manager_default_environment */ env = strv_free(env); - env = strv_new("PATH=" DEFAULT_PATH, NULL); + env = strv_new("PATH=" DEFAULT_PATH); + assert_se(env); r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, env); assert_se(r >= 0); diff --git a/src/test/test-nss.c b/src/test/test-nss.c index d9440682d2..20aa6cf01f 100644 --- a/src/test/test-nss.c +++ b/src/test/test-nss.c @@ -424,7 +424,7 @@ static int parse_argv(int argc, char **argv, size_t n_allocated = 0; if (argc > 1) - modules = strv_new(argv[1], NULL); + modules = strv_new(argv[1]); else modules = strv_new( #if ENABLE_NSS_MYHOSTNAME @@ -436,8 +436,7 @@ static int parse_argv(int argc, char **argv, #if ENABLE_NSS_MYMACHINES "mymachines", #endif - "dns", - NULL); + "dns"); if (!modules) return -ENOMEM; @@ -468,7 +467,7 @@ static int parse_argv(int argc, char **argv, if (!hostname) return -ENOMEM; - names = strv_new("localhost", "_gateway", "foo_no_such_host", hostname, NULL); + names = strv_new("localhost", "_gateway", "foo_no_such_host", hostname); if (!names) return -ENOMEM; diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 8b9686d50b..fd5f598701 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -293,7 +293,7 @@ static void test_strv_resolve(void) { assert_se(mkdtemp(tmp_dir) != NULL); - search_dirs = strv_new("/dir1", "/dir2", "/dir3", NULL); + search_dirs = strv_new("/dir1", "/dir2", "/dir3"); assert_se(search_dirs); STRV_FOREACH(d, search_dirs) { char *p = strappend(tmp_dir, *d); diff --git a/src/test/test-serialize.c b/src/test/test-serialize.c index b70eb3daa7..9a16688a67 100644 --- a/src/test/test-serialize.c +++ b/src/test/test-serialize.c @@ -138,7 +138,7 @@ static void test_deserialize_environment(void) { log_info("/* %s */", __func__); - assert_se(env = strv_new("A=1", NULL)); + assert_se(env = strv_new("A=1")); assert_se(deserialize_environment("B=2", &env) >= 0); assert_se(deserialize_environment("FOO%%=a\\177b\\nc\\td e", &env) >= 0); @@ -162,8 +162,7 @@ static void test_serialize_environment(void) { "B=2", "C=ąęółń", "D=D=a\\x0Ab", - "FOO%%=a\177b\nc\td e", - NULL)); + "FOO%%=a\177b\nc\td e")); assert_se(serialize_strv(f, "env", env) == 1); assert_se(fflush_and_check(f) == 0); diff --git a/src/test/test-sleep.c b/src/test/test-sleep.c index 442541a298..2a6d5e765a 100644 --- a/src/test/test-sleep.c +++ b/src/test/test-sleep.c @@ -50,14 +50,14 @@ static int test_fiemap(const char *path) { static void test_sleep(void) { _cleanup_strv_free_ char - **standby = strv_new("standby", NULL), - **mem = strv_new("mem", NULL), - **disk = strv_new("disk", NULL), - **suspend = strv_new("suspend", NULL), - **reboot = strv_new("reboot", NULL), - **platform = strv_new("platform", NULL), - **shutdown = strv_new("shutdown", NULL), - **freez = strv_new("freeze", NULL); + **standby = strv_new("standby"), + **mem = strv_new("mem"), + **disk = strv_new("disk"), + **suspend = strv_new("suspend"), + **reboot = strv_new("reboot"), + **platform = strv_new("platform"), + **shutdown = strv_new("shutdown"), + **freez = strv_new("freeze"); int r; log_info("/* %s */", __func__); diff --git a/src/test/test-strv.c b/src/test/test-strv.c index 5e392c75ac..63757afdce 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -441,8 +441,8 @@ static void test_strv_sort(void) { static void test_strv_extend_strv_concat(void) { _cleanup_strv_free_ char **a = NULL, **b = NULL; - a = strv_new("without", "suffix", NULL); - b = strv_new("with", "suffix", NULL); + a = strv_new("without", "suffix"); + b = strv_new("with", "suffix"); assert_se(a); assert_se(b); @@ -457,8 +457,8 @@ static void test_strv_extend_strv_concat(void) { static void test_strv_extend_strv(void) { _cleanup_strv_free_ char **a = NULL, **b = NULL, **n = NULL; - a = strv_new("abc", "def", "ghi", NULL); - b = strv_new("jkl", "mno", "abc", "pqr", NULL); + a = strv_new("abc", "def", "ghi"); + b = strv_new("jkl", "mno", "abc", "pqr"); assert_se(a); assert_se(b); @@ -483,7 +483,7 @@ static void test_strv_extend_strv(void) { static void test_strv_extend(void) { _cleanup_strv_free_ char **a = NULL, **b = NULL; - a = strv_new("test", "test1", NULL); + a = strv_new("test", "test1"); assert_se(a); assert_se(strv_extend(&a, "test2") >= 0); assert_se(strv_extend(&b, "test3") >= 0); @@ -497,7 +497,7 @@ static void test_strv_extend(void) { static void test_strv_extendf(void) { _cleanup_strv_free_ char **a = NULL, **b = NULL; - a = strv_new("test", "test1", NULL); + a = strv_new("test", "test1"); assert_se(a); assert_se(strv_extendf(&a, "test2 %s %d %s", "foo", 128, "bar") >= 0); assert_se(strv_extendf(&b, "test3 %s %s %d", "bar", "foo", 128) >= 0); @@ -513,7 +513,7 @@ static void test_strv_foreach(void) { unsigned i = 0; char **check; - a = strv_new("one", "two", "three", NULL); + a = strv_new("one", "two", "three"); assert_se(a); @@ -527,7 +527,7 @@ static void test_strv_foreach_backwards(void) { unsigned i = 2; char **check; - a = strv_new("one", "two", "three", NULL); + a = strv_new("one", "two", "three"); assert_se(a); @@ -547,8 +547,7 @@ static void test_strv_foreach_pair(void) { a = strv_new("pair_one", "pair_one", "pair_two", "pair_two", - "pair_three", "pair_three", - NULL); + "pair_three", "pair_three"); STRV_FOREACH_PAIR(x, y, a) { assert_se(streq(*x, *y)); @@ -608,7 +607,7 @@ static void test_strv_insert(void) { static void test_strv_push_prepend(void) { _cleanup_strv_free_ char **a = NULL; - a = strv_new("foo", "bar", "three", NULL); + a = strv_new("foo", "bar", "three"); assert_se(strv_push_prepend(&a, strdup("first")) >= 0); assert_se(streq(a[0], "first")); @@ -648,11 +647,11 @@ static void test_strv_equal(void) { _cleanup_strv_free_ char **b = NULL; _cleanup_strv_free_ char **c = NULL; - a = strv_new("one", "two", "three", NULL); + a = strv_new("one", "two", "three"); assert_se(a); - b = strv_new("one", "two", "three", NULL); + b = strv_new("one", "two", "three"); assert_se(a); - c = strv_new("one", "two", "three", "four", NULL); + c = strv_new("one", "two", "three", "four"); assert_se(a); assert_se(strv_equal(a, a)); @@ -667,19 +666,19 @@ static void test_strv_equal(void) { static void test_strv_is_uniq(void) { _cleanup_strv_free_ char **a = NULL, **b = NULL, **c = NULL, **d = NULL; - a = strv_new(NULL, NULL); + a = strv_new(NULL); assert_se(a); assert_se(strv_is_uniq(a)); - b = strv_new("foo", NULL); + b = strv_new("foo"); assert_se(b); assert_se(strv_is_uniq(b)); - c = strv_new("foo", "bar", NULL); + c = strv_new("foo", "bar"); assert_se(c); assert_se(strv_is_uniq(c)); - d = strv_new("foo", "bar", "waldo", "bar", "piep", NULL); + d = strv_new("foo", "bar", "waldo", "bar", "piep"); assert_se(d); assert_se(!strv_is_uniq(d)); } @@ -687,26 +686,26 @@ static void test_strv_is_uniq(void) { static void test_strv_reverse(void) { _cleanup_strv_free_ char **a = NULL, **b = NULL, **c = NULL, **d = NULL; - a = strv_new(NULL, NULL); + a = strv_new(NULL); assert_se(a); strv_reverse(a); assert_se(strv_isempty(a)); - b = strv_new("foo", NULL); + b = strv_new("foo"); assert_se(b); strv_reverse(b); assert_se(streq_ptr(b[0], "foo")); assert_se(streq_ptr(b[1], NULL)); - c = strv_new("foo", "bar", NULL); + c = strv_new("foo", "bar"); assert_se(c); strv_reverse(c); assert_se(streq_ptr(c[0], "bar")); assert_se(streq_ptr(c[1], "foo")); assert_se(streq_ptr(c[2], NULL)); - d = strv_new("foo", "bar", "waldo", NULL); + d = strv_new("foo", "bar", "waldo"); assert_se(d); strv_reverse(d); assert_se(streq_ptr(d[0], "waldo")); @@ -718,7 +717,7 @@ static void test_strv_reverse(void) { static void test_strv_shell_escape(void) { _cleanup_strv_free_ char **v = NULL; - v = strv_new("foo:bar", "bar,baz", "wal\\do", NULL); + v = strv_new("foo:bar", "bar,baz", "wal\\do"); assert_se(v); assert_se(strv_shell_escape(v, ",:")); assert_se(streq_ptr(v[0], "foo\\:bar")); @@ -752,7 +751,7 @@ static void test_strv_skip(void) { static void test_strv_extend_n(void) { _cleanup_strv_free_ char **v = NULL; - v = strv_new("foo", "bar", NULL); + v = strv_new("foo", "bar"); assert_se(v); assert_se(strv_extend_n(&v, "waldo", 3) >= 0); @@ -808,8 +807,8 @@ static void test_strv_free_free(void) { char ***t; assert_se(t = new(char**, 3)); - assert_se(t[0] = strv_new("a", "b", NULL)); - assert_se(t[1] = strv_new("c", "d", "e", NULL)); + assert_se(t[0] = strv_new("a", "b")); + assert_se(t[1] = strv_new("c", "d", "e")); t[2] = NULL; t = strv_free_free(t); @@ -839,7 +838,7 @@ static void test_strv_fnmatch(void) { assert_se(!strv_fnmatch(STRV_MAKE_EMPTY, "a", 0)); - v = strv_new("*\\*", NULL); + v = strv_new("*\\*"); assert_se(!strv_fnmatch(v, "\\", 0)); assert_se(strv_fnmatch(v, "\\", FNM_NOESCAPE)); } -- cgit v1.2.3 From 1ad6e8b302e87b6891a2bfc35ad397b0afe3d940 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 31 Oct 2018 15:49:19 +0100 Subject: core: split environment block mantained by PID 1's Manager object in two This splits the "environment" field of Manager into two: transient_environment and client_environment. The former is generated from configuration file, kernel cmdline, environment generators. The latter is the one the user can control with "systemctl set-environment" and similar. Both sets are merged transparently whenever needed. Separating the two sets has the benefit that we can safely flush out the former while keeping the latter during daemon reload cycles, so that env var settings from env generators or configuration files do not accumulate, but dynamic API changes are kept around. Note that this change is not entirely transparent to users: if the user first uses "set-environment" to override a transient variable, and then uses "unset-environment" to unset it again things will revert to the original transient variable now, while previously the variable was fully removed. This change in behaviour should not matter too much though I figure. Fixes: #9972 --- src/core/dbus-manager.c | 32 +++++++++++++++--- src/core/main.c | 2 +- src/core/manager.c | 89 +++++++++++++++++++++++++++++++++++++------------ src/core/manager.h | 8 +++-- src/core/mount.c | 4 ++- src/core/service.c | 4 ++- src/core/socket.c | 4 ++- src/core/swap.c | 4 ++- src/core/unit.c | 11 ++++-- src/core/unit.h | 2 +- 10 files changed, 124 insertions(+), 36 deletions(-) (limited to 'src') diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 480a143091..677d9c3226 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -216,6 +216,30 @@ static int property_get_progress( return sd_bus_message_append(reply, "d", d); } +static int property_get_environment( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + _cleanup_strv_free_ char **l = NULL; + Manager *m = userdata; + int r; + + assert(bus); + assert(reply); + assert(m); + + r = manager_get_effective_environment(m, &l); + if (r < 0) + return r; + + return sd_bus_message_append_strv(reply, l); +} + static int property_get_show_status( sd_bus *bus, const char *path, @@ -1578,7 +1602,7 @@ static int method_set_environment(sd_bus_message *message, void *userdata, sd_bu if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - r = manager_environment_add(m, NULL, plus); + r = manager_client_environment_modify(m, NULL, plus); if (r < 0) return r; @@ -1610,7 +1634,7 @@ static int method_unset_environment(sd_bus_message *message, void *userdata, sd_ if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - r = manager_environment_add(m, minus, NULL); + r = manager_client_environment_modify(m, minus, NULL); if (r < 0) return r; @@ -1648,7 +1672,7 @@ static int method_unset_and_set_environment(sd_bus_message *message, void *userd if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - r = manager_environment_add(m, minus, plus); + r = manager_client_environment_modify(m, minus, plus); if (r < 0) return r; @@ -2432,7 +2456,7 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_PROPERTY("NInstalledJobs", "u", bus_property_get_unsigned, offsetof(Manager, n_installed_jobs), 0), SD_BUS_PROPERTY("NFailedJobs", "u", bus_property_get_unsigned, offsetof(Manager, n_failed_jobs), 0), SD_BUS_PROPERTY("Progress", "d", property_get_progress, 0, 0), - SD_BUS_PROPERTY("Environment", "as", NULL, offsetof(Manager, environment), 0), + SD_BUS_PROPERTY("Environment", "as", property_get_environment, 0, 0), SD_BUS_PROPERTY("ConfirmSpawn", "b", bus_property_get_bool, offsetof(Manager, confirm_spawn), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("ShowStatus", "b", property_get_show_status, 0, 0), SD_BUS_PROPERTY("UnitPath", "as", NULL, offsetof(Manager, lookup_paths.search_path), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/core/main.c b/src/core/main.c index 807f5457c2..8a09ab67b5 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -761,7 +761,7 @@ static void set_manager_defaults(Manager *m) { m->default_tasks_max = arg_default_tasks_max; manager_set_default_rlimits(m, arg_default_rlimit); - manager_environment_add(m, NULL, arg_default_environment); + manager_transient_environment_add(m, arg_default_environment); } static void set_manager_settings(Manager *m) { diff --git a/src/core/manager.c b/src/core/manager.c index adf0319701..bee415c725 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -572,12 +572,11 @@ static int manager_setup_signals(Manager *m) { return 0; } -static void manager_sanitize_environment(Manager *m) { - assert(m); +static char** sanitize_environment(char **l) { /* Let's remove some environment variables that we need ourselves to communicate with our clients */ strv_env_unset_many( - m->environment, + l, "EXIT_CODE", "EXIT_STATUS", "INVOCATION_ID", @@ -596,12 +595,16 @@ static void manager_sanitize_environment(Manager *m) { NULL); /* Let's order the environment alphabetically, just to make it pretty */ - strv_sort(m->environment); + strv_sort(l); + + return l; } static int manager_default_environment(Manager *m) { assert(m); + m->transient_environment = strv_free(m->transient_environment); + if (MANAGER_IS_SYSTEM(m)) { /* The system manager always starts with a clean * environment for its children. It does not import @@ -610,20 +613,19 @@ static int manager_default_environment(Manager *m) { * The initial passed environment is untouched to keep * /proc/self/environ valid; it is used for tagging * the init process inside containers. */ - m->environment = strv_new("PATH=" DEFAULT_PATH, - NULL); + m->transient_environment = strv_new("PATH=" DEFAULT_PATH); /* Import locale variables LC_*= from configuration */ - locale_setup(&m->environment); + (void) locale_setup(&m->transient_environment); } else /* The user manager passes its own environment * along to its children. */ - m->environment = strv_copy(environ); + m->transient_environment = strv_copy(environ); - if (!m->environment) + if (!m->transient_environment) return -ENOMEM; - manager_sanitize_environment(m); + sanitize_environment(m->transient_environment); return 0; } @@ -1346,7 +1348,8 @@ Manager* manager_free(Manager *m) { free(m->notify_socket); lookup_paths_free(&m->lookup_paths); - strv_free(m->environment); + strv_free(m->transient_environment); + strv_free(m->client_environment); hashmap_free(m->cgroup_unit); set_free_free(m->unit_path_cache); @@ -3145,7 +3148,7 @@ int manager_serialize( } if (!switching_root) - (void) serialize_strv(f, "env", m->environment); + (void) serialize_strv(f, "env", m->client_environment); if (m->notify_fd >= 0) { r = serialize_fd(f, fds, "notify-fd", m->notify_fd); @@ -3378,7 +3381,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { manager_override_log_target(m, target); } else if (startswith(l, "env=")) { - r = deserialize_environment(l + 4, &m->environment); + r = deserialize_environment(l + 4, &m->client_environment); if (r < 0) log_notice_errno(r, "Failed to parse environment entry: \"%s\", ignoring: %m", l); @@ -3801,7 +3804,11 @@ static const char* user_env_generator_binary_paths[] = { static int manager_run_environment_generators(Manager *m) { char **tmp = NULL; /* this is only used in the forked process, no cleanup here */ const char **paths; - void* args[] = {&tmp, &tmp, &m->environment}; + void* args[] = { + [STDOUT_GENERATE] = &tmp, + [STDOUT_COLLECT] = &tmp, + [STDOUT_CONSUME] = &m->transient_environment, + }; if (MANAGER_IS_TEST_RUN(m) && !(m->test_run_flags & MANAGER_TEST_RUN_ENV_GENERATORS)) return 0; @@ -3811,7 +3818,7 @@ static int manager_run_environment_generators(Manager *m) { if (!generator_path_any(paths)) return 0; - return execute_directories(paths, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, m->environment); + return execute_directories(paths, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, m->transient_environment); } static int manager_run_generators(Manager *m) { @@ -3845,7 +3852,7 @@ static int manager_run_generators(Manager *m) { RUN_WITH_UMASK(0022) (void) execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC, - NULL, NULL, (char**) argv, m->environment); + NULL, NULL, (char**) argv, m->transient_environment); r = 0; @@ -3854,11 +3861,36 @@ finish: return r; } -int manager_environment_add(Manager *m, char **minus, char **plus) { +int manager_transient_environment_add(Manager *m, char **plus) { + char **a; + + assert(m); + + if (strv_isempty(plus)) + return 0; + + a = strv_env_merge(2, m->transient_environment, plus); + if (!a) + return -ENOMEM; + + sanitize_environment(a); + + return strv_free_and_replace(m->transient_environment, a); +} + +int manager_client_environment_modify( + Manager *m, + char **minus, + char **plus) { + char **a = NULL, **b = NULL, **l; + assert(m); - l = m->environment; + if (strv_isempty(minus) && strv_isempty(plus)) + return 0; + + l = m->client_environment; if (!strv_isempty(minus)) { a = strv_env_delete(l, 1, minus); @@ -3878,16 +3910,29 @@ int manager_environment_add(Manager *m, char **minus, char **plus) { l = b; } - if (m->environment != l) - strv_free(m->environment); + if (m->client_environment != l) + strv_free(m->client_environment); + if (a != l) strv_free(a); if (b != l) strv_free(b); - m->environment = l; - manager_sanitize_environment(m); + m->client_environment = sanitize_environment(l); + return 0; +} + +int manager_get_effective_environment(Manager *m, char ***ret) { + char **l; + + assert(m); + assert(ret); + + l = strv_env_merge(2, m->transient_environment, m->client_environment); + if (!l) + return -ENOMEM; + *ret = l; return 0; } diff --git a/src/core/manager.h b/src/core/manager.h index 1daa9f2308..45fa5d35d8 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -212,7 +212,8 @@ struct Manager { LookupPaths lookup_paths; Set *unit_path_cache; - char **environment; + char **transient_environment; /* The environment, as determined from config files, kernel cmdline and environment generators */ + char **client_environment; /* Environment variables created by clients through the bus API */ usec_t runtime_watchdog; usec_t shutdown_watchdog; @@ -442,7 +443,10 @@ void manager_clear_jobs(Manager *m); unsigned manager_dispatch_load_queue(Manager *m); -int manager_environment_add(Manager *m, char **minus, char **plus); +int manager_transient_environment_add(Manager *m, char **plus); +int manager_client_environment_modify(Manager *m, char **minus, char **plus); +int manager_get_effective_environment(Manager *m, char ***ret); + int manager_set_default_rlimits(Manager *m, struct rlimit **default_rlimit); int manager_loop(Manager *m); diff --git a/src/core/mount.c b/src/core/mount.c index dafd6ea90b..14f1fa9e50 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -769,7 +769,9 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) { if (r < 0) return r; - unit_set_exec_params(UNIT(m), &exec_params); + r = unit_set_exec_params(UNIT(m), &exec_params); + if (r < 0) + return r; r = exec_spawn(UNIT(m), c, diff --git a/src/core/service.c b/src/core/service.c index f6f3ecc0e6..8fc8d1ff94 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1523,7 +1523,9 @@ static int service_spawn( } } - unit_set_exec_params(UNIT(s), &exec_params); + r = unit_set_exec_params(UNIT(s), &exec_params); + if (r < 0) + return r; final_env = strv_env_merge(2, exec_params.environment, our_env, NULL); if (!final_env) diff --git a/src/core/socket.c b/src/core/socket.c index d54dadbacf..ad8b9b6a41 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1882,7 +1882,9 @@ static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) { if (r < 0) return r; - unit_set_exec_params(UNIT(s), &exec_params); + r = unit_set_exec_params(UNIT(s), &exec_params); + if (r < 0) + return r; r = exec_spawn(UNIT(s), c, diff --git a/src/core/swap.c b/src/core/swap.c index dd1ef8e88e..429b2b1062 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -618,7 +618,9 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { if (r < 0) goto fail; - unit_set_exec_params(UNIT(s), &exec_params); + r = unit_set_exec_params(UNIT(s), &exec_params); + if (r < 0) + goto fail; r = exec_spawn(UNIT(s), c, diff --git a/src/core/unit.c b/src/core/unit.c index e35f7f26d1..c0ac8e5c3a 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5023,12 +5023,17 @@ int unit_acquire_invocation_id(Unit *u) { return 0; } -void unit_set_exec_params(Unit *u, ExecParameters *p) { +int unit_set_exec_params(Unit *u, ExecParameters *p) { + int r; + assert(u); assert(p); /* Copy parameters from manager */ - p->environment = u->manager->environment; + r = manager_get_effective_environment(u->manager, &p->environment); + if (r < 0) + return r; + p->confirm_spawn = manager_get_confirm_spawn(u->manager); p->cgroup_supported = u->manager->cgroup_supported; p->prefix = u->manager->prefix; @@ -5037,6 +5042,8 @@ void unit_set_exec_params(Unit *u, ExecParameters *p) { /* Copy paramaters from unit */ p->cgroup_path = u->cgroup_path; SET_FLAG(p->flags, EXEC_CGROUP_DELEGATE, unit_cgroup_delegate(u)); + + return 0; } int unit_fork_helper_process(Unit *u, const char *name, pid_t *ret) { diff --git a/src/core/unit.h b/src/core/unit.h index 1aea658e97..b6ef9d97ee 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -777,7 +777,7 @@ int unit_acquire_invocation_id(Unit *u); bool unit_shall_confirm_spawn(Unit *u); -void unit_set_exec_params(Unit *s, ExecParameters *p); +int unit_set_exec_params(Unit *s, ExecParameters *p); int unit_fork_helper_process(Unit *u, const char *name, pid_t *ret); -- cgit v1.2.3