From 6171b8225f468fc90c79087f653ad4fc7ac21453 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 17:32:01 +0100 Subject: [PATCH 01/33] core: drop specifier expansion when parsing transient dbus properties Specifier expansion (much like C escape handling) should be a helper for writing unit files, but should be nothing we do on programatic APIs. For those, the client can do the necessary replacements anyway, and we really should be careful with doing such string processing of data we get via lower level programmatic APIs. We currently do specifier expansion only for the env var transient unit APIs, no other properties do this. Let's remove it here too, to be fully systematic. Yes, in a way this is API breakage, but then again this API isn't documented yet, and an outlier, hence let's clear this up now, before it is too late. --- src/core/dbus-execute.c | 37 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 7b0e91cdc4..57ccdcbe33 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -2061,35 +2061,30 @@ int bus_exec_context_set_transient_property( } else if (streq(name, "Environment")) { - _cleanup_strv_free_ char **l = NULL, **q = NULL; + _cleanup_strv_free_ char **l = NULL; r = sd_bus_message_read_strv(message, &l); if (r < 0) return r; - r = unit_full_printf_strv(u, l, &q); - if (r < 0) - return r; - - if (!strv_env_is_valid(q)) + if (!strv_env_is_valid(l)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid environment block."); if (mode != UNIT_CHECK) { - if (strv_length(q) == 0) { + if (strv_length(l) == 0) { c->environment = strv_free(c->environment); unit_write_drop_in_private_format(u, mode, name, "Environment="); } else { _cleanup_free_ char *joined = NULL; char **e; - e = strv_env_merge(2, c->environment, q); + e = strv_env_merge(2, c->environment, l); if (!e) return -ENOMEM; strv_free(c->environment); c->environment = e; - /* We write just the new settings out to file, with unresolved specifiers */ joined = strv_join_quoted(l); if (!joined) return -ENOMEM; @@ -2102,28 +2097,24 @@ int bus_exec_context_set_transient_property( } else if (streq(name, "UnsetEnvironment")) { - _cleanup_strv_free_ char **l = NULL, **q = NULL; + _cleanup_strv_free_ char **l = NULL; r = sd_bus_message_read_strv(message, &l); if (r < 0) return r; - r = unit_full_printf_strv(u, l, &q); - if (r < 0) - return r; - - if (!strv_env_name_or_assignment_is_valid(q)) + if (!strv_env_name_or_assignment_is_valid(l)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid UnsetEnvironment= list."); if (mode != UNIT_CHECK) { - if (strv_length(q) == 0) { + if (strv_length(l) == 0) { c->unset_environment = strv_free(c->unset_environment); unit_write_drop_in_private_format(u, mode, name, "UnsetEnvironment="); } else { _cleanup_free_ char *joined = NULL; char **e; - e = strv_env_merge(2, c->unset_environment, q); + e = strv_env_merge(2, c->unset_environment, l); if (!e) return -ENOMEM; @@ -2250,17 +2241,13 @@ int bus_exec_context_set_transient_property( } else if (streq(name, "PassEnvironment")) { - _cleanup_strv_free_ char **l = NULL, **q = NULL; + _cleanup_strv_free_ char **l = NULL; r = sd_bus_message_read_strv(message, &l); if (r < 0) return r; - r = unit_full_printf_strv(u, l, &q); - if (r < 0) - return r; - - if (!strv_env_name_is_valid(q)) + if (!strv_env_name_is_valid(l)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid PassEnvironment= block."); if (mode != UNIT_CHECK) { @@ -2270,10 +2257,6 @@ int bus_exec_context_set_transient_property( } else { _cleanup_free_ char *joined = NULL; - r = strv_extend_strv(&c->pass_environment, q, true); - if (r < 0) - return r; - /* We write just the new settings out to file, with unresolved specifiers. */ joined = strv_join_quoted(l); if (!joined) From 9d73565ac02eb1cab448258a03f8dad7faa00b17 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 19:38:49 +0100 Subject: [PATCH 02/33] string-util: update strreplace() a bit, use GREEDY_REALLOC() --- src/basic/string-util.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 06cdcf1c6e..312e83433e 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -603,26 +603,26 @@ char* strshorten(char *s, size_t l) { } char *strreplace(const char *text, const char *old_string, const char *new_string) { + size_t l, old_len, new_len, allocated = 0; + char *t, *ret = NULL; const char *f; - char *t, *r; - size_t l, old_len, new_len; - assert(text); assert(old_string); assert(new_string); + if (!text) + return NULL; + old_len = strlen(old_string); new_len = strlen(new_string); l = strlen(text); - r = new(char, l+1); - if (!r) + if (!GREEDY_REALLOC(ret, allocated, l+1)) return NULL; f = text; - t = r; + t = ret; while (*f) { - char *a; size_t d, nl; if (!startswith(f, old_string)) { @@ -630,25 +630,21 @@ char *strreplace(const char *text, const char *old_string, const char *new_strin continue; } - d = t - r; + d = t - ret; nl = l - old_len + new_len; - a = realloc(r, nl + 1); - if (!a) - goto oom; + + if (!GREEDY_REALLOC(ret, allocated, nl + 1)) + return mfree(ret); l = nl; - r = a; - t = r + d; + t = ret + d; t = stpcpy(t, new_string); f += old_len; } *t = 0; - return r; - -oom: - return mfree(r); + return ret; } char *strip_tab_ansi(char **ibuf, size_t *_isz) { From e82f30d17f3dd8d4209b8eec3c0c6ab529e9c182 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 17:52:31 +0100 Subject: [PATCH 03/33] specifier: add helper for escaping '%' characters to avoid making them subject for expansion This is ultimately just a wrapper around strreplace(), but it makes things a bit more self-descriptive. --- src/shared/specifier.c | 30 ++++++++++++++++++ src/shared/specifier.h | 8 +++++ src/test/meson.build | 4 +++ src/test/test-specifier.c | 66 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+) create mode 100644 src/test/test-specifier.c diff --git a/src/shared/specifier.c b/src/shared/specifier.c index b0f00dbb5a..dc7be0a993 100644 --- a/src/shared/specifier.c +++ b/src/shared/specifier.c @@ -32,6 +32,7 @@ #include "macro.h" #include "specifier.h" #include "string-util.h" +#include "strv.h" /* * Generic infrastructure for replacing %x style specifiers in @@ -191,3 +192,32 @@ int specifier_kernel_release(char specifier, void *data, void *userdata, char ** *ret = n; return 0; } + +int specifier_escape_strv(char **l, char ***ret) { + char **z, **p, **q; + + assert(ret); + + if (strv_isempty(l)) { + *ret = NULL; + return 0; + } + + z = new(char*, strv_length(l)+1); + if (!z) + return -ENOMEM; + + for (p = l, q = z; *p; p++, q++) { + + *q = specifier_escape(*p); + if (!*q) { + strv_free(z); + return -ENOMEM; + } + } + + *q = NULL; + *ret = z; + + return 0; +} diff --git a/src/shared/specifier.h b/src/shared/specifier.h index 5d2b859f64..0401e13e50 100644 --- a/src/shared/specifier.h +++ b/src/shared/specifier.h @@ -20,6 +20,8 @@ along with systemd; If not, see . ***/ +#include "string-util.h" + typedef int (*SpecifierCallback)(char specifier, void *data, void *userdata, char **ret); typedef struct Specifier { @@ -36,3 +38,9 @@ int specifier_machine_id(char specifier, void *data, void *userdata, char **ret) int specifier_boot_id(char specifier, void *data, void *userdata, char **ret); int specifier_host_name(char specifier, void *data, void *userdata, char **ret); int specifier_kernel_release(char specifier, void *data, void *userdata, char **ret); + +static inline char* specifier_escape(const char *string) { + return strreplace(string, "%", "%%"); +} + +int specifier_escape_strv(char **l, char ***ret); diff --git a/src/test/meson.build b/src/test/meson.build index 9a8d620bac..efaa259d76 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -249,6 +249,10 @@ tests += [ [], []], + [['src/test/test-specifier.c'], + [], + []], + [['src/test/test-string-util.c'], [], []], diff --git a/src/test/test-specifier.c b/src/test/test-specifier.c new file mode 100644 index 0000000000..6bf312057a --- /dev/null +++ b/src/test/test-specifier.c @@ -0,0 +1,66 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +/*** + This file is part of systemd. + + Copyright 2017 Lennart Poettering + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include "alloc-util.h" +#include "log.h" +#include "specifier.h" +#include "string-util.h" +#include "strv.h" + +static void test_specifier_escape_one(const char *a, const char *b) { + _cleanup_free_ char *x = NULL; + + x = specifier_escape(a); + assert_se(streq_ptr(x, b)); +} + +static void test_specifier_escape(void) { + test_specifier_escape_one(NULL, NULL); + test_specifier_escape_one("", ""); + test_specifier_escape_one("%", "%%"); + test_specifier_escape_one("foo bar", "foo bar"); + test_specifier_escape_one("foo%bar", "foo%%bar"); + test_specifier_escape_one("%%%%%", "%%%%%%%%%%"); +} + +static void test_specifier_escape_strv_one(char **a, char **b) { + _cleanup_strv_free_ char **x = NULL; + + assert_se(specifier_escape_strv(a, &x) >= 0); + assert_se(strv_equal(x, b)); +} + +static void test_specifier_escape_strv(void) { + test_specifier_escape_strv_one(NULL, NULL); + test_specifier_escape_strv_one(STRV_MAKE(NULL), STRV_MAKE(NULL)); + test_specifier_escape_strv_one(STRV_MAKE(""), STRV_MAKE("")); + test_specifier_escape_strv_one(STRV_MAKE("foo"), STRV_MAKE("foo")); + test_specifier_escape_strv_one(STRV_MAKE("%"), STRV_MAKE("%%")); + test_specifier_escape_strv_one(STRV_MAKE("foo", "%", "foo%", "%foo", "foo%foo", "quux", "%%%"), STRV_MAKE("foo", "%%", "foo%%", "%%foo", "foo%%foo", "quux", "%%%%%%")); +} + +int main(int argc, char *argv[]) { + log_set_max_level(LOG_DEBUG); + + test_specifier_escape(); + test_specifier_escape_strv(); + + return 0; +} From 36444d2213e7a3c5d23e0c4a04e90ee4e8c11a2e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 17:57:56 +0100 Subject: [PATCH 04/33] specifier: unify specifier implementations for user-related specifiers The code in install-printf.c and unit-printf.c for these is pretty much the same and very generic. Let's move this all over to the generic specifier.c, and share the implementations. --- src/core/unit-printf.c | 40 ---------------------------------- src/shared/install-printf.c | 28 ------------------------ src/shared/specifier.c | 43 +++++++++++++++++++++++++++++++++++++ src/shared/specifier.h | 5 +++++ src/test/test-unit-name.c | 2 +- 5 files changed, 49 insertions(+), 69 deletions(-) diff --git a/src/core/unit-printf.c b/src/core/unit-printf.c index c8896c41e2..e61e0d1475 100644 --- a/src/core/unit-printf.c +++ b/src/core/unit-printf.c @@ -158,46 +158,6 @@ static int specifier_special_directory(char specifier, void *data, void *userdat return 0; } -static int specifier_user_name(char specifier, void *data, void *userdata, char **ret) { - char *t; - - /* If we are UID 0 (root), this will not result in NSS, - * otherwise it might. This is good, as we want to be able to - * run this in PID 1, where our user ID is 0, but where NSS - * lookups are not allowed. */ - - t = getusername_malloc(); - if (!t) - return -ENOMEM; - - *ret = t; - return 0; -} - -static int specifier_user_id(char specifier, void *data, void *userdata, char **ret) { - - if (asprintf(ret, UID_FMT, getuid()) < 0) - return -ENOMEM; - - return 0; -} - -static int specifier_user_home(char specifier, void *data, void *userdata, char **ret) { - - /* On PID 1 (which runs as root) this will not result in NSS, - * which is good. See above */ - - return get_home_dir(ret); -} - -static int specifier_user_shell(char specifier, void *data, void *userdata, char **ret) { - - /* On PID 1 (which runs as root) this will not result in NSS, - * which is good. See above */ - - return get_shell(ret); -} - int unit_name_printf(Unit *u, const char* format, char **ret) { /* diff --git a/src/shared/install-printf.c b/src/shared/install-printf.c index f8d0e53eca..aaab2e6665 100644 --- a/src/shared/install-printf.c +++ b/src/shared/install-printf.c @@ -103,34 +103,6 @@ static int specifier_instance(char specifier, void *data, void *userdata, char * return 0; } -static int specifier_user_name(char specifier, void *data, void *userdata, char **ret) { - char *t; - - /* If we are UID 0 (root), this will not result in NSS, - * otherwise it might. This is good, as we want to be able to - * run this in PID 1, where our user ID is 0, but where NSS - * lookups are not allowed. - - * We don't user getusername_malloc() here, because we don't want to look - * at $USER, to remain consistent with specifer_user_id() below. - */ - - t = uid_to_name(getuid()); - if (!t) - return -ENOMEM; - - *ret = t; - return 0; -} - -static int specifier_user_id(char specifier, void *data, void *userdata, char **ret) { - - if (asprintf(ret, UID_FMT, getuid()) < 0) - return -ENOMEM; - - return 0; -} - int install_full_printf(UnitFileInstallInfo *i, const char *format, char **ret) { /* This is similar to unit_full_printf() but does not support diff --git a/src/shared/specifier.c b/src/shared/specifier.c index dc7be0a993..9bef890346 100644 --- a/src/shared/specifier.c +++ b/src/shared/specifier.c @@ -33,6 +33,7 @@ #include "specifier.h" #include "string-util.h" #include "strv.h" +#include "user-util.h" /* * Generic infrastructure for replacing %x style specifiers in @@ -193,6 +194,48 @@ int specifier_kernel_release(char specifier, void *data, void *userdata, char ** return 0; } +int specifier_user_name(char specifier, void *data, void *userdata, char **ret) { + char *t; + + /* If we are UID 0 (root), this will not result in NSS, otherwise it might. This is good, as we want to be able + * to run this in PID 1, where our user ID is 0, but where NSS lookups are not allowed. + + * We don't user getusername_malloc() here, because we don't want to look at $USER, to remain consistent with + * specifer_user_id() below. + */ + + t = uid_to_name(getuid()); + if (!t) + return -ENOMEM; + + *ret = t; + return 0; +} + +int specifier_user_id(char specifier, void *data, void *userdata, char **ret) { + + if (asprintf(ret, UID_FMT, getuid()) < 0) + return -ENOMEM; + + return 0; +} + +int specifier_user_home(char specifier, void *data, void *userdata, char **ret) { + + /* On PID 1 (which runs as root) this will not result in NSS, + * which is good. See above */ + + return get_home_dir(ret); +} + +int specifier_user_shell(char specifier, void *data, void *userdata, char **ret) { + + /* On PID 1 (which runs as root) this will not result in NSS, + * which is good. See above */ + + return get_shell(ret); +} + int specifier_escape_strv(char **l, char ***ret) { char **z, **p, **q; diff --git a/src/shared/specifier.h b/src/shared/specifier.h index 0401e13e50..bd6bc55577 100644 --- a/src/shared/specifier.h +++ b/src/shared/specifier.h @@ -39,6 +39,11 @@ int specifier_boot_id(char specifier, void *data, void *userdata, char **ret); int specifier_host_name(char specifier, void *data, void *userdata, char **ret); int specifier_kernel_release(char specifier, void *data, void *userdata, char **ret); +int specifier_user_name(char specifier, void *data, void *userdata, char **ret); +int specifier_user_id(char specifier, void *data, void *userdata, char **ret); +int specifier_user_home(char specifier, void *data, void *userdata, char **ret); +int specifier_user_shell(char specifier, void *data, void *userdata, char **ret); + static inline char* specifier_escape(const char *string) { return strreplace(string, "%", "%%"); } diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c index 013e87e73b..e24892b590 100644 --- a/src/test/test-unit-name.c +++ b/src/test/test-unit-name.c @@ -207,7 +207,7 @@ static int test_unit_printf(void) { assert_se(specifier_machine_id('m', NULL, NULL, &mid) >= 0 && mid); assert_se(specifier_boot_id('b', NULL, NULL, &bid) >= 0 && bid); assert_se(host = gethostname_malloc()); - assert_se(user = getusername_malloc()); + assert_se(user = uid_to_name(getuid())); assert_se(asprintf(&uid, UID_FMT, getuid())); assert_se(get_home_dir(&home) >= 0); assert_se(get_shell(&shell) >= 0); From 35838def32a9ab13a12aa886b847b5a0d6056899 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 17:58:51 +0100 Subject: [PATCH 05/33] CODING_STYLE: document the order in which unescaping + specifier expansion should be done --- CODING_STYLE | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CODING_STYLE b/CODING_STYLE index 9dcc09030c..b090db370f 100644 --- a/CODING_STYLE +++ b/CODING_STYLE @@ -434,3 +434,8 @@ that interrupted system calls are automatically restarted, and we minimize hassles with handling EINTR (in particular as EINTR handling is pretty broken on Linux). + +- When applying C-style unescaping as well as specifier expansion on the same + string, always apply the C-style unescaping fist, followed by the specifier + expansion. When doing the reverse, make sure to escape '%' in specifier-style + first (i.e. '%' → '%%'), and then do C-style escaping where necessary. From 613613f1ee2914d8ccee63b2d1b19b9f2a1661a7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 19:23:31 +0100 Subject: [PATCH 06/33] core: use config_parse_unit_string_printf() for decoding RebootArgument= All other cases where we accept a reboot argument are decoded with config_parse_unit_string_printf() rather than config_parse_unit_path_printf(), and that's really the only thing what makes sense here, hence adjust this here, too. --- src/core/load-fragment-gperf.gperf.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index eff2b0e741..ef990161ad 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -291,7 +291,7 @@ Service.StartLimitInterval, config_parse_sec, 0, Service.StartLimitBurst, config_parse_unsigned, 0, offsetof(Unit, start_limit.burst) Service.StartLimitAction, config_parse_emergency_action, 0, offsetof(Unit, start_limit_action) Service.FailureAction, config_parse_emergency_action, 0, offsetof(Unit, failure_action) -Service.RebootArgument, config_parse_unit_path_printf, 0, offsetof(Unit, reboot_arg) +Service.RebootArgument, config_parse_unit_string_printf, 0, offsetof(Unit, reboot_arg) Service.Type, config_parse_service_type, 0, offsetof(Service, type) Service.Restart, config_parse_service_restart, 0, offsetof(Service, restart) Service.PermissionsStartOnly, config_parse_bool, 0, offsetof(Service, permissions_start_only) From 6c5def2ff201ecbd07e39c794c9392f9ef2d8dcb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 19:50:52 +0100 Subject: [PATCH 07/33] core: process C-style escapes in UnsetEnvironment= We process C-style escapes in Environment=, hence we should process it in UnsetEnvironment= too, as the latter accepts assignments much like the former, including arbitrary values specified by the user. --- src/core/load-fragment.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index ac58ce8a14..220e617699 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2474,7 +2474,7 @@ int config_parse_unset_environ( for (;;) { _cleanup_free_ char *word = NULL, *k = NULL; - r = extract_first_word(&rvalue, &word, NULL, EXTRACT_QUOTES); + r = extract_first_word(&rvalue, &word, NULL, EXTRACT_CUNESCAPE|EXTRACT_QUOTES); if (r == 0) break; if (r == -ENOMEM) From b238be1e0d13f587d3a48645cea3f47d1dda3475 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 20:03:51 +0100 Subject: [PATCH 08/33] core: enable specifier expansion for What=/Where=/Type=/SourcePath= too Using specifiers in these settings isn't particularly useful by itself, but it unifies behaviour a bit. It's kinda surprising that What= in mount units resolves specifies, but Where= does not. Hence let's add that too. Also, it's surprising Where=/What= in mount units behaves differently than in automount and swap units, hence resolve specifiers there too. Then, Type= in mount units is nowadays an arbitrary, sometimes non-trivial string (think fuse!), hence let's also expand specifiers there, to match the rest of the mount settings. This has the benefit that when writing code that generates unit files, less care has to be taken to check whether escaping of specifiers is necessary or not: broadly everything that takes arbitrary user strings now does specifier expansion, while enums/numerics/booleans do not. --- src/core/load-fragment-gperf.gperf.m4 | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index ef990161ad..240f331778 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -189,7 +189,7 @@ $1.NetClass, config_parse_warn_compat, DISABLED_LE )m4_dnl Unit.Description, config_parse_unit_string_printf, 0, offsetof(Unit, description) Unit.Documentation, config_parse_documentation, 0, offsetof(Unit, documentation) -Unit.SourcePath, config_parse_path, 0, offsetof(Unit, source_path) +Unit.SourcePath, config_parse_unit_path_printf, 0, offsetof(Unit, source_path) Unit.Requires, config_parse_unit_deps, UNIT_REQUIRES, 0 Unit.Requisite, config_parse_unit_deps, UNIT_REQUISITE, 0 Unit.Wants, config_parse_unit_deps, UNIT_WANTS, 0 @@ -382,9 +382,9 @@ CGROUP_CONTEXT_CONFIG_ITEMS(Socket)m4_dnl KILL_CONTEXT_CONFIG_ITEMS(Socket)m4_dnl m4_dnl Mount.What, config_parse_unit_string_printf, 0, offsetof(Mount, parameters_fragment.what) -Mount.Where, config_parse_path, 0, offsetof(Mount, where) +Mount.Where, config_parse_unit_path_printf, 0, offsetof(Mount, where) Mount.Options, config_parse_unit_string_printf, 0, offsetof(Mount, parameters_fragment.options) -Mount.Type, config_parse_string, 0, offsetof(Mount, parameters_fragment.fstype) +Mount.Type, config_parse_unit_string_printf, 0, offsetof(Mount, parameters_fragment.fstype) Mount.TimeoutSec, config_parse_sec_fix_0, 0, offsetof(Mount, timeout_usec) Mount.DirectoryMode, config_parse_mode, 0, offsetof(Mount, directory_mode) Mount.SloppyOptions, config_parse_bool, 0, offsetof(Mount, sloppy_options) @@ -394,11 +394,11 @@ EXEC_CONTEXT_CONFIG_ITEMS(Mount)m4_dnl CGROUP_CONTEXT_CONFIG_ITEMS(Mount)m4_dnl KILL_CONTEXT_CONFIG_ITEMS(Mount)m4_dnl m4_dnl -Automount.Where, config_parse_path, 0, offsetof(Automount, where) +Automount.Where, config_parse_unit_path_printf, 0, offsetof(Automount, where) Automount.DirectoryMode, config_parse_mode, 0, offsetof(Automount, directory_mode) Automount.TimeoutIdleSec, config_parse_sec_fix_0, 0, offsetof(Automount, timeout_idle_usec) m4_dnl -Swap.What, config_parse_path, 0, offsetof(Swap, parameters_fragment.what) +Swap.What, config_parse_unit_path_printf, 0, offsetof(Swap, parameters_fragment.what) Swap.Priority, config_parse_int, 0, offsetof(Swap, parameters_fragment.priority) Swap.Options, config_parse_unit_string_printf, 0, offsetof(Swap, parameters_fragment.options) Swap.TimeoutSec, config_parse_sec_fix_0, 0, offsetof(Swap, timeout_usec) From 98bad05e75cad988d5b2c2cc928f7cc2605cbc2d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 20:09:31 +0100 Subject: [PATCH 09/33] generators: be more careful when writing unit settings that support specifier expansion Let's always escape strings we receive from the user before writing them out to unit file settings that suppor specifier expansion, so that user strings are transported as-is. --- src/cryptsetup/cryptsetup-generator.c | 32 ++++++++++++++---- src/fstab-generator/fstab-generator.c | 37 +++++++++++++++------ src/gpt-auto-generator/gpt-auto-generator.c | 19 +++++++++-- src/shared/generator.c | 13 +++++--- src/sysv-generator/sysv-generator.c | 34 +++++++++++++++---- src/veritysetup/veritysetup-generator.c | 16 +++++++-- 6 files changed, 118 insertions(+), 33 deletions(-) diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c index d0be412625..e2dc96bdb7 100644 --- a/src/cryptsetup/cryptsetup-generator.c +++ b/src/cryptsetup/cryptsetup-generator.c @@ -32,6 +32,7 @@ #include "parse-util.h" #include "path-util.h" #include "proc-cmdline.h" +#include "specifier.h" #include "string-util.h" #include "strv.h" #include "unit-name.h" @@ -60,7 +61,7 @@ static int create_disk( const char *options) { _cleanup_free_ char *p = NULL, *n = NULL, *d = NULL, *u = NULL, *e = NULL, - *filtered = NULL; + *filtered = NULL, *u_escaped = NULL, *password_escaped = NULL, *filtered_escaped = NULL, *name_escaped = NULL; _cleanup_fclose_ FILE *f = NULL; const char *dmname; bool noauto, nofail, tmp, swap, netdev; @@ -80,6 +81,10 @@ static int create_disk( return -EINVAL; } + name_escaped = specifier_escape(name); + if (!name_escaped) + return log_oom(); + e = unit_name_escape(name); if (!e) return log_oom(); @@ -96,10 +101,18 @@ static int create_disk( if (!u) return log_oom(); + u_escaped = specifier_escape(u); + if (!u_escaped) + return log_oom(); + r = unit_name_from_path(u, ".device", &d); if (r < 0) return log_error_errno(r, "Failed to generate unit name: %m"); + password_escaped = specifier_escape(password); + if (!password_escaped) + return log_oom(); + f = fopen(p, "wxe"); if (!f) return log_error_errno(errno, "Failed to create unit file %s: %m", p); @@ -142,7 +155,7 @@ static int create_disk( fprintf(f, "After=%1$s\nRequires=%1$s\n", dd); } else - fprintf(f, "RequiresMountsFor=%s\n", password); + fprintf(f, "RequiresMountsFor=%s\n", password_escaped); } } } @@ -160,12 +173,17 @@ static int create_disk( } else fprintf(f, "RequiresMountsFor=%s\n", - u); + u_escaped); + r = generator_write_timeouts(arg_dest, device, name, options, &filtered); if (r < 0) return r; + filtered_escaped = specifier_escape(filtered); + if (!filtered_escaped) + return log_oom(); + fprintf(f, "\n[Service]\n" "Type=oneshot\n" @@ -174,18 +192,18 @@ static int create_disk( "KeyringMode=shared\n" /* make sure we can share cached keys among instances */ "ExecStart=" SYSTEMD_CRYPTSETUP_PATH " attach '%s' '%s' '%s' '%s'\n" "ExecStop=" SYSTEMD_CRYPTSETUP_PATH " detach '%s'\n", - name, u, strempty(password), strempty(filtered), - name); + name_escaped, u_escaped, strempty(password_escaped), strempty(filtered_escaped), + name_escaped); if (tmp) fprintf(f, "ExecStartPost=/sbin/mke2fs '/dev/mapper/%s'\n", - name); + name_escaped); if (swap) fprintf(f, "ExecStartPost=/sbin/mkswap '/dev/mapper/%s'\n", - name); + name_escaped); r = fflush_and_check(f); if (r < 0) diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index 243c1160b3..6d6895a216 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -26,8 +26,8 @@ #include "alloc-util.h" #include "fd-util.h" -#include "fs-util.h" #include "fileio.h" +#include "fs-util.h" #include "fstab-util.h" #include "generator.h" #include "log.h" @@ -38,6 +38,7 @@ #include "path-util.h" #include "proc-cmdline.h" #include "special.h" +#include "specifier.h" #include "stat-util.h" #include "string-util.h" #include "strv.h" @@ -68,7 +69,7 @@ static int write_options(FILE *f, const char *options) { if (streq(options, "defaults")) return 0; - o = strreplace(options, "%", "%%"); + o = specifier_escape(options); if (!o) return log_oom(); @@ -79,7 +80,7 @@ static int write_options(FILE *f, const char *options) { static int write_what(FILE *f, const char *what) { _cleanup_free_ char *w = NULL; - w = strreplace(what, "%", "%%"); + w = specifier_escape(what); if (!w) return log_oom(); @@ -262,7 +263,7 @@ static int write_before(FILE *f, const char *opts) { } static int write_requires_mounts_for(FILE *f, const char *opts) { - _cleanup_strv_free_ char **paths = NULL; + _cleanup_strv_free_ char **paths = NULL, **paths_escaped = NULL; _cleanup_free_ char *res = NULL; int r; @@ -275,7 +276,11 @@ static int write_requires_mounts_for(FILE *f, const char *opts) { if (r == 0) return 0; - res = strv_join(paths, " "); + r = specifier_escape_strv(paths, &paths_escaped); + if (r < 0) + return log_error_errno(r, "Failed to escape paths: %m"); + + res = strv_join(paths_escaped, " "); if (!res) return log_oom(); @@ -301,7 +306,8 @@ static int add_mount( _cleanup_free_ char *name = NULL, *unit = NULL, *automount_name = NULL, *automount_unit = NULL, - *filtered = NULL; + *filtered = NULL, + *where_escaped = NULL; _cleanup_fclose_ FILE *f = NULL; int r; @@ -398,14 +404,25 @@ static int add_mount( fprintf(f, "\n[Mount]\n"); if (original_where) fprintf(f, "# Canonicalized from %s\n", original_where); - fprintf(f, "Where=%s\n", where); + + where_escaped = specifier_escape(where); + if (!where_escaped) + return log_oom(); + fprintf(f, "Where=%s\n", where_escaped); r = write_what(f, what); if (r < 0) return r; - if (!isempty(fstype) && !streq(fstype, "auto")) - fprintf(f, "Type=%s\n", fstype); + if (!isempty(fstype) && !streq(fstype, "auto")) { + _cleanup_free_ char *t; + + t = specifier_escape(fstype); + if (!t) + return -ENOMEM; + + fprintf(f, "Type=%s\n", t); + } r = generator_write_timeouts(dest, what, where, opts, &filtered); if (r < 0) @@ -476,7 +493,7 @@ static int add_mount( "\n" "[Automount]\n" "Where=%s\n", - where); + where_escaped); r = write_idle_timeout(f, where, opts); if (r < 0) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index 7d014db69d..9e8b956d5c 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -44,6 +44,7 @@ #include "path-util.h" #include "proc-cmdline.h" #include "special.h" +#include "specifier.h" #include "stat-util.h" #include "string-util.h" #include "udev-util.h" @@ -57,7 +58,7 @@ static bool arg_root_enabled = true; static bool arg_root_rw = false; static int add_cryptsetup(const char *id, const char *what, bool rw, bool require, char **device) { - _cleanup_free_ char *e = NULL, *n = NULL, *p = NULL, *d = NULL; + _cleanup_free_ char *e = NULL, *n = NULL, *p = NULL, *d = NULL, *id_escaped = NULL, *what_escaped = NULL; _cleanup_fclose_ FILE *f = NULL; char *ret; int r; @@ -77,6 +78,14 @@ static int add_cryptsetup(const char *id, const char *what, bool rw, bool requir if (r < 0) return log_error_errno(r, "Failed to generate unit name: %m"); + id_escaped = specifier_escape(id); + if (!id_escaped) + return log_oom(); + + what_escaped = specifier_escape(what); + if (!what_escaped) + return log_oom(); + p = strjoin(arg_dest, "/", n); if (!p) return log_oom(); @@ -104,8 +113,8 @@ static int add_cryptsetup(const char *id, const char *what, bool rw, bool requir "ExecStart=" SYSTEMD_CRYPTSETUP_PATH " attach '%s' '%s' '' '%s'\n" "ExecStop=" SYSTEMD_CRYPTSETUP_PATH " detach '%s'\n", d, d, - id, what, rw ? "" : "read-only", - id); + id_escaped, what_escaped, rw ? "" : "read-only", + id_escaped); r = fflush_and_check(f); if (r < 0) @@ -165,6 +174,10 @@ static int add_mount( _cleanup_fclose_ FILE *f = NULL; int r; + /* Note that we don't apply specifier escaping on the input strings here, since we know they are not configured + * externally, but all originate from our own sources here, and hence we know they contain no % characters that + * could potentially be understood as specifiers. */ + assert(id); assert(what); assert(where); diff --git a/src/shared/generator.c b/src/shared/generator.c index a9670213d4..462457e2fe 100644 --- a/src/shared/generator.c +++ b/src/shared/generator.c @@ -33,6 +33,7 @@ #include "mkdir.h" #include "path-util.h" #include "special.h" +#include "specifier.h" #include "string-util.h" #include "time-util.h" #include "unit-name.h" @@ -55,15 +56,19 @@ int generator_add_symlink(const char *root, const char *dst, const char *dep_typ } static int write_fsck_sysroot_service(const char *dir, const char *what) { - _cleanup_free_ char *device = NULL, *escaped = NULL; + _cleanup_free_ char *device = NULL, *escaped = NULL, *escaped2 = NULL; _cleanup_fclose_ FILE *f = NULL; const char *unit; int r; - escaped = cescape(what); + escaped = specifier_escape(what); if (!escaped) return log_oom(); + escaped2 = cescape(escaped); + if (!escaped2) + return log_oom(); + unit = strjoina(dir, "/systemd-fsck-root.service"); log_debug("Creating %s", unit); @@ -91,9 +96,9 @@ static int write_fsck_sysroot_service(const char *dir, const char *what) { "ExecStart=" SYSTEMD_FSCK_PATH " %4$s\n" "TimeoutSec=0\n", program_invocation_short_name, - what, + escaped, device, - escaped); + escaped2); r = fflush_and_check(f); if (r < 0) diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index f59c277e13..51306b5625 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -39,6 +39,7 @@ #include "path-util.h" #include "set.h" #include "special.h" +#include "specifier.h" #include "stat-util.h" #include "string-util.h" #include "strv.h" @@ -119,6 +120,7 @@ static int add_alias(const char *service, const char *alias) { } static int generate_unit_file(SysvStub *s) { + _cleanup_free_ char *path_escaped = NULL; _cleanup_fclose_ FILE *f = NULL; const char *unit; char **p; @@ -129,6 +131,10 @@ static int generate_unit_file(SysvStub *s) { if (!s->loaded) return 0; + path_escaped = specifier_escape(s->path); + if (!path_escaped) + return log_oom(); + unit = strjoina(arg_dest, "/", s->name); /* We might already have a symlink with the same name from a Provides:, @@ -148,10 +154,17 @@ static int generate_unit_file(SysvStub *s) { "[Unit]\n" "Documentation=man:systemd-sysv-generator(8)\n" "SourcePath=%s\n", - s->path); + path_escaped); - if (s->description) - fprintf(f, "Description=%s\n", s->description); + if (s->description) { + _cleanup_free_ char *t; + + t = specifier_escape(s->description); + if (!t) + return log_oom(); + + fprintf(f, "Description=%s\n", t); + } STRV_FOREACH(p, s->before) fprintf(f, "Before=%s\n", *p); @@ -171,8 +184,15 @@ static int generate_unit_file(SysvStub *s) { "RemainAfterExit=%s\n", yes_no(!s->pid_file)); - if (s->pid_file) - fprintf(f, "PIDFile=%s\n", s->pid_file); + if (s->pid_file) { + _cleanup_free_ char *t; + + t = specifier_escape(s->pid_file); + if (!t) + return log_oom(); + + fprintf(f, "PIDFile=%s\n", t); + } /* Consider two special LSB exit codes a clean exit */ if (s->has_lsb) @@ -184,10 +204,10 @@ static int generate_unit_file(SysvStub *s) { fprintf(f, "ExecStart=%s start\n" "ExecStop=%s stop\n", - s->path, s->path); + path_escaped, path_escaped); if (s->reload) - fprintf(f, "ExecReload=%s reload\n", s->path); + fprintf(f, "ExecReload=%s reload\n", path_escaped); r = fflush_and_check(f); if (r < 0) diff --git a/src/veritysetup/veritysetup-generator.c b/src/veritysetup/veritysetup-generator.c index ef43aed42f..5919b1380e 100644 --- a/src/veritysetup/veritysetup-generator.c +++ b/src/veritysetup/veritysetup-generator.c @@ -32,6 +32,7 @@ #include "mkdir.h" #include "parse-util.h" #include "proc-cmdline.h" +#include "specifier.h" #include "string-util.h" #include "unit-name.h" @@ -42,7 +43,7 @@ static char *arg_data_what = NULL; static char *arg_hash_what = NULL; static int create_device(void) { - _cleanup_free_ char *u = NULL, *v = NULL, *d = NULL, *e = NULL; + _cleanup_free_ char *u = NULL, *v = NULL, *d = NULL, *e = NULL, *u_escaped = NULL, *v_escaped = NULL, *root_hash_escaped = NULL; _cleanup_fclose_ FILE *f = NULL; const char *p, *to; int r; @@ -75,6 +76,13 @@ static int create_device(void) { if (!v) return log_oom(); + u_escaped = specifier_escape(u); + if (!u_escaped) + return log_oom(); + v_escaped = specifier_escape(v); + if (!v_escaped) + return log_oom(); + r = unit_name_from_path(u, ".device", &d); if (r < 0) return log_error_errno(r, "Failed to generate unit name: %m"); @@ -82,6 +90,10 @@ static int create_device(void) { if (r < 0) return log_error_errno(r, "Failed to generate unit name: %m"); + root_hash_escaped = specifier_escape(arg_root_hash); + if (!root_hash_escaped) + return log_oom(); + f = fopen(p, "wxe"); if (!f) return log_error_errno(errno, "Failed to create unit file %s: %m", p); @@ -105,7 +117,7 @@ static int create_device(void) { "ExecStop=" ROOTLIBEXECDIR "/systemd-veritysetup detach root\n", d, e, d, e, - u, v, arg_root_hash); + u_escaped, v_escaped, root_hash_escaped); r = fflush_and_check(f); if (r < 0) From ddd59d0c7ffc77158ab2d6f52f08812670bb9160 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 22 Nov 2017 12:10:39 +0100 Subject: [PATCH 10/33] logind: fix sysfs change trigger code We can't create files in sysfs, hence don't bother. Also if we ignore the return value, do so explicitly by casting to void. --- src/login/logind-dbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 196d68bbfb..ae36ececb5 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -1282,7 +1282,7 @@ static int trigger_device(Manager *m, struct udev_device *d) { if (!t) return -ENOMEM; - write_string_file(t, "change", WRITE_STRING_FILE_CREATE); + (void) write_string_file(t, "change", 0); } return 0; From 2e59b241ca86b8338c706a7a704d301fde908581 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 22 Nov 2017 15:03:51 +0100 Subject: [PATCH 11/33] core: add proper escaping to writing of drop-ins/transient unit files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This majorly refactors the transient unit file and drop-in writing logic, so that we properly C-escape and specifier-escape (% → %%) everything we write out, so that when we read it back again, specifiers are parsed that aren't supposed to be parsed. This renames unit_write_drop_in() and friends by unit_write_setting(). The name change is supposed to clarify that the functions are not only used to write drop-in files, but also transient unit files. The previous "mode" parameter to this function is replaced by a more generic "flags", which knows additional flags for implicit C-style and specifier escaping before writing things out. This can cover most properties where either form of escaping is defined. For the cases where this isn't sufficient, we add helpers unit_escape_setting() and unit_concat_strv() for escaping individual strings or strvs properly. While we are at it, we also prettify generation of transient unit files: we try to reduce the number of section headers written out: previously we'd write the right section header our for each setting. With this change we do so only if the setting lives in a different section than the one before. (This should also be considered preparation for when we add proper APIs to systemd to write normal, persistant unit files through the bus API) --- src/core/dbus-automount.c | 22 +-- src/core/dbus-automount.h | 2 +- src/core/dbus-cgroup.c | 205 +++++++++++----------- src/core/dbus-cgroup.h | 2 +- src/core/dbus-execute.c | 359 ++++++++++++++++++++------------------ src/core/dbus-execute.h | 2 +- src/core/dbus-kill.c | 20 ++- src/core/dbus-kill.h | 2 +- src/core/dbus-mount.c | 28 ++- src/core/dbus-mount.h | 2 +- src/core/dbus-scope.c | 47 +++-- src/core/dbus-scope.h | 2 +- src/core/dbus-service.c | 58 +++--- src/core/dbus-service.h | 2 +- src/core/dbus-slice.c | 4 +- src/core/dbus-slice.h | 2 +- src/core/dbus-socket.c | 4 +- src/core/dbus-socket.h | 2 +- src/core/dbus-swap.c | 4 +- src/core/dbus-swap.h | 2 +- src/core/dbus-timer.c | 38 ++-- src/core/dbus-timer.h | 2 +- src/core/dbus-unit.c | 40 +++-- src/core/dbus-unit.h | 2 +- src/core/unit.c | 188 ++++++++++++++------ src/core/unit.h | 39 +++-- 26 files changed, 602 insertions(+), 478 deletions(-) diff --git a/src/core/dbus-automount.c b/src/core/dbus-automount.c index 3c165c41a5..4b1b86f7b9 100644 --- a/src/core/dbus-automount.c +++ b/src/core/dbus-automount.c @@ -38,7 +38,7 @@ static int bus_automount_set_transient_property( Automount *a, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { int r; @@ -47,18 +47,21 @@ static int bus_automount_set_transient_property( assert(name); assert(message); + flags |= UNIT_PRIVATE; + if (streq(name, "TimeoutIdleUSec")) { usec_t timeout_idle_usec; + r = sd_bus_message_read(message, "t", &timeout_idle_usec); if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { char time[FORMAT_TIMESPAN_MAX]; a->timeout_idle_usec = timeout_idle_usec; - unit_write_drop_in_format(UNIT(a), mode, name, "[Automount]\nTimeoutIdleSec=%s\n", - format_timespan(time, sizeof(time), timeout_idle_usec, USEC_PER_MSEC)); + unit_write_settingf(UNIT(a), flags, name, "TimeoutIdleSec=%s\n", + format_timespan(time, sizeof(time), timeout_idle_usec, USEC_PER_MSEC)); } } else return 0; @@ -70,20 +73,17 @@ int bus_automount_set_property( Unit *u, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { Automount *a = AUTOMOUNT(u); - int r = 0; assert(a); assert(name); assert(message); - if (u->transient && u->load_state == UNIT_STUB) - /* This is a transient unit, let's load a little more */ + if (u->transient && u->load_state == UNIT_STUB) /* This is a transient unit? let's load a little more */ + return bus_automount_set_transient_property(a, name, message, flags, error); - r = bus_automount_set_transient_property(a, name, message, mode, error); - - return r; + return 0; } diff --git a/src/core/dbus-automount.h b/src/core/dbus-automount.h index dc56de954a..378119410c 100644 --- a/src/core/dbus-automount.h +++ b/src/core/dbus-automount.h @@ -23,4 +23,4 @@ extern const sd_bus_vtable bus_automount_vtable[]; -int bus_automount_set_property(Unit *u, const char *name, sd_bus_message *message, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_automount_set_property(Unit *u, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index f09d9fecab..0343aaed62 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -334,7 +334,7 @@ static int bus_cgroup_set_transient_property( CGroupContext *c, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { int r; @@ -344,6 +344,8 @@ static int bus_cgroup_set_transient_property( assert(name); assert(message); + flags |= UNIT_PRIVATE; + if (streq(name, "Delegate")) { int b; @@ -351,11 +353,11 @@ static int bus_cgroup_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->delegate = b; c->delegate_controllers = b ? _CGROUP_MASK_ALL : 0; - unit_write_drop_in_private(u, mode, name, b ? "Delegate=yes" : "Delegate=no"); + unit_write_settingf(u, flags, name, "Delegate=%s", yes_no(b)); } return 1; @@ -388,7 +390,7 @@ static int bus_cgroup_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *t = NULL; r = cg_mask_to_string(mask, &t); @@ -401,7 +403,7 @@ static int bus_cgroup_set_transient_property( else c->delegate_controllers |= mask; - unit_write_drop_in_private_format(u, mode, name, "Delegate=%s", strempty(t)); + unit_write_settingf(u, flags, name, "Delegate=%s", strempty(t)); } return 1; @@ -415,7 +417,7 @@ int bus_cgroup_set_property( CGroupContext *c, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { CGroupIOLimitType iol_type; @@ -426,6 +428,8 @@ int bus_cgroup_set_property( assert(name); assert(message); + flags |= UNIT_PRIVATE; + if (streq(name, "CPUAccounting")) { int b; @@ -433,10 +437,10 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->cpu_accounting = b; unit_invalidate_cgroup(u, CGROUP_MASK_CPUACCT|CGROUP_MASK_CPU); - unit_write_drop_in_private(u, mode, name, b ? "CPUAccounting=yes" : "CPUAccounting=no"); + unit_write_settingf(u, flags, name, "CPUAccounting=%s", yes_no(b)); } return 1; @@ -449,16 +453,16 @@ int bus_cgroup_set_property( return r; if (!CGROUP_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "CPUWeight value out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "CPUWeight= value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->cpu_weight = weight; unit_invalidate_cgroup(u, CGROUP_MASK_CPU); if (weight == CGROUP_WEIGHT_INVALID) - unit_write_drop_in_private(u, mode, name, "CPUWeight="); + unit_write_setting(u, flags, name, "CPUWeight="); else - unit_write_drop_in_private_format(u, mode, name, "CPUWeight=%" PRIu64, weight); + unit_write_settingf(u, flags, name, "CPUWeight=%" PRIu64, weight); } return 1; @@ -471,16 +475,16 @@ int bus_cgroup_set_property( return r; if (!CGROUP_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "StartupCPUWeight value out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "StartupCPUWeight= value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->startup_cpu_weight = weight; unit_invalidate_cgroup(u, CGROUP_MASK_CPU); if (weight == CGROUP_CPU_SHARES_INVALID) - unit_write_drop_in_private(u, mode, name, "StartupCPUWeight="); + unit_write_setting(u, flags, name, "StartupCPUWeight="); else - unit_write_drop_in_private_format(u, mode, name, "StartupCPUWeight=%" PRIu64, weight); + unit_write_settingf(u, flags, name, "StartupCPUWeight=%" PRIu64, weight); } return 1; @@ -493,16 +497,16 @@ int bus_cgroup_set_property( return r; if (!CGROUP_CPU_SHARES_IS_OK(shares)) - return sd_bus_error_set_errnof(error, EINVAL, "CPUShares value out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "CPUShares= value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->cpu_shares = shares; unit_invalidate_cgroup(u, CGROUP_MASK_CPU); if (shares == CGROUP_CPU_SHARES_INVALID) - unit_write_drop_in_private(u, mode, name, "CPUShares="); + unit_write_setting(u, flags, name, "CPUShares="); else - unit_write_drop_in_private_format(u, mode, name, "CPUShares=%" PRIu64, shares); + unit_write_settingf(u, flags, name, "CPUShares=%" PRIu64, shares); } return 1; @@ -515,16 +519,16 @@ int bus_cgroup_set_property( return r; if (!CGROUP_CPU_SHARES_IS_OK(shares)) - return sd_bus_error_set_errnof(error, EINVAL, "StartupCPUShares value out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "StartupCPUShares= value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->startup_cpu_shares = shares; unit_invalidate_cgroup(u, CGROUP_MASK_CPU); if (shares == CGROUP_CPU_SHARES_INVALID) - unit_write_drop_in_private(u, mode, name, "StartupCPUShares="); + unit_write_setting(u, flags, name, "StartupCPUShares="); else - unit_write_drop_in_private_format(u, mode, name, "StartupCPUShares=%" PRIu64, shares); + unit_write_settingf(u, flags, name, "StartupCPUShares=%" PRIu64, shares); } return 1; @@ -537,20 +541,20 @@ int bus_cgroup_set_property( return r; if (u64 <= 0) - return sd_bus_error_set_errnof(error, EINVAL, "CPUQuotaPerSecUSec value out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "CPUQuotaPerSecUSec= value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->cpu_quota_per_sec_usec = u64; unit_invalidate_cgroup(u, CGROUP_MASK_CPU); + if (c->cpu_quota_per_sec_usec == USEC_INFINITY) - unit_write_drop_in_private_format(u, mode, "CPUQuota", - "CPUQuota="); + unit_write_setting(u, flags, "CPUQuota", "CPUQuota="); else - /* config_parse_cpu_quota() requires an integer, so - * truncating division is used on purpose here. */ - unit_write_drop_in_private_format(u, mode, "CPUQuota", - "CPUQuota=%0.f%%", - (double) (c->cpu_quota_per_sec_usec / 10000)); + /* config_parse_cpu_quota() requires an integer, so truncating division is used on + * purpose here. */ + unit_write_settingf(u, flags, "CPUQuota", + "CPUQuota=%0.f%%", + (double) (c->cpu_quota_per_sec_usec / 10000)); } return 1; @@ -562,10 +566,10 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->io_accounting = b; unit_invalidate_cgroup(u, CGROUP_MASK_IO); - unit_write_drop_in_private(u, mode, name, b ? "IOAccounting=yes" : "IOAccounting=no"); + unit_write_settingf(u, flags, name, "IOAccounting=%s", yes_no(b)); } return 1; @@ -578,16 +582,16 @@ int bus_cgroup_set_property( return r; if (!CGROUP_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "IOWeight value out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "IOWeight= value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->io_weight = weight; unit_invalidate_cgroup(u, CGROUP_MASK_IO); if (weight == CGROUP_WEIGHT_INVALID) - unit_write_drop_in_private(u, mode, name, "IOWeight="); + unit_write_setting(u, flags, name, "IOWeight="); else - unit_write_drop_in_private_format(u, mode, name, "IOWeight=%" PRIu64, weight); + unit_write_settingf(u, flags, name, "IOWeight=%" PRIu64, weight); } return 1; @@ -600,16 +604,16 @@ int bus_cgroup_set_property( return r; if (CGROUP_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "StartupIOWeight value out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "StartupIOWeight= value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->startup_io_weight = weight; unit_invalidate_cgroup(u, CGROUP_MASK_IO); if (weight == CGROUP_WEIGHT_INVALID) - unit_write_drop_in_private(u, mode, name, "StartupIOWeight="); + unit_write_setting(u, flags, name, "StartupIOWeight="); else - unit_write_drop_in_private_format(u, mode, name, "StartupIOWeight=%" PRIu64, weight); + unit_write_settingf(u, flags, name, "StartupIOWeight=%" PRIu64, weight); } return 1; @@ -625,7 +629,7 @@ int bus_cgroup_set_property( while ((r = sd_bus_message_read(message, "(st)", &path, &u64)) > 0) { - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { CGroupIODeviceLimit *a = NULL, *b; LIST_FOREACH(device_limits, b, c->io_device_limits) { @@ -666,7 +670,7 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { CGroupIODeviceLimit *a; _cleanup_free_ char *buf = NULL; _cleanup_fclose_ FILE *f = NULL; @@ -691,7 +695,7 @@ int bus_cgroup_set_property( r = fflush_and_check(f); if (r < 0) return r; - unit_write_drop_in_private(u, mode, name, buf); + unit_write_setting(u, flags, name, buf); } return 1; @@ -708,9 +712,9 @@ int bus_cgroup_set_property( while ((r = sd_bus_message_read(message, "(st)", &path, &weight)) > 0) { if (!CGROUP_WEIGHT_IS_OK(weight) || weight == CGROUP_WEIGHT_INVALID) - return sd_bus_error_set_errnof(error, EINVAL, "IODeviceWeight out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "IODeviceWeight= value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { CGroupIODeviceWeight *a = NULL, *b; LIST_FOREACH(device_weights, b, c->io_device_weights) { @@ -743,7 +747,7 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *buf = NULL; _cleanup_fclose_ FILE *f = NULL; CGroupIODeviceWeight *a; @@ -767,7 +771,7 @@ int bus_cgroup_set_property( r = fflush_and_check(f); if (r < 0) return r; - unit_write_drop_in_private(u, mode, name, buf); + unit_write_setting(u, flags, name, buf); } return 1; @@ -779,10 +783,10 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->blockio_accounting = b; unit_invalidate_cgroup(u, CGROUP_MASK_BLKIO); - unit_write_drop_in_private(u, mode, name, b ? "BlockIOAccounting=yes" : "BlockIOAccounting=no"); + unit_write_settingf(u, flags, name, "BlockIOAccounting=%s", yes_no(b)); } return 1; @@ -795,16 +799,16 @@ int bus_cgroup_set_property( return r; if (!CGROUP_BLKIO_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "BlockIOWeight value out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "BlockIOWeight= value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->blockio_weight = weight; unit_invalidate_cgroup(u, CGROUP_MASK_BLKIO); if (weight == CGROUP_BLKIO_WEIGHT_INVALID) - unit_write_drop_in_private(u, mode, name, "BlockIOWeight="); + unit_write_setting(u, flags, name, "BlockIOWeight="); else - unit_write_drop_in_private_format(u, mode, name, "BlockIOWeight=%" PRIu64, weight); + unit_write_settingf(u, flags, name, "BlockIOWeight=%" PRIu64, weight); } return 1; @@ -817,16 +821,16 @@ int bus_cgroup_set_property( return r; if (!CGROUP_BLKIO_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "StartupBlockIOWeight value out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "StartupBlockIOWeight= value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->startup_blockio_weight = weight; unit_invalidate_cgroup(u, CGROUP_MASK_BLKIO); if (weight == CGROUP_BLKIO_WEIGHT_INVALID) - unit_write_drop_in_private(u, mode, name, "StartupBlockIOWeight="); + unit_write_setting(u, flags, name, "StartupBlockIOWeight="); else - unit_write_drop_in_private_format(u, mode, name, "StartupBlockIOWeight=%" PRIu64, weight); + unit_write_settingf(u, flags, name, "StartupBlockIOWeight=%" PRIu64, weight); } return 1; @@ -846,7 +850,7 @@ int bus_cgroup_set_property( while ((r = sd_bus_message_read(message, "(st)", &path, &u64)) > 0) { - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { CGroupBlockIODeviceBandwidth *a = NULL, *b; LIST_FOREACH(device_bandwidths, b, c->blockio_device_bandwidths) { @@ -887,7 +891,7 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { CGroupBlockIODeviceBandwidth *a; _cleanup_free_ char *buf = NULL; _cleanup_fclose_ FILE *f = NULL; @@ -923,7 +927,8 @@ int bus_cgroup_set_property( r = fflush_and_check(f); if (r < 0) return r; - unit_write_drop_in_private(u, mode, name, buf); + + unit_write_setting(u, flags, name, buf); } return 1; @@ -940,9 +945,9 @@ int bus_cgroup_set_property( while ((r = sd_bus_message_read(message, "(st)", &path, &weight)) > 0) { if (!CGROUP_BLKIO_WEIGHT_IS_OK(weight) || weight == CGROUP_BLKIO_WEIGHT_INVALID) - return sd_bus_error_set_errnof(error, EINVAL, "BlockIODeviceWeight out of range"); + return sd_bus_error_set_errnof(error, EINVAL, "BlockIODeviceWeight= out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { CGroupBlockIODeviceWeight *a = NULL, *b; LIST_FOREACH(device_weights, b, c->blockio_device_weights) { @@ -975,7 +980,7 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *buf = NULL; _cleanup_fclose_ FILE *f = NULL; CGroupBlockIODeviceWeight *a; @@ -999,7 +1004,8 @@ int bus_cgroup_set_property( r = fflush_and_check(f); if (r < 0) return r; - unit_write_drop_in_private(u, mode, name, buf); + + unit_write_setting(u, flags, name, buf); } return 1; @@ -1011,10 +1017,10 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->memory_accounting = b; unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); - unit_write_drop_in_private(u, mode, name, b ? "MemoryAccounting=yes" : "MemoryAccounting=no"); + unit_write_settingf(u, flags, name, "MemoryAccounting=%s", yes_no(b)); } return 1; @@ -1028,7 +1034,7 @@ int bus_cgroup_set_property( if (v <= 0) return sd_bus_error_set_errnof(error, EINVAL, "%s= is too small", name); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (streq(name, "MemoryLow")) c->memory_low = v; else if (streq(name, "MemoryHigh")) @@ -1041,9 +1047,9 @@ int bus_cgroup_set_property( unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); if (v == CGROUP_LIMIT_MAX) - unit_write_drop_in_private_format(u, mode, name, "%s=infinity", name); + unit_write_settingf(u, flags, name, "%s=infinity", name); else - unit_write_drop_in_private_format(u, mode, name, "%s=%" PRIu64, name, v); + unit_write_settingf(u, flags, name, "%s=%" PRIu64, name, v); } return 1; @@ -1060,7 +1066,7 @@ int bus_cgroup_set_property( if (v <= 0 || v == UINT64_MAX) return sd_bus_error_set_errnof(error, EINVAL, "%s= is out of range", name); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { const char *e; /* Chop off suffix */ @@ -1075,8 +1081,8 @@ int bus_cgroup_set_property( c->memory_max = v; unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); - unit_write_drop_in_private_format(u, mode, name, "%s=%" PRIu32 "%%", name, - (uint32_t) (DIV_ROUND_UP((uint64_t) raw * 100U, (uint64_t) UINT32_MAX))); + unit_write_settingf(u, flags, name, "%s=%" PRIu32 "%%", name, + (uint32_t) (DIV_ROUND_UP((uint64_t) raw * 100U, (uint64_t) UINT32_MAX))); } return 1; @@ -1090,14 +1096,14 @@ int bus_cgroup_set_property( if (limit <= 0) return sd_bus_error_set_errnof(error, EINVAL, "%s= is too small", name); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->memory_limit = limit; unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); if (limit == (uint64_t) -1) - unit_write_drop_in_private(u, mode, name, "MemoryLimit=infinity"); + unit_write_setting(u, flags, name, "MemoryLimit=infinity"); else - unit_write_drop_in_private_format(u, mode, name, "MemoryLimit=%" PRIu64, limit); + unit_write_settingf(u, flags, name, "MemoryLimit=%" PRIu64, limit); } return 1; @@ -1114,10 +1120,10 @@ int bus_cgroup_set_property( if (limit <= 0 || limit == UINT64_MAX) return sd_bus_error_set_errnof(error, EINVAL, "%s= is out of range", name); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->memory_limit = limit; unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); - unit_write_drop_in_private_format(u, mode, "MemoryLimit", "MemoryLimit=%" PRIu32 "%%", + unit_write_settingf(u, flags, "MemoryLimit", "MemoryLimit=%" PRIu32 "%%", (uint32_t) (DIV_ROUND_UP((uint64_t) raw * 100U, (uint64_t) UINT32_MAX))); } @@ -1135,10 +1141,10 @@ int bus_cgroup_set_property( if (p < 0) return -EINVAL; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->device_policy = p; unit_invalidate_cgroup(u, CGROUP_MASK_DEVICES); - unit_write_drop_in_private_format(u, mode, name, "DevicePolicy=%s", policy); + unit_write_settingf(u, flags, name, "DevicePolicy=%s", policy); } return 1; @@ -1166,7 +1172,7 @@ int bus_cgroup_set_property( if (!in_charset(rwm, "rwm")) return sd_bus_error_set_errnof(error, EINVAL, "DeviceAllow= requires combination of rwm flags"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { CGroupDeviceAllow *a = NULL, *b; LIST_FOREACH(device_allow, b, c->device_allow) { @@ -1204,7 +1210,7 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *buf = NULL; _cleanup_fclose_ FILE *f = NULL; CGroupDeviceAllow *a; @@ -1228,7 +1234,7 @@ int bus_cgroup_set_property( r = fflush_and_check(f); if (r < 0) return r; - unit_write_drop_in_private(u, mode, name, buf); + unit_write_setting(u, flags, name, buf); } return 1; @@ -1240,10 +1246,10 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->tasks_accounting = b; unit_invalidate_cgroup(u, CGROUP_MASK_PIDS); - unit_write_drop_in_private(u, mode, name, b ? "TasksAccounting=yes" : "TasksAccounting=no"); + unit_write_settingf(u, flags, name, "TasksAccounting=%s", yes_no(b)); } return 1; @@ -1257,14 +1263,14 @@ int bus_cgroup_set_property( if (limit <= 0) return sd_bus_error_set_errnof(error, EINVAL, "%s= is too small", name); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->tasks_max = limit; unit_invalidate_cgroup(u, CGROUP_MASK_PIDS); if (limit == (uint64_t) -1) - unit_write_drop_in_private(u, mode, name, "TasksMax=infinity"); + unit_write_setting(u, flags, name, "TasksMax=infinity"); else - unit_write_drop_in_private_format(u, mode, name, "TasksMax=%" PRIu64, limit); + unit_write_settingf(u, flags, name, "TasksMax=%" PRIu64, limit); } return 1; @@ -1281,11 +1287,11 @@ int bus_cgroup_set_property( if (limit <= 0 || limit >= UINT64_MAX) return sd_bus_error_set_errnof(error, EINVAL, "%s= is out of range", name); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->tasks_max = limit; unit_invalidate_cgroup(u, CGROUP_MASK_PIDS); - unit_write_drop_in_private_format(u, mode, name, "TasksMax=%" PRIu32 "%%", - (uint32_t) (DIV_ROUND_UP((uint64_t) raw * 100U, (uint64_t) UINT32_MAX))); + unit_write_settingf(u, flags, name, "TasksMax=%" PRIu32 "%%", + (uint32_t) (DIV_ROUND_UP((uint64_t) raw * 100U, (uint64_t) UINT32_MAX))); } return 1; @@ -1297,11 +1303,11 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->ip_accounting = b; unit_invalidate_cgroup_bpf(u); - unit_write_drop_in_private(u, mode, name, b ? "IPAccounting=yes" : "IPAccounting=no"); + unit_write_settingf(u, flags, name, "IPAccounting=%s", yes_no(b)); } return 1; @@ -1350,7 +1356,7 @@ int bus_cgroup_set_property( if (prefixlen > FAMILY_ADDRESS_SIZE(family)*8) return sd_bus_error_set_errnof(error, EINVAL, "Prefix length %" PRIu32 " too large for address family %s.", prefixlen, af_to_name(family)); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { IPAddressAccessItem *item; item = new0(IPAddressAccessItem, 1); @@ -1377,7 +1383,7 @@ int bus_cgroup_set_property( *list = ip_address_access_reduce(*list); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *buf = NULL; _cleanup_fclose_ FILE *f = NULL; IPAddressAccessItem *item; @@ -1407,7 +1413,8 @@ int bus_cgroup_set_property( r = fflush_and_check(f); if (r < 0) return r; - unit_write_drop_in_private(u, mode, name, buf); + + unit_write_setting(u, flags, name, buf); if (*list) { r = bpf_firewall_supported(); @@ -1429,7 +1436,7 @@ int bus_cgroup_set_property( } if (u->transient && u->load_state == UNIT_STUB) { - r = bus_cgroup_set_transient_property(u, c, name, message, mode, error); + r = bus_cgroup_set_transient_property(u, c, name, message, flags, error); if (r != 0) return r; diff --git a/src/core/dbus-cgroup.h b/src/core/dbus-cgroup.h index 0e3b903f6f..0588370aa5 100644 --- a/src/core/dbus-cgroup.h +++ b/src/core/dbus-cgroup.h @@ -26,4 +26,4 @@ extern const sd_bus_vtable bus_cgroup_vtable[]; -int bus_cgroup_set_property(Unit *u, CGroupContext *c, const char *name, sd_bus_message *message, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_cgroup_set_property(Unit *u, CGroupContext *c, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 57ccdcbe33..412be2a6c4 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -32,6 +32,7 @@ #include "dbus-execute.h" #include "env-util.h" #include "errno-list.h" +#include "escape.h" #include "execute.h" #include "fd-util.h" #include "fileio.h" @@ -50,6 +51,7 @@ #include "seccomp-util.h" #endif #include "securebits-util.h" +#include "specifier.h" #include "strv.h" #include "syslog-util.h" #include "unit-printf.h" @@ -1037,7 +1039,7 @@ int bus_exec_context_set_transient_property( ExecContext *c, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { const char *soft = NULL; @@ -1048,6 +1050,8 @@ int bus_exec_context_set_transient_property( assert(name); assert(message); + flags |= UNIT_PRIVATE; + if (streq(name, "User")) { const char *uu; @@ -1058,14 +1062,13 @@ int bus_exec_context_set_transient_property( if (!isempty(uu) && !valid_user_group_name_or_id(uu)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid user name: %s", uu); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - if (isempty(uu)) - c->user = mfree(c->user); - else if (free_and_strdup(&c->user, uu) < 0) - return -ENOMEM; + r = free_and_strdup(&c->user, empty_to_null(uu)); + if (r < 0) + return r; - unit_write_drop_in_private_format(u, mode, name, "User=%s", uu); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "User=%s", uu); } return 1; @@ -1080,14 +1083,13 @@ int bus_exec_context_set_transient_property( if (!isempty(gg) && !valid_user_group_name_or_id(gg)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid group name: %s", gg); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - if (isempty(gg)) - c->group = mfree(c->group); - else if (free_and_strdup(&c->group, gg) < 0) - return -ENOMEM; + r = free_and_strdup(&c->group, empty_to_null(gg)); + if (r < 0) + return r; - unit_write_drop_in_private_format(u, mode, name, "Group=%s", gg); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "Group=%s", gg); } return 1; @@ -1105,10 +1107,10 @@ int bus_exec_context_set_transient_property( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid supplementary group names"); } - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (strv_length(l) == 0) { c->supplementary_groups = strv_free(c->supplementary_groups); - unit_write_drop_in_private_format(u, mode, name, "%s=", name); + unit_write_settingf(u, flags, name, "%s=", name); } else { _cleanup_free_ char *joined = NULL; @@ -1120,7 +1122,7 @@ int bus_exec_context_set_transient_property( if (!joined) return -ENOMEM; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, joined); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "%s=%s", name, joined); } } @@ -1133,14 +1135,14 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (isempty(id)) c->syslog_identifier = mfree(c->syslog_identifier); else if (free_and_strdup(&c->syslog_identifier, id) < 0) return -ENOMEM; - unit_write_drop_in_private_format(u, mode, name, "SyslogIdentifier=%s", id); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "SyslogIdentifier=%s", id); } return 1; @@ -1154,9 +1156,9 @@ int bus_exec_context_set_transient_property( if (!log_level_is_valid(level)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Log level value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->syslog_priority = (c->syslog_priority & LOG_FACMASK) | level; - unit_write_drop_in_private_format(u, mode, name, "SyslogLevel=%i", level); + unit_write_settingf(u, flags, name, "SyslogLevel=%i", level); } return 1; @@ -1170,9 +1172,9 @@ int bus_exec_context_set_transient_property( if (!log_facility_unshifted_is_valid(facility)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Log facility value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->syslog_priority = (facility << 3) | LOG_PRI(c->syslog_priority); - unit_write_drop_in_private_format(u, mode, name, "SyslogFacility=%i", facility); + unit_write_settingf(u, flags, name, "SyslogFacility=%i", facility); } return 1; @@ -1187,9 +1189,9 @@ int bus_exec_context_set_transient_property( if (!log_level_is_valid(level)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Maximum log level value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->log_level_max = level; - unit_write_drop_in_private_format(u, mode, name, "LogLevelMax=%i", level); + unit_write_settingf(u, flags, name, "LogLevelMax=%i", level); } return 1; @@ -1230,7 +1232,7 @@ int bus_exec_context_set_transient_property( if (!journal_field_valid(p, eq - (const char*) p, false)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Journal field invalid"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { t = realloc_multiply(c->log_extra_fields, sizeof(struct iovec), c->n_log_extra_fields+1); if (!t) return -ENOMEM; @@ -1247,9 +1249,9 @@ int bus_exec_context_set_transient_property( if (!utf8_is_valid(copy)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Journal field is not valid UTF-8"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->log_extra_fields[c->n_log_extra_fields++] = IOVEC_MAKE(copy, sz); - unit_write_drop_in_private_format(u, mode, name, "LogExtraFields=%s", (char*) copy); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS|UNIT_ESCAPE_C, name, "LogExtraFields=%s", (char*) copy); copy = NULL; } @@ -1261,9 +1263,9 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK && n == 0) { + if (!UNIT_WRITE_FLAGS_NOOP(flags) && n == 0) { exec_context_free_log_extra_fields(c); - unit_write_drop_in_private(u, mode, name, "LogExtraFields="); + unit_write_setting(u, flags, name, "LogExtraFields="); } return 1; @@ -1278,7 +1280,7 @@ int bus_exec_context_set_transient_property( if (!secure_bits_is_valid(n)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid secure bits"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *str = NULL; c->secure_bits = n; @@ -1286,7 +1288,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - unit_write_drop_in_private_format(u, mode, name, "SecureBits=%s", str); + unit_write_settingf(u, flags, name, "SecureBits=%s", str); } return 1; @@ -1297,7 +1299,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *str = NULL; if (streq(name, "CapabilityBoundingSet")) @@ -1309,7 +1311,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, str); + unit_write_settingf(u, flags, name, "%s=%s", name, str); } return 1; @@ -1326,9 +1328,9 @@ int bus_exec_context_set_transient_property( if (p == PERSONALITY_INVALID) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid personality"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->personality = p; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, s); + unit_write_settingf(u, flags, name, "%s=%s", name, s); } return 1; @@ -1355,7 +1357,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *joined = NULL; if (strv_length(l) == 0) { @@ -1416,7 +1418,7 @@ int bus_exec_context_set_transient_property( if (!joined) return -ENOMEM; - unit_write_drop_in_private_format(u, mode, name, "SystemCallFilter=%s%s", whitelist ? "" : "~", joined); + unit_write_settingf(u, flags, name, "SystemCallFilter=%s%s", whitelist ? "" : "~", joined); } return 1; @@ -1428,7 +1430,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *joined = NULL; if (strv_length(l) == 0) @@ -1458,7 +1460,7 @@ int bus_exec_context_set_transient_property( if (!joined) return -ENOMEM; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, joined); + unit_write_settingf(u, flags, name, "%s=%s", name, joined); } return 1; @@ -1473,10 +1475,10 @@ int bus_exec_context_set_transient_property( if (n <= 0 || n > ERRNO_MAX) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid SystemCallErrorNumber"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->syscall_errno = n; - unit_write_drop_in_private_format(u, mode, name, "SystemCallErrorNumber=%d", n); + unit_write_settingf(u, flags, name, "SystemCallErrorNumber=%d", n); } return 1; @@ -1501,7 +1503,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *joined = NULL; if (strv_length(l) == 0) { @@ -1533,7 +1535,7 @@ int bus_exec_context_set_transient_property( if (!joined) return -ENOMEM; - unit_write_drop_in_private_format(u, mode, name, "RestrictAddressFamilies=%s%s", whitelist ? "" : "~", joined); + unit_write_settingf(u, flags, name, "RestrictAddressFamilies=%s%s", whitelist ? "" : "~", joined); } return 1; @@ -1549,7 +1551,7 @@ int bus_exec_context_set_transient_property( if (!sched_policy_is_valid(n)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid CPU scheduling policy"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *str = NULL; c->cpu_sched_policy = n; @@ -1557,7 +1559,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - unit_write_drop_in_private_format(u, mode, name, "CPUSchedulingPolicy=%s", str); + unit_write_settingf(u, flags, name, "CPUSchedulingPolicy=%s", str); } return 1; @@ -1572,9 +1574,9 @@ int bus_exec_context_set_transient_property( if (!ioprio_priority_is_valid(n)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid CPU scheduling priority"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->cpu_sched_priority = n; - unit_write_drop_in_private_format(u, mode, name, "CPUSchedulingPriority=%i", n); + unit_write_settingf(u, flags, name, "CPUSchedulingPriority=%i", n); } return 1; @@ -1587,10 +1589,10 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (n == 0) { c->cpuset = mfree(c->cpuset); - unit_write_drop_in_private_format(u, mode, name, "%s=", name); + unit_write_settingf(u, flags, name, "%s=", name); } else { _cleanup_free_ char *str = NULL; uint8_t *l; @@ -1621,7 +1623,7 @@ int bus_exec_context_set_transient_property( if (len != 0) str[len - 1] = '\0'; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, str); + unit_write_settingf(u, flags, name, "%s=%s", name, str); } } @@ -1636,9 +1638,9 @@ int bus_exec_context_set_transient_property( if (!nice_is_valid(n)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Nice value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->nice = n; - unit_write_drop_in_private_format(u, mode, name, "Nice=%i", n); + unit_write_settingf(u, flags, name, "Nice=%i", n); } return 1; @@ -1653,7 +1655,7 @@ int bus_exec_context_set_transient_property( if (!ioprio_class_is_valid(q)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid IO scheduling class: %i", q); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *s = NULL; r = ioprio_class_to_string_alloc(q, &s); @@ -1663,7 +1665,7 @@ int bus_exec_context_set_transient_property( c->ioprio = IOPRIO_PRIO_VALUE(q, IOPRIO_PRIO_DATA(c->ioprio)); c->ioprio_set = true; - unit_write_drop_in_private_format(u, mode, name, "IOSchedulingClass=%s", s); + unit_write_settingf(u, flags, name, "IOSchedulingClass=%s", s); } return 1; @@ -1678,11 +1680,11 @@ int bus_exec_context_set_transient_property( if (!ioprio_priority_is_valid(p)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid IO scheduling priority: %i", p); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->ioprio = IOPRIO_PRIO_VALUE(IOPRIO_PRIO_CLASS(c->ioprio), p); c->ioprio_set = true; - unit_write_drop_in_private_format(u, mode, name, "IOSchedulingPriority=%i", p); + unit_write_settingf(u, flags, name, "IOSchedulingPriority=%i", p); } return 1; @@ -1697,7 +1699,7 @@ int bus_exec_context_set_transient_property( if (!path_is_absolute(s)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s takes an absolute path", name); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (streq(name, "TTYPath")) r = free_and_strdup(&c->tty_path, s); else if (streq(name, "RootImage")) @@ -1709,7 +1711,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, s); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "%s=%s", name, s); } return 1; @@ -1731,7 +1733,7 @@ int bus_exec_context_set_transient_property( if (!streq(s, "~") && !path_is_absolute(s)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "WorkingDirectory= expects an absolute path or '~'"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (streq(s, "~")) { c->working_directory = mfree(c->working_directory); c->working_directory_home = true; @@ -1744,7 +1746,7 @@ int bus_exec_context_set_transient_property( } c->working_directory_missing_ok = missing_ok; - unit_write_drop_in_private_format(u, mode, name, "WorkingDirectory=%s%s", missing_ok ? "-" : "", s); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "WorkingDirectory=%s%s", missing_ok ? "-" : "", s); } return 1; @@ -1761,10 +1763,10 @@ int bus_exec_context_set_transient_property( if (p < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid standard input name"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->std_input = p; - unit_write_drop_in_private_format(u, mode, name, "StandardInput=%s", exec_input_to_string(p)); + unit_write_settingf(u, flags, name, "StandardInput=%s", exec_input_to_string(p)); } return 1; @@ -1781,10 +1783,10 @@ int bus_exec_context_set_transient_property( if (p < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid standard output name"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->std_output = p; - unit_write_drop_in_private_format(u, mode, name, "StandardOutput=%s", exec_output_to_string(p)); + unit_write_settingf(u, flags, name, "StandardOutput=%s", exec_output_to_string(p)); } return 1; @@ -1801,10 +1803,10 @@ int bus_exec_context_set_transient_property( if (p < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid standard error name"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->std_error = p; - unit_write_drop_in_private_format(u, mode, name, "StandardError=%s", exec_output_to_string(p)); + unit_write_settingf(u, flags, name, "StandardError=%s", exec_output_to_string(p)); } return 1; @@ -1822,7 +1824,7 @@ int bus_exec_context_set_transient_property( else if (!fdname_is_valid(s)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid file descriptor name"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (streq(name, "StandardInputFileDescriptorName")) { r = free_and_strdup(c->stdio_fdname + STDIN_FILENO, s); @@ -1830,7 +1832,7 @@ int bus_exec_context_set_transient_property( return r; c->std_input = EXEC_INPUT_NAMED_FD; - unit_write_drop_in_private_format(u, mode, name, "StandardInput=fd:%s", exec_context_fdname(c, STDIN_FILENO)); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "StandardInput=fd:%s", exec_context_fdname(c, STDIN_FILENO)); } else if (streq(name, "StandardOutputFileDescriptorName")) { r = free_and_strdup(c->stdio_fdname + STDOUT_FILENO, s); @@ -1838,7 +1840,7 @@ int bus_exec_context_set_transient_property( return r; c->std_output = EXEC_OUTPUT_NAMED_FD; - unit_write_drop_in_private_format(u, mode, name, "StandardOutput=fd:%s", exec_context_fdname(c, STDOUT_FILENO)); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "StandardOutput=fd:%s", exec_context_fdname(c, STDOUT_FILENO)); } else { assert(streq(name, "StandardErrorFileDescriptorName")); @@ -1848,7 +1850,7 @@ int bus_exec_context_set_transient_property( return r; c->std_error = EXEC_OUTPUT_NAMED_FD; - unit_write_drop_in_private_format(u, mode, name, "StandardError=fd:%s", exec_context_fdname(c, STDERR_FILENO)); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "StandardError=fd:%s", exec_context_fdname(c, STDERR_FILENO)); } } @@ -1866,7 +1868,7 @@ int bus_exec_context_set_transient_property( if (!path_is_normalized(s)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not normalized", s); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (streq(name, "StandardInputFile")) { r = free_and_strdup(&c->stdio_file[STDIN_FILENO], s); @@ -1874,7 +1876,7 @@ int bus_exec_context_set_transient_property( return r; c->std_input = EXEC_INPUT_FILE; - unit_write_drop_in_private_format(u, mode, name, "StandardInput=file:%s", s); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "StandardInput=file:%s", s); } else if (streq(name, "StandardOutputFile")) { r = free_and_strdup(&c->stdio_file[STDOUT_FILENO], s); @@ -1882,7 +1884,7 @@ int bus_exec_context_set_transient_property( return r; c->std_output = EXEC_OUTPUT_FILE; - unit_write_drop_in_private_format(u, mode, name, "StandardOutput=file:%s", s); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "StandardOutput=file:%s", s); } else { assert(streq(name, "StandardErrorFile")); @@ -1892,7 +1894,7 @@ int bus_exec_context_set_transient_property( return r; c->std_error = EXEC_OUTPUT_FILE; - unit_write_drop_in_private_format(u, mode, name, "StandardError=file:%s", s); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "StandardError=file:%s", s); } } @@ -1906,14 +1908,14 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *encoded = NULL; if (sz == 0) { c->stdin_data = mfree(c->stdin_data); c->stdin_data_size = 0; - unit_write_drop_in_private(u, mode, name, "StandardInputData="); + unit_write_settingf(u, flags, name, "StandardInputData="); } else { void *q; ssize_t n; @@ -1935,7 +1937,7 @@ int bus_exec_context_set_transient_property( c->stdin_data = q; c->stdin_data_size += sz; - unit_write_drop_in_private_format(u, mode, name, "StandardInputData=%s", encoded); + unit_write_settingf(u, flags, name, "StandardInputData=%s", encoded); } } @@ -1954,7 +1956,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (streq(name, "IgnoreSIGPIPE")) c->ignore_sigpipe = b; else if (streq(name, "TTYVHangup")) @@ -1998,7 +2000,7 @@ int bus_exec_context_set_transient_property( else if (streq(name, "LockPersonality")) c->lock_personality = b; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, yes_no(b)); + unit_write_settingf(u, flags, name, "%s=%s", name, yes_no(b)); } return 1; @@ -2010,13 +2012,13 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { - if (isempty(id)) - c->utmp_id = mfree(c->utmp_id); - else if (free_and_strdup(&c->utmp_id, id) < 0) - return -ENOMEM; + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - unit_write_drop_in_private_format(u, mode, name, "UtmpIdentifier=%s", strempty(id)); + r = free_and_strdup(&c->utmp_id, empty_to_null(id)); + if (r < 0) + return r; + + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "UtmpIdentifier=%s", strempty(id)); } return 1; @@ -2033,10 +2035,10 @@ int bus_exec_context_set_transient_property( if (m < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid utmp mode"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->utmp_mode = m; - unit_write_drop_in_private_format(u, mode, name, "UtmpMode=%s", exec_utmp_mode_to_string(m)); + unit_write_settingf(u, flags, name, "UtmpMode=%s", exec_utmp_mode_to_string(m)); } return 1; @@ -2048,13 +2050,13 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { - if (isempty(n)) - c->pam_name = mfree(c->pam_name); - else if (free_and_strdup(&c->pam_name, n) < 0) - return -ENOMEM; + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - unit_write_drop_in_private_format(u, mode, name, "PAMName=%s", strempty(n)); + r = free_and_strdup(&c->pam_name, empty_to_null(n)); + if (r < 0) + return r; + + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "PAMName=%s", strempty(n)); } return 1; @@ -2070,14 +2072,18 @@ int bus_exec_context_set_transient_property( if (!strv_env_is_valid(l)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid environment block."); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (strv_length(l) == 0) { c->environment = strv_free(c->environment); - unit_write_drop_in_private_format(u, mode, name, "Environment="); + unit_write_setting(u, flags, name, "Environment="); } else { _cleanup_free_ char *joined = NULL; char **e; + joined = unit_concat_strv(l, UNIT_ESCAPE_SPECIFIERS|UNIT_ESCAPE_C); + if (!joined) + return -ENOMEM; + e = strv_env_merge(2, c->environment, l); if (!e) return -ENOMEM; @@ -2085,11 +2091,7 @@ int bus_exec_context_set_transient_property( strv_free(c->environment); c->environment = e; - joined = strv_join_quoted(l); - if (!joined) - return -ENOMEM; - - unit_write_drop_in_private_format(u, mode, name, "Environment=%s", joined); + unit_write_settingf(u, flags, name, "Environment=%s", joined); } } @@ -2106,14 +2108,18 @@ int bus_exec_context_set_transient_property( if (!strv_env_name_or_assignment_is_valid(l)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid UnsetEnvironment= list."); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (strv_length(l) == 0) { c->unset_environment = strv_free(c->unset_environment); - unit_write_drop_in_private_format(u, mode, name, "UnsetEnvironment="); + unit_write_setting(u, flags, name, "UnsetEnvironment="); } else { _cleanup_free_ char *joined = NULL; char **e; + joined = unit_concat_strv(l, UNIT_ESCAPE_SPECIFIERS|UNIT_ESCAPE_C); + if (!joined) + return -ENOMEM; + e = strv_env_merge(2, c->unset_environment, l); if (!e) return -ENOMEM; @@ -2121,12 +2127,7 @@ int bus_exec_context_set_transient_property( strv_free(c->unset_environment); c->unset_environment = e; - /* We write just the new settings out to file, with unresolved specifiers */ - joined = strv_join_quoted(l); - if (!joined) - return -ENOMEM; - - unit_write_drop_in_private_format(u, mode, name, "UnsetEnvironment=%s", joined); + unit_write_settingf(u, flags, name, "UnsetEnvironment=%s", joined); } } @@ -2140,9 +2141,9 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->timer_slack_nsec = n; - unit_write_drop_in_private_format(u, mode, name, "TimerSlackNSec=" NSEC_FMT, n); + unit_write_settingf(u, flags, name, "TimerSlackNSec=" NSEC_FMT, n); } return 1; @@ -2157,10 +2158,10 @@ int bus_exec_context_set_transient_property( if (!oom_score_adjust_is_valid(oa)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "OOM score adjust value out of range"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->oom_score_adjust = oa; c->oom_score_adjust_set = true; - unit_write_drop_in_private_format(u, mode, name, "OOMScoreAdjust=%i", oa); + unit_write_settingf(u, flags, name, "OOMScoreAdjust=%i", oa); } return 1; @@ -2181,8 +2182,17 @@ int bus_exec_context_set_transient_property( if (!f) return -ENOMEM; - STRV_FOREACH(i, c->environment_files) - fprintf(f, "EnvironmentFile=%s", *i); + fputs("EnvironmentFile=\n", f); + + STRV_FOREACH(i, c->environment_files) { + _cleanup_free_ char *q = NULL; + + q = specifier_escape(*i); + if (!q) + return -ENOMEM; + + fprintf(f, "EnvironmentFile=%s\n", q); + } while ((r = sd_bus_message_enter_container(message, 'r', "sb")) > 0) { const char *path; @@ -2199,14 +2209,21 @@ int bus_exec_context_set_transient_property( if (!path_is_absolute(path)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not absolute.", path); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { + _cleanup_free_ char *q = NULL; char *buf; buf = strjoin(b ? "-" : "", path); if (!buf) return -ENOMEM; - fprintf(f, "EnvironmentFile=%s", buf); + q = specifier_escape(buf); + if (!q) { + free(buf); + return -ENOMEM; + } + + fprintf(f, "EnvironmentFile=%s\n", q); r = strv_consume(&l, buf); if (r < 0) @@ -2224,16 +2241,16 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (strv_isempty(l)) { c->environment_files = strv_free(c->environment_files); - unit_write_drop_in_private(u, mode, name, "EnvironmentFile="); + unit_write_setting(u, flags, name, "EnvironmentFile="); } else { r = strv_extend_strv(&c->environment_files, l, true); if (r < 0) return r; - unit_write_drop_in_private(u, mode, name, joined); + unit_write_setting(u, flags, name, joined); } } @@ -2250,19 +2267,19 @@ int bus_exec_context_set_transient_property( if (!strv_env_name_is_valid(l)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid PassEnvironment= block."); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (strv_isempty(l)) { c->pass_environment = strv_free(c->pass_environment); - unit_write_drop_in_private_format(u, mode, name, "PassEnvironment="); + unit_write_setting(u, flags, name, "PassEnvironment="); } else { _cleanup_free_ char *joined = NULL; /* We write just the new settings out to file, with unresolved specifiers. */ - joined = strv_join_quoted(l); + joined = unit_concat_strv(l, UNIT_ESCAPE_SPECIFIERS); if (!joined) return -ENOMEM; - unit_write_drop_in_private_format(u, mode, name, "PassEnvironment=%s", joined); + unit_write_settingf(u, flags, name, "PassEnvironment=%s", joined); } } @@ -2291,9 +2308,7 @@ int bus_exec_context_set_transient_property( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid %s", name); } - if (mode != UNIT_CHECK) { - _cleanup_free_ char *joined = NULL; - + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (STR_IN_SET(name, "ReadWriteDirectories", "ReadWritePaths")) dirs = &c->read_write_paths; else if (STR_IN_SET(name, "ReadOnlyDirectories", "ReadOnlyPaths")) @@ -2303,19 +2318,20 @@ int bus_exec_context_set_transient_property( if (strv_length(l) == 0) { *dirs = strv_free(*dirs); - unit_write_drop_in_private_format(u, mode, name, "%s=", name); + unit_write_settingf(u, flags, name, "%s=", name); } else { + _cleanup_free_ char *joined = NULL; + + joined = unit_concat_strv(l, UNIT_ESCAPE_SPECIFIERS); + if (!joined) + return -ENOMEM; + r = strv_extend_strv(dirs, l, true); if (r < 0) return -ENOMEM; - joined = strv_join_quoted(*dirs); - if (!joined) - return -ENOMEM; - - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, joined); + unit_write_settingf(u, flags, name, "%s=%s", name, joined); } - } return 1; @@ -2339,9 +2355,9 @@ int bus_exec_context_set_transient_property( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Failed to parse protect system value"); } - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->protect_system = ps; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, s); + unit_write_settingf(u, flags, name, "%s=%s", name, s); } return 1; @@ -2365,9 +2381,9 @@ int bus_exec_context_set_transient_property( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Failed to parse protect home value"); } - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->protect_home = ph; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, s); + unit_write_settingf(u, flags, name, "%s=%s", name, s); } return 1; @@ -2385,10 +2401,10 @@ int bus_exec_context_set_transient_property( if (m < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid keyring mode"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->keyring_mode = m; - unit_write_drop_in_private_format(u, mode, name, "KeyringMode=%s", exec_keyring_mode_to_string(m)); + unit_write_settingf(u, flags, name, "KeyringMode=%s", exec_keyring_mode_to_string(m)); } return 1; @@ -2405,10 +2421,10 @@ int bus_exec_context_set_transient_property( if (m < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid preserve mode"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->runtime_directory_preserve_mode = m; - unit_write_drop_in_private_format(u, mode, name, "RuntimeDirectoryPreserve=%s", exec_preserve_mode_to_string(m)); + unit_write_settingf(u, flags, name, "RuntimeDirectoryPreserve=%s", exec_preserve_mode_to_string(m)); } return 1; @@ -2420,7 +2436,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { ExecDirectoryType i; if (streq(name, "UMask")) @@ -2432,7 +2448,7 @@ int bus_exec_context_set_transient_property( break; } - unit_write_drop_in_private_format(u, mode, name, "%s=%040o", name, m); + unit_write_settingf(u, flags, name, "%s=%040o", name, m); } return 1; @@ -2450,8 +2466,7 @@ int bus_exec_context_set_transient_property( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= path is not valid: %s", name, *p); } - if (mode != UNIT_CHECK) { - _cleanup_free_ char *joined = NULL; + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { char ***dirs = NULL; ExecDirectoryType i; @@ -2465,17 +2480,19 @@ int bus_exec_context_set_transient_property( if (strv_isempty(l)) { *dirs = strv_free(*dirs); - unit_write_drop_in_private_format(u, mode, name, "%s=", name); + unit_write_settingf(u, flags, name, "%s=", name); } else { + _cleanup_free_ char *joined = NULL; + r = strv_extend_strv(dirs, l, true); if (r < 0) return -ENOMEM; - joined = strv_join_quoted(*dirs); + joined = unit_concat_strv(l, UNIT_ESCAPE_SPECIFIERS); if (!joined) return -ENOMEM; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, joined); + unit_write_settingf(u, flags, name, "%s=%s", name, joined); } } @@ -2487,13 +2504,13 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (isempty(s)) c->selinux_context = mfree(c->selinux_context); else if (free_and_strdup(&c->selinux_context, s) < 0) return -ENOMEM; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, strempty(s)); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "%s=%s", name, strempty(s)); } return 1; @@ -2510,7 +2527,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { char **p; bool *b; @@ -2531,45 +2548,45 @@ int bus_exec_context_set_transient_property( *b = ignore; } - unit_write_drop_in_private_format(u, mode, name, "%s=%s%s", name, ignore ? "-" : "", strempty(s)); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "%s=%s%s", name, ignore ? "-" : "", strempty(s)); } return 1; } else if (streq(name, "RestrictNamespaces")) { - uint64_t flags; + uint64_t rf; - r = sd_bus_message_read(message, "t", &flags); + r = sd_bus_message_read(message, "t", &rf); if (r < 0) return r; - if ((flags & NAMESPACE_FLAGS_ALL) != flags) + if ((rf & NAMESPACE_FLAGS_ALL) != rf) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unknown namespace types"); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *s = NULL; - r = namespace_flag_to_string_many(flags, &s); + r = namespace_flag_to_string_many(rf, &s); if (r < 0) return r; - c->restrict_namespaces = flags; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, s); + c->restrict_namespaces = rf; + unit_write_settingf(u, flags, name, "%s=%s", name, s); } return 1; } else if (streq(name, "MountFlags")) { - uint64_t flags; + uint64_t fl; - r = sd_bus_message_read(message, "t", &flags); + r = sd_bus_message_read(message, "t", &fl); if (r < 0) return r; - if (!IN_SET(flags, 0, MS_SHARED, MS_PRIVATE, MS_SLAVE)) + if (!IN_SET(fl, 0, MS_SHARED, MS_PRIVATE, MS_SLAVE)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unknown mount propagation flags"); - if (mode != UNIT_CHECK) { - c->mount_flags = flags; + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { + c->mount_flags = fl; - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, mount_propagation_flags_to_string(flags)); + unit_write_settingf(u, flags, name, "%s=%s", name, mount_propagation_flags_to_string(fl)); } return 1; @@ -2600,7 +2617,7 @@ int bus_exec_context_set_transient_property( if (!IN_SET(mount_flags, 0, MS_REC)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unknown mount flags."); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { r = bind_mount_add(&c->bind_mounts, &c->n_bind_mounts, &(BindMount) { .source = strdup(source), @@ -2612,8 +2629,8 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - unit_write_drop_in_private_format( - u, mode, name, + unit_write_settingf( + u, flags|UNIT_ESCAPE_SPECIFIERS, name, "%s=%s%s:%s:%s", name, ignore_enoent ? "-" : "", @@ -2635,6 +2652,8 @@ int bus_exec_context_set_transient_property( bind_mount_free_many(c->bind_mounts, c->n_bind_mounts); c->bind_mounts = NULL; c->n_bind_mounts = 0; + + unit_write_settingf(u, flags, name, "%s=", name); } return 1; @@ -2671,7 +2690,7 @@ int bus_exec_context_set_transient_property( return -ERANGE; } - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *f = NULL; struct rlimit nl; @@ -2701,7 +2720,7 @@ int bus_exec_context_set_transient_property( return -ENOMEM; } - unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, f); + unit_write_settingf(u, flags, name, "%s=%s", name, f); } return 1; diff --git a/src/core/dbus-execute.h b/src/core/dbus-execute.h index 631edcc8c8..f30f0774cd 100644 --- a/src/core/dbus-execute.h +++ b/src/core/dbus-execute.h @@ -43,4 +43,4 @@ int bus_property_get_exec_output(sd_bus *bus, const char *path, const char *inte int bus_property_get_exec_command(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *ret_error); int bus_property_get_exec_command_list(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *ret_error); -int bus_exec_context_set_transient_property(Unit *u, ExecContext *c, const char *name, sd_bus_message *message, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_exec_context_set_transient_property(Unit *u, ExecContext *c, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); diff --git a/src/core/dbus-kill.c b/src/core/dbus-kill.c index 4ac9f363cc..bf3bbb2047 100644 --- a/src/core/dbus-kill.c +++ b/src/core/dbus-kill.c @@ -39,7 +39,7 @@ int bus_kill_context_set_transient_property( KillContext *c, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { int r; @@ -49,6 +49,8 @@ int bus_kill_context_set_transient_property( assert(name); assert(message); + flags |= UNIT_PRIVATE; + if (streq(name, "KillMode")) { const char *m; KillMode k; @@ -61,10 +63,10 @@ int bus_kill_context_set_transient_property( if (k < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Kill mode '%s' not known.", m); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->kill_mode = k; - unit_write_drop_in_private_format(u, mode, name, "KillMode=%s", kill_mode_to_string(k)); + unit_write_settingf(u, flags, name, "KillMode=%s", kill_mode_to_string(k)); } return 1; @@ -79,10 +81,10 @@ int bus_kill_context_set_transient_property( if (!SIGNAL_VALID(sig)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Signal %i out of range", sig); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->kill_signal = sig; - unit_write_drop_in_private_format(u, mode, name, "KillSignal=%s", signal_to_string(sig)); + unit_write_settingf(u, flags, name, "KillSignal=%s", signal_to_string(sig)); } return 1; @@ -94,10 +96,10 @@ int bus_kill_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->send_sighup = b; - unit_write_drop_in_private_format(u, mode, name, "SendSIGHUP=%s", yes_no(b)); + unit_write_settingf(u, flags, name, "SendSIGHUP=%s", yes_no(b)); } return 1; @@ -109,10 +111,10 @@ int bus_kill_context_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->send_sigkill = b; - unit_write_drop_in_private_format(u, mode, name, "SendSIGKILL=%s", yes_no(b)); + unit_write_settingf(u, flags, name, "SendSIGKILL=%s", yes_no(b)); } return 1; diff --git a/src/core/dbus-kill.h b/src/core/dbus-kill.h index 44fb9f9a93..1df3b37c65 100644 --- a/src/core/dbus-kill.h +++ b/src/core/dbus-kill.h @@ -27,4 +27,4 @@ extern const sd_bus_vtable bus_kill_vtable[]; -int bus_kill_context_set_transient_property(Unit *u, KillContext *c, const char *name, sd_bus_message *message, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_kill_context_set_transient_property(Unit *u, KillContext *c, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); diff --git a/src/core/dbus-mount.c b/src/core/dbus-mount.c index 794b292ad3..628bce0b6a 100644 --- a/src/core/dbus-mount.c +++ b/src/core/dbus-mount.c @@ -126,18 +126,19 @@ static int bus_mount_set_transient_property( Mount *m, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { const char *new_property; char **property; - char *p; int r; assert(m); assert(name); assert(message); + flags |= UNIT_PRIVATE; + if (streq(name, "What")) property = &m->parameters_fragment.what; else if (streq(name, "Options")) @@ -151,16 +152,13 @@ static int bus_mount_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { - p = strdup(new_property); - if (!p) - return -ENOMEM; + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - unit_write_drop_in_format(UNIT(m), mode, name, "[Mount]\n%s=%s\n", - name, new_property); + r = free_and_strdup(property, new_property); + if (r < 0) + return r; - free(*property); - *property = p; + unit_write_settingf(UNIT(m), flags|UNIT_ESCAPE_SPECIFIERS, name, "%s=%s", name, new_property); } return 1; @@ -170,7 +168,7 @@ int bus_mount_set_property( Unit *u, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { Mount *m = MOUNT(u); @@ -180,22 +178,22 @@ int bus_mount_set_property( assert(name); assert(message); - r = bus_cgroup_set_property(u, &m->cgroup_context, name, message, mode, error); + r = bus_cgroup_set_property(u, &m->cgroup_context, name, message, flags, error); if (r != 0) return r; if (u->transient && u->load_state == UNIT_STUB) { /* This is a transient unit, let's load a little more */ - r = bus_mount_set_transient_property(m, name, message, mode, error); + r = bus_mount_set_transient_property(m, name, message, flags, error); if (r != 0) return r; - r = bus_exec_context_set_transient_property(u, &m->exec_context, name, message, mode, error); + r = bus_exec_context_set_transient_property(u, &m->exec_context, name, message, flags, error); if (r != 0) return r; - r = bus_kill_context_set_transient_property(u, &m->kill_context, name, message, mode, error); + r = bus_kill_context_set_transient_property(u, &m->kill_context, name, message, flags, error); if (r != 0) return r; } diff --git a/src/core/dbus-mount.h b/src/core/dbus-mount.h index f0a763788f..5d5e1f679f 100644 --- a/src/core/dbus-mount.h +++ b/src/core/dbus-mount.h @@ -26,5 +26,5 @@ extern const sd_bus_vtable bus_mount_vtable[]; -int bus_mount_set_property(Unit *u, const char *name, sd_bus_message *message, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_mount_set_property(Unit *u, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); int bus_mount_commit_properties(Unit *u); diff --git a/src/core/dbus-scope.c b/src/core/dbus-scope.c index 4737c6d406..9195ad36d0 100644 --- a/src/core/dbus-scope.c +++ b/src/core/dbus-scope.c @@ -73,7 +73,7 @@ static int bus_scope_set_transient_property( Scope *s, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { int r; @@ -82,6 +82,8 @@ static int bus_scope_set_transient_property( assert(name); assert(message); + flags |= UNIT_PRIVATE; + if (streq(name, "PIDs")) { unsigned n = 0; uint32_t pid; @@ -95,7 +97,7 @@ static int bus_scope_set_transient_property( if (pid <= 1) return -EINVAL; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { r = unit_watch_pid(UNIT(s), pid); if (r < 0 && r != -EEXIST) return r; @@ -117,7 +119,6 @@ static int bus_scope_set_transient_property( } else if (streq(name, "Controller")) { const char *controller; - char *c; /* We can't support direct connections with this, as direct connections know no service or unique name * concept, but the Controller field stores exactly that. */ @@ -131,33 +132,25 @@ static int bus_scope_set_transient_property( if (!isempty(controller) && !service_name_is_valid(controller)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Controller '%s' is not a valid bus name.", controller); - if (mode != UNIT_CHECK) { - if (isempty(controller)) - c = NULL; - else { - c = strdup(controller); - if (!c) - return -ENOMEM; - } - - free(s->controller); - s->controller = c; + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { + r = free_and_strdup(&s->controller, empty_to_null(controller)); + if (r < 0) + return r; } return 1; } else if (streq(name, "TimeoutStopUSec")) { + uint64_t t; - if (mode != UNIT_CHECK) { - r = sd_bus_message_read(message, "t", &s->timeout_stop_usec); - if (r < 0) - return r; + r = sd_bus_message_read(message, "t", &t); + if (r < 0) + return r; - unit_write_drop_in_private_format(UNIT(s), mode, name, "TimeoutStopSec="USEC_FMT"us", s->timeout_stop_usec); - } else { - r = sd_bus_message_skip(message, "t"); - if (r < 0) - return r; + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { + s->timeout_stop_usec = t; + + unit_write_settingf(UNIT(s), flags, name, "TimeoutStopSec=" USEC_FMT "us", t); } return 1; @@ -170,7 +163,7 @@ int bus_scope_set_property( Unit *u, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { Scope *s = SCOPE(u); @@ -180,18 +173,18 @@ int bus_scope_set_property( assert(name); assert(message); - r = bus_cgroup_set_property(u, &s->cgroup_context, name, message, mode, error); + r = bus_cgroup_set_property(u, &s->cgroup_context, name, message, flags, error); if (r != 0) return r; if (u->load_state == UNIT_STUB) { /* While we are created we still accept PIDs */ - r = bus_scope_set_transient_property(s, name, message, mode, error); + r = bus_scope_set_transient_property(s, name, message, flags, error); if (r != 0) return r; - r = bus_kill_context_set_transient_property(u, &s->kill_context, name, message, mode, error); + r = bus_kill_context_set_transient_property(u, &s->kill_context, name, message, flags, error); if (r != 0) return r; } diff --git a/src/core/dbus-scope.h b/src/core/dbus-scope.h index c80317c4b2..aa604897eb 100644 --- a/src/core/dbus-scope.h +++ b/src/core/dbus-scope.h @@ -26,7 +26,7 @@ extern const sd_bus_vtable bus_scope_vtable[]; -int bus_scope_set_property(Unit *u, const char *name, sd_bus_message *i, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_scope_set_property(Unit *u, const char *name, sd_bus_message *i, UnitWriteFlags flags, sd_bus_error *error); int bus_scope_commit_properties(Unit *u); int bus_scope_send_request_stop(Scope *s); diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 7b6cb395d8..593094ee33 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -90,7 +90,7 @@ static int bus_service_set_transient_property( Service *s, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { int r; @@ -99,6 +99,8 @@ static int bus_service_set_transient_property( assert(name); assert(message); + flags |= UNIT_PRIVATE; + if (streq(name, "RemainAfterExit")) { int b; @@ -106,9 +108,9 @@ static int bus_service_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { s->remain_after_exit = b; - unit_write_drop_in_private_format(UNIT(s), mode, name, "RemainAfterExit=%s", yes_no(b)); + unit_write_settingf(UNIT(s), flags, name, "RemainAfterExit=%s", yes_no(b)); } return 1; @@ -125,9 +127,9 @@ static int bus_service_set_transient_property( if (k < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid service type %s", t); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { s->type = k; - unit_write_drop_in_private_format(UNIT(s), mode, name, "Type=%s", service_type_to_string(s->type)); + unit_write_settingf(UNIT(s), flags, name, "Type=%s", service_type_to_string(s->type)); } return 1; @@ -138,9 +140,9 @@ static int bus_service_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { s->runtime_max_usec = u; - unit_write_drop_in_private_format(UNIT(s), mode, name, "RuntimeMaxSec=" USEC_FMT "us", u); + unit_write_settingf(UNIT(s), flags, name, "RuntimeMaxSec=" USEC_FMT "us", u); } return 1; @@ -161,9 +163,9 @@ static int bus_service_set_transient_property( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid restart setting: %s", v); } - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { s->restart = sr; - unit_write_drop_in_private_format(UNIT(s), mode, name, "Restart=%s", service_restart_to_string(sr)); + unit_write_settingf(UNIT(s), flags, name, "Restart=%s", service_restart_to_string(sr)); } return 1; @@ -178,7 +180,7 @@ static int bus_service_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { int copy; copy = fcntl(fd, F_DUPFD_CLOEXEC, 3); @@ -208,9 +210,9 @@ static int bus_service_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { s->n_fd_store_max = (unsigned) u; - unit_write_drop_in_private_format(UNIT(s), mode, name, "FileDescriptorStoreMax=%" PRIu32, u); + unit_write_settingf(UNIT(s), flags, name, "FileDescriptorStoreMax=%" PRIu32, u); } return 1; @@ -227,9 +229,9 @@ static int bus_service_set_transient_property( if (k < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid notify access setting %s", t); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { s->notify_access = k; - unit_write_drop_in_private_format(UNIT(s), mode, name, "NotifyAccess=%s", notify_access_to_string(s->notify_access)); + unit_write_settingf(UNIT(s), flags, name, "NotifyAccess=%s", notify_access_to_string(s->notify_access)); } return 1; @@ -265,7 +267,7 @@ static int bus_service_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { ExecCommand *c; c = new0(ExecCommand, 1); @@ -297,7 +299,7 @@ static int bus_service_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *buf = NULL; _cleanup_fclose_ FILE *f = NULL; ExecCommand *c; @@ -313,22 +315,28 @@ static int bus_service_set_transient_property( fputs_unlocked("ExecStart=\n", f); LIST_FOREACH(command, c, s->exec_command[SERVICE_EXEC_START]) { - _cleanup_free_ char *a; + _cleanup_free_ char *a = NULL, *t = NULL; + const char *p; - a = strv_join_quoted(c->argv); + p = unit_escape_setting(c->path, UNIT_ESCAPE_C|UNIT_ESCAPE_SPECIFIERS, &t); + if (!p) + return -ENOMEM; + + a = unit_concat_strv(c->argv, UNIT_ESCAPE_C|UNIT_ESCAPE_SPECIFIERS); if (!a) return -ENOMEM; fprintf(f, "ExecStart=%s@%s %s\n", c->flags & EXEC_COMMAND_IGNORE_FAILURE ? "-" : "", - c->path, + p, a); } r = fflush_and_check(f); if (r < 0) return r; - unit_write_drop_in_private(UNIT(s), mode, name, buf); + + unit_write_setting(UNIT(s), flags, name, buf); } return 1; @@ -341,7 +349,7 @@ int bus_service_set_property( Unit *u, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { Service *s = SERVICE(u); @@ -351,22 +359,22 @@ int bus_service_set_property( assert(name); assert(message); - r = bus_cgroup_set_property(u, &s->cgroup_context, name, message, mode, error); + r = bus_cgroup_set_property(u, &s->cgroup_context, name, message, flags, error); if (r != 0) return r; if (u->transient && u->load_state == UNIT_STUB) { /* This is a transient unit, let's load a little more */ - r = bus_service_set_transient_property(s, name, message, mode, error); + r = bus_service_set_transient_property(s, name, message, flags, error); if (r != 0) return r; - r = bus_exec_context_set_transient_property(u, &s->exec_context, name, message, mode, error); + r = bus_exec_context_set_transient_property(u, &s->exec_context, name, message, flags, error); if (r != 0) return r; - r = bus_kill_context_set_transient_property(u, &s->kill_context, name, message, mode, error); + r = bus_kill_context_set_transient_property(u, &s->kill_context, name, message, flags, error); if (r != 0) return r; } diff --git a/src/core/dbus-service.h b/src/core/dbus-service.h index a0f89fa23c..2da29ec601 100644 --- a/src/core/dbus-service.h +++ b/src/core/dbus-service.h @@ -26,5 +26,5 @@ extern const sd_bus_vtable bus_service_vtable[]; -int bus_service_set_property(Unit *u, const char *name, sd_bus_message *i, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_service_set_property(Unit *u, const char *name, sd_bus_message *i, UnitWriteFlags flags, sd_bus_error *error); int bus_service_commit_properties(Unit *u); diff --git a/src/core/dbus-slice.c b/src/core/dbus-slice.c index d269cb2b1e..fa2ff72151 100644 --- a/src/core/dbus-slice.c +++ b/src/core/dbus-slice.c @@ -32,7 +32,7 @@ int bus_slice_set_property( Unit *u, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { Slice *s = SLICE(u); @@ -40,7 +40,7 @@ int bus_slice_set_property( assert(name); assert(u); - return bus_cgroup_set_property(u, &s->cgroup_context, name, message, mode, error); + return bus_cgroup_set_property(u, &s->cgroup_context, name, message, flags, error); } int bus_slice_commit_properties(Unit *u) { diff --git a/src/core/dbus-slice.h b/src/core/dbus-slice.h index 5855332126..0c21919ad1 100644 --- a/src/core/dbus-slice.h +++ b/src/core/dbus-slice.h @@ -26,5 +26,5 @@ extern const sd_bus_vtable bus_slice_vtable[]; -int bus_slice_set_property(Unit *u, const char *name, sd_bus_message *message, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_slice_set_property(Unit *u, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); int bus_slice_commit_properties(Unit *u); diff --git a/src/core/dbus-socket.c b/src/core/dbus-socket.c index 3c7d99c356..930b7fa87d 100644 --- a/src/core/dbus-socket.c +++ b/src/core/dbus-socket.c @@ -166,7 +166,7 @@ int bus_socket_set_property( Unit *u, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { Socket *s = SOCKET(u); @@ -175,7 +175,7 @@ int bus_socket_set_property( assert(name); assert(message); - return bus_cgroup_set_property(u, &s->cgroup_context, name, message, mode, error); + return bus_cgroup_set_property(u, &s->cgroup_context, name, message, flags, error); } int bus_socket_commit_properties(Unit *u) { diff --git a/src/core/dbus-socket.h b/src/core/dbus-socket.h index ee9663f904..e6db2d0772 100644 --- a/src/core/dbus-socket.h +++ b/src/core/dbus-socket.h @@ -26,5 +26,5 @@ extern const sd_bus_vtable bus_socket_vtable[]; -int bus_socket_set_property(Unit *u, const char *name, sd_bus_message *message, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_socket_set_property(Unit *u, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); int bus_socket_commit_properties(Unit *u); diff --git a/src/core/dbus-swap.c b/src/core/dbus-swap.c index ae67f3e6d5..795aaa94fe 100644 --- a/src/core/dbus-swap.c +++ b/src/core/dbus-swap.c @@ -96,7 +96,7 @@ int bus_swap_set_property( Unit *u, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { Swap *s = SWAP(u); @@ -105,7 +105,7 @@ int bus_swap_set_property( assert(name); assert(message); - return bus_cgroup_set_property(u, &s->cgroup_context, name, message, mode, error); + return bus_cgroup_set_property(u, &s->cgroup_context, name, message, flags, error); } int bus_swap_commit_properties(Unit *u) { diff --git a/src/core/dbus-swap.h b/src/core/dbus-swap.h index 7df5cc7e54..6cca7483d8 100644 --- a/src/core/dbus-swap.h +++ b/src/core/dbus-swap.h @@ -27,5 +27,5 @@ extern const sd_bus_vtable bus_swap_vtable[]; -int bus_swap_set_property(Unit *u, const char *name, sd_bus_message *message, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_swap_set_property(Unit *u, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); int bus_swap_commit_properties(Unit *u); diff --git a/src/core/dbus-timer.c b/src/core/dbus-timer.c index 282d7f6fab..e4403203ed 100644 --- a/src/core/dbus-timer.c +++ b/src/core/dbus-timer.c @@ -176,7 +176,7 @@ static int bus_timer_set_transient_property( Timer *t, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { int r; @@ -185,6 +185,8 @@ static int bus_timer_set_transient_property( assert(name); assert(message); + flags |= UNIT_PRIVATE; + if (STR_IN_SET(name, "OnActiveSec", "OnBootSec", @@ -204,10 +206,10 @@ static int bus_timer_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { char time[FORMAT_TIMESPAN_MAX]; - unit_write_drop_in_private_format(UNIT(t), mode, name, "%s=%s", name, format_timespan(time, sizeof(time), u, USEC_PER_MSEC)); + unit_write_settingf(UNIT(t), flags|UNIT_ESCAPE_SPECIFIERS, name, "%s=%s", name, format_timespan(time, sizeof(time), u, USEC_PER_MSEC)); v = new0(TimerValue, 1); if (!v) @@ -231,12 +233,12 @@ static int bus_timer_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { r = calendar_spec_from_string(str, &c); if (r < 0) return r; - unit_write_drop_in_private_format(UNIT(t), mode, name, "%s=%s", name, str); + unit_write_settingf(UNIT(t), flags|UNIT_ESCAPE_SPECIFIERS, name, "%s=%s", name, str); v = new0(TimerValue, 1); if (!v) { @@ -262,9 +264,9 @@ static int bus_timer_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { t->accuracy_usec = u; - unit_write_drop_in_private_format(UNIT(t), mode, name, "AccuracySec=" USEC_FMT "us", u); + unit_write_settingf(UNIT(t), flags, name, "AccuracySec=" USEC_FMT "us", u); } return 1; @@ -276,9 +278,9 @@ static int bus_timer_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { t->random_usec = u; - unit_write_drop_in_private_format(UNIT(t), mode, name, "RandomizedDelaySec=" USEC_FMT "us", u); + unit_write_settingf(UNIT(t), flags, name, "RandomizedDelaySec=" USEC_FMT "us", u); } return 1; @@ -290,9 +292,9 @@ static int bus_timer_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { t->wake_system = b; - unit_write_drop_in_private_format(UNIT(t), mode, name, "%s=%s", name, yes_no(b)); + unit_write_settingf(UNIT(t), flags, name, "%s=%s", name, yes_no(b)); } return 1; @@ -304,9 +306,9 @@ static int bus_timer_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { t->remain_after_elapse = b; - unit_write_drop_in_private_format(UNIT(t), mode, name, "%s=%s", name, yes_no(b)); + unit_write_settingf(UNIT(t), flags, name, "%s=%s", name, yes_no(b)); } return 1; @@ -319,21 +321,17 @@ int bus_timer_set_property( Unit *u, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags mode, sd_bus_error *error) { Timer *t = TIMER(u); - int r; assert(t); assert(name); assert(message); - if (u->transient && u->load_state == UNIT_STUB) { - r = bus_timer_set_transient_property(t, name, message, mode, error); - if (r != 0) - return r; - } + if (u->transient && u->load_state == UNIT_STUB) + return bus_timer_set_transient_property(t, name, message, mode, error); return 0; } diff --git a/src/core/dbus-timer.h b/src/core/dbus-timer.h index e71f6cfefb..d810b048ee 100644 --- a/src/core/dbus-timer.h +++ b/src/core/dbus-timer.h @@ -26,4 +26,4 @@ extern const sd_bus_vtable bus_timer_vtable[]; -int bus_timer_set_property(Unit *u, const char *name, sd_bus_message *i, UnitSetPropertiesMode mode, sd_bus_error *error); +int bus_timer_set_property(Unit *u, const char *name, sd_bus_message *i, UnitWriteFlags flags, sd_bus_error *error); diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index d4bdf1760a..5d4bc28cde 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1319,7 +1319,7 @@ static int bus_unit_set_transient_property( Unit *u, const char *name, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, sd_bus_error *error) { int r; @@ -1335,12 +1335,12 @@ static int bus_unit_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { r = unit_set_description(u, d); if (r < 0) return r; - unit_write_drop_in_format(u, mode, name, "[Unit]\nDescription=%s", d); + unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "Description=%s", d); } return 1; @@ -1352,9 +1352,9 @@ static int bus_unit_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { u->default_dependencies = b; - unit_write_drop_in_format(u, mode, name, "[Unit]\nDefaultDependencies=%s", yes_no(b)); + unit_write_settingf(u, flags, name, "DefaultDependencies=%s", yes_no(b)); } return 1; @@ -1371,9 +1371,9 @@ static int bus_unit_set_transient_property( if (m < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unknown garbage collection mode: %s", s); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { u->collect_mode = m; - unit_write_drop_in_format(u, mode, name, "[Unit]\nCollectMode=%s", collect_mode_to_string(m)); + unit_write_settingf(u, flags, name, "CollectMode=%s", collect_mode_to_string(m)); } return 1; @@ -1406,12 +1406,12 @@ static int bus_unit_set_transient_property( if (slice->type != UNIT_SLICE) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unit name '%s' is not a slice", s); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { r = unit_set_slice(u, slice); if (r < 0) return r; - unit_write_drop_in_private_format(u, mode, name, "Slice=%s", s); + unit_write_settingf(u, flags|UNIT_PRIVATE, name, "Slice=%s", s); } return 1; @@ -1448,7 +1448,7 @@ static int bus_unit_set_transient_property( if (!unit_name_is_valid(other, UNIT_NAME_PLAIN|UNIT_NAME_INSTANCE)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid unit name %s", other); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *label = NULL; r = unit_add_dependency_by_name(u, d, other, NULL, true, UNIT_DEPENDENCY_FILE); @@ -1459,7 +1459,7 @@ static int bus_unit_set_transient_property( if (!label) return -ENOMEM; - unit_write_drop_in_format(u, mode, label, "[Unit]\n%s=%s", name, other); + unit_write_settingf(u, flags, label, "%s=%s", name, other); } } @@ -1484,14 +1484,14 @@ static int bus_unit_set_transient_property( if (action < 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid emergency action: %s", s); - if (mode != UNIT_CHECK) { + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (streq(name, "FailureAction")) u->failure_action = action; else u->success_action = action; - unit_write_drop_in_format(u, mode, name, "%s=%s", name, emergency_action_to_string(action)); + unit_write_settingf(u, flags, name, "%s=%s", name, emergency_action_to_string(action)); } return 1; @@ -1514,7 +1514,7 @@ static int bus_unit_set_transient_property( if (r < 0) return r; - if (mode != UNIT_CHECK) + if (!UNIT_WRITE_FLAGS_NOOP(flags)) u->bus_track_add = b; return 1; @@ -1526,7 +1526,7 @@ static int bus_unit_set_transient_property( int bus_unit_set_properties( Unit *u, sd_bus_message *message, - UnitSetPropertiesMode mode, + UnitWriteFlags flags, bool commit, sd_bus_error *error) { @@ -1548,12 +1548,13 @@ int bus_unit_set_properties( for (;;) { const char *name; + UnitWriteFlags f; r = sd_bus_message_enter_container(message, 'r', "sv"); if (r < 0) return r; if (r == 0) { - if (for_real || mode == UNIT_CHECK) + if (for_real || UNIT_WRITE_FLAGS_NOOP(flags)) break; /* Reached EOF. Let's try again, and this time for realz... */ @@ -1576,9 +1577,12 @@ int bus_unit_set_properties( if (r < 0) return r; - r = UNIT_VTABLE(u)->bus_set_property(u, name, message, for_real ? mode : UNIT_CHECK, error); + /* If not for real, then mask out the two target flags */ + f = for_real ? flags : (flags & ~(UNIT_RUNTIME|UNIT_PERSISTENT)); + + r = UNIT_VTABLE(u)->bus_set_property(u, name, message, f, error); if (r == 0 && u->transient && u->load_state == UNIT_STUB) - r = bus_unit_set_transient_property(u, name, message, for_real ? mode : UNIT_CHECK, error); + r = bus_unit_set_transient_property(u, name, message, f, error); if (r < 0) return r; if (r == 0) diff --git a/src/core/dbus-unit.h b/src/core/dbus-unit.h index b723453f37..d7066eefc8 100644 --- a/src/core/dbus-unit.h +++ b/src/core/dbus-unit.h @@ -34,7 +34,7 @@ int bus_unit_method_start_generic(sd_bus_message *message, Unit *u, JobType job_ int bus_unit_method_kill(sd_bus_message *message, void *userdata, sd_bus_error *error); int bus_unit_method_reset_failed(sd_bus_message *message, void *userdata, sd_bus_error *error); -int bus_unit_set_properties(Unit *u, sd_bus_message *message, UnitSetPropertiesMode mode, bool commit, sd_bus_error *error); +int bus_unit_set_properties(Unit *u, sd_bus_message *message, UnitWriteFlags flags, bool commit, sd_bus_error *error); int bus_unit_method_set_properties(sd_bus_message *message, void *userdata, sd_bus_error *error); int bus_unit_method_get_processes(sd_bus_message *message, void *userdata, sd_bus_error *error); int bus_unit_method_ref(sd_bus_message *message, void *userdata, sd_bus_error *error); diff --git a/src/core/unit.c b/src/core/unit.c index 856939853a..90a09ce659 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -55,6 +55,7 @@ #include "signal-util.h" #include "sparse-endian.h" #include "special.h" +#include "specifier.h" #include "stat-util.h" #include "stdio-util.h" #include "string-table.h" @@ -117,6 +118,8 @@ Unit *unit_new(Manager *m, size_t size) { u->ipv4_deny_map_fd = -1; u->ipv6_deny_map_fd = -1; + u->last_section_private = -1; + RATELIMIT_INIT(u->start_limit, m->default_start_limit_interval, m->default_start_limit_burst); RATELIMIT_INIT(u->auto_stop_ratelimit, 10 * USEC_PER_SEC, 16); @@ -4120,43 +4123,156 @@ ExecRuntime *unit_get_exec_runtime(Unit *u) { return *(ExecRuntime**) ((uint8_t*) u + offset); } -static const char* unit_drop_in_dir(Unit *u, UnitSetPropertiesMode mode) { +static const char* unit_drop_in_dir(Unit *u, UnitWriteFlags flags) { assert(u); - if (!IN_SET(mode, UNIT_RUNTIME, UNIT_PERSISTENT)) + if (UNIT_WRITE_FLAGS_NOOP(flags)) return NULL; if (u->transient) /* Redirect drop-ins for transient units always into the transient directory. */ return u->manager->lookup_paths.transient; - if (mode == UNIT_RUNTIME) - return u->manager->lookup_paths.runtime_control; - - if (mode == UNIT_PERSISTENT) + if (flags & UNIT_PERSISTENT) return u->manager->lookup_paths.persistent_control; + if (flags & UNIT_RUNTIME) + return u->manager->lookup_paths.runtime_control; + return NULL; } -int unit_write_drop_in(Unit *u, UnitSetPropertiesMode mode, const char *name, const char *data) { - _cleanup_free_ char *p = NULL, *q = NULL; +char* unit_escape_setting(const char *s, UnitWriteFlags flags, char **buf) { + char *ret = NULL; + + if (!s) + return NULL; + + /* Escapes the input string as requested. Returns the escaped string. If 'buf' is specified then the allocated + * return buffer pointer is also written to *buf, except if no escaping was necessary, in which case *buf is + * set to NULL, and the input pointer is returned as-is. This means the return value always contains a properly + * escaped version, but *buf when passed only contains a pointer if an allocation was necessary. If *buf is + * not specified, then the return value always needs to be freed. Callers can use this to optimize memory + * allocations. */ + + if (flags & UNIT_ESCAPE_SPECIFIERS) { + ret = specifier_escape(s); + if (!ret) + return NULL; + + s = ret; + } + + if (flags & UNIT_ESCAPE_C) { + char *a; + + a = cescape(s); + free(ret); + if (!a) + return NULL; + + ret = a; + } + + if (buf) { + *buf = ret; + return ret ?: (char*) s; + } + + return ret ?: strdup(s); +} + +char* unit_concat_strv(char **l, UnitWriteFlags flags) { + _cleanup_free_ char *result = NULL; + size_t n = 0, allocated = 0; + char **i, *ret; + + /* Takes a list of strings, escapes them, and concatenates them. This may be used to format command lines in a + * way suitable for ExecStart= stanzas */ + + STRV_FOREACH(i, l) { + _cleanup_free_ char *buf = NULL; + const char *p; + size_t a; + char *q; + + p = unit_escape_setting(*i, flags, &buf); + if (!p) + return NULL; + + a = (n > 0) + 1 + strlen(p) + 1; /* separating space + " + entry + " */ + if (!GREEDY_REALLOC(result, allocated, n + a + 1)) + return NULL; + + q = result + n; + if (n > 0) + *(q++) = ' '; + + *(q++) = '"'; + q = stpcpy(q, p); + *(q++) = '"'; + + n += a; + } + + if (!GREEDY_REALLOC(result, allocated, n + 1)) + return NULL; + + result[n] = 0; + + ret = result; + result = NULL; + + return ret; +} + +int unit_write_setting(Unit *u, UnitWriteFlags flags, const char *name, const char *data) { + _cleanup_free_ char *p = NULL, *q = NULL, *escaped = NULL; const char *dir, *wrapped; int r; assert(u); + assert(name); + assert(data); + + if (UNIT_WRITE_FLAGS_NOOP(flags)) + return 0; + + data = unit_escape_setting(data, flags, &escaped); + if (!data) + return -ENOMEM; + + /* Prefix the section header. If we are writing this out as transient file, then let's suppress this if the + * previous section header is the same */ + + if (flags & UNIT_PRIVATE) { + if (!UNIT_VTABLE(u)->private_section) + return -EINVAL; + + if (!u->transient_file || u->last_section_private < 0) + data = strjoina("[", UNIT_VTABLE(u)->private_section, "]\n", data); + else if (u->last_section_private == 0) + data = strjoina("\n[", UNIT_VTABLE(u)->private_section, "]\n", data); + } else { + if (!u->transient_file || u->last_section_private < 0) + data = strjoina("[Unit]\n", data); + else if (u->last_section_private > 0) + data = strjoina("\n[Unit]\n", data); + } if (u->transient_file) { /* When this is a transient unit file in creation, then let's not create a new drop-in but instead * write to the transient unit file. */ fputs(data, u->transient_file); - fputc('\n', u->transient_file); + + if (!endswith(data, "\n")) + fputc('\n', u->transient_file); + + /* Remember which section we wrote this entry to */ + u->last_section_private = !!(flags & UNIT_PRIVATE); return 0; } - if (!IN_SET(mode, UNIT_PERSISTENT, UNIT_RUNTIME)) - return 0; - - dir = unit_drop_in_dir(u, mode); + dir = unit_drop_in_dir(u, flags); if (!dir) return -EINVAL; @@ -4186,7 +4302,7 @@ int unit_write_drop_in(Unit *u, UnitSetPropertiesMode mode, const char *name, co return 0; } -int unit_write_drop_in_format(Unit *u, UnitSetPropertiesMode mode, const char *name, const char *format, ...) { +int unit_write_settingf(Unit *u, UnitWriteFlags flags, const char *name, const char *format, ...) { _cleanup_free_ char *p = NULL; va_list ap; int r; @@ -4195,7 +4311,7 @@ int unit_write_drop_in_format(Unit *u, UnitSetPropertiesMode mode, const char *n assert(name); assert(format); - if (!IN_SET(mode, UNIT_PERSISTENT, UNIT_RUNTIME)) + if (UNIT_WRITE_FLAGS_NOOP(flags)) return 0; va_start(ap, format); @@ -4205,47 +4321,7 @@ int unit_write_drop_in_format(Unit *u, UnitSetPropertiesMode mode, const char *n if (r < 0) return -ENOMEM; - return unit_write_drop_in(u, mode, name, p); -} - -int unit_write_drop_in_private(Unit *u, UnitSetPropertiesMode mode, const char *name, const char *data) { - const char *ndata; - - assert(u); - assert(name); - assert(data); - - if (!UNIT_VTABLE(u)->private_section) - return -EINVAL; - - if (!IN_SET(mode, UNIT_PERSISTENT, UNIT_RUNTIME)) - return 0; - - ndata = strjoina("[", UNIT_VTABLE(u)->private_section, "]\n", data); - - return unit_write_drop_in(u, mode, name, ndata); -} - -int unit_write_drop_in_private_format(Unit *u, UnitSetPropertiesMode mode, const char *name, const char *format, ...) { - _cleanup_free_ char *p = NULL; - va_list ap; - int r; - - assert(u); - assert(name); - assert(format); - - if (!IN_SET(mode, UNIT_PERSISTENT, UNIT_RUNTIME)) - return 0; - - va_start(ap, format); - r = vasprintf(&p, format, ap); - va_end(ap); - - if (r < 0) - return -ENOMEM; - - return unit_write_drop_in_private(u, mode, name, p); + return unit_write_setting(u, flags, name, p); } int unit_make_transient(Unit *u) { diff --git a/src/core/unit.h b/src/core/unit.h index cd7c08a20d..92bb4779a9 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -360,6 +360,10 @@ struct Unit { bool exported_invocation_id:1; bool exported_log_level_max:1; bool exported_log_extra_fields:1; + + /* When writing transient unit files, stores which section we stored last. If < 0, we didn't write any yet. If + * == 0 we are in the [Unit] section, if > 0 we are in the unit type-specific section. */ + int last_section_private:2; }; struct UnitStatusMessageFormats { @@ -368,11 +372,26 @@ struct UnitStatusMessageFormats { const char *finished_stop_job[_JOB_RESULT_MAX]; }; -typedef enum UnitSetPropertiesMode { - UNIT_CHECK = 0, - UNIT_RUNTIME = 1, - UNIT_PERSISTENT = 2, -} UnitSetPropertiesMode; +/* Flags used when writing drop-in files or transient unit files */ +typedef enum UnitWriteFlags { + /* Write a runtime unit file or drop-in (i.e. one below /run) */ + UNIT_RUNTIME = 1 << 0, + + /* Write a persistent drop-in (i.e. one below /etc) */ + UNIT_PERSISTENT = 1 << 1, + + /* Place this item in the per-unit-type private section, instead of [Unit] */ + UNIT_PRIVATE = 1 << 2, + + /* Apply specifier escaping before writing */ + UNIT_ESCAPE_SPECIFIERS = 1 << 3, + + /* Apply C escaping before writing */ + UNIT_ESCAPE_C = 1 << 4, +} UnitWriteFlags; + +/* Returns true if neither persistent, nor runtime storage is requested, i.e. this is a check invocation only */ +#define UNIT_WRITE_FLAGS_NOOP(flags) (((flags) & (UNIT_RUNTIME|UNIT_PERSISTENT)) == 0) #include "automount.h" #include "device.h" @@ -490,7 +509,7 @@ struct UnitVTable { void (*bus_name_owner_change)(Unit *u, const char *name, const char *old_owner, const char *new_owner); /* Called for each property that is being set */ - int (*bus_set_property)(Unit *u, const char *name, sd_bus_message *message, UnitSetPropertiesMode mode, sd_bus_error *error); + int (*bus_set_property)(Unit *u, const char *name, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); /* Called after at least one property got changed to apply the necessary change */ int (*bus_commit_properties)(Unit *u); @@ -713,11 +732,11 @@ ExecRuntime *unit_get_exec_runtime(Unit *u) _pure_; int unit_setup_exec_runtime(Unit *u); int unit_setup_dynamic_creds(Unit *u); -int unit_write_drop_in(Unit *u, UnitSetPropertiesMode mode, const char *name, const char *data); -int unit_write_drop_in_format(Unit *u, UnitSetPropertiesMode mode, const char *name, const char *format, ...) _printf_(4,5); +char* unit_escape_setting(const char *s, UnitWriteFlags flags, char **buf); +char* unit_concat_strv(char **l, UnitWriteFlags flags); -int unit_write_drop_in_private(Unit *u, UnitSetPropertiesMode mode, const char *name, const char *data); -int unit_write_drop_in_private_format(Unit *u, UnitSetPropertiesMode mode, const char *name, const char *format, ...) _printf_(4,5); +int unit_write_setting(Unit *u, UnitWriteFlags flags, const char *name, const char *data); +int unit_write_settingf(Unit *u, UnitWriteFlags mode, const char *name, const char *format, ...) _printf_(4,5); int unit_kill_context(Unit *u, KillContext *c, KillOperation k, pid_t main_pid, pid_t control_pid, bool main_pid_alien); From e74f76ca866c0c2a9149b6afe5bd5d349a46c68f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 22 Nov 2017 15:15:11 +0100 Subject: [PATCH 12/33] tree-wide: generate SD_BUS_ERROR_INVALID_ARGS when we get invalid arguments on bus calls Let's make sure that when we return a D-Bus error, we return a native one, if we generate it ourselves, and use errno-based error synthetization only if we received an errno ourselves. Yes, this makes things slightly longer, but is highly misleading as we propagate D-Bus errors, and not errnos to the client. --- src/core/dbus-cgroup.c | 46 ++++++++++++++++++++--------------------- src/core/dbus-service.c | 2 +- src/locale/localed.c | 4 ++-- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 0343aaed62..6c999f681f 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -381,7 +381,7 @@ static int bus_cgroup_set_transient_property( cc = cgroup_controller_from_string(t); if (cc < 0) - return sd_bus_error_set_errnof(error, EINVAL, "Unknown cgroup contoller '%s'", t); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unknown cgroup contoller '%s'", t); mask |= CGROUP_CONTROLLER_TO_MASK(cc); } @@ -453,7 +453,7 @@ int bus_cgroup_set_property( return r; if (!CGROUP_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "CPUWeight= value out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "CPUWeight= value out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->cpu_weight = weight; @@ -475,7 +475,7 @@ int bus_cgroup_set_property( return r; if (!CGROUP_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "StartupCPUWeight= value out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "StartupCPUWeight= value out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->startup_cpu_weight = weight; @@ -497,7 +497,7 @@ int bus_cgroup_set_property( return r; if (!CGROUP_CPU_SHARES_IS_OK(shares)) - return sd_bus_error_set_errnof(error, EINVAL, "CPUShares= value out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "CPUShares= value out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->cpu_shares = shares; @@ -519,7 +519,7 @@ int bus_cgroup_set_property( return r; if (!CGROUP_CPU_SHARES_IS_OK(shares)) - return sd_bus_error_set_errnof(error, EINVAL, "StartupCPUShares= value out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "StartupCPUShares= value out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->startup_cpu_shares = shares; @@ -541,7 +541,7 @@ int bus_cgroup_set_property( return r; if (u64 <= 0) - return sd_bus_error_set_errnof(error, EINVAL, "CPUQuotaPerSecUSec= value out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "CPUQuotaPerSecUSec= value out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->cpu_quota_per_sec_usec = u64; @@ -582,7 +582,7 @@ int bus_cgroup_set_property( return r; if (!CGROUP_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "IOWeight= value out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "IOWeight= value out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->io_weight = weight; @@ -604,7 +604,7 @@ int bus_cgroup_set_property( return r; if (CGROUP_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "StartupIOWeight= value out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "StartupIOWeight= value out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->startup_io_weight = weight; @@ -712,7 +712,7 @@ int bus_cgroup_set_property( while ((r = sd_bus_message_read(message, "(st)", &path, &weight)) > 0) { if (!CGROUP_WEIGHT_IS_OK(weight) || weight == CGROUP_WEIGHT_INVALID) - return sd_bus_error_set_errnof(error, EINVAL, "IODeviceWeight= value out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "IODeviceWeight= value out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { CGroupIODeviceWeight *a = NULL, *b; @@ -799,7 +799,7 @@ int bus_cgroup_set_property( return r; if (!CGROUP_BLKIO_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "BlockIOWeight= value out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "BlockIOWeight= value out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->blockio_weight = weight; @@ -821,7 +821,7 @@ int bus_cgroup_set_property( return r; if (!CGROUP_BLKIO_WEIGHT_IS_OK(weight)) - return sd_bus_error_set_errnof(error, EINVAL, "StartupBlockIOWeight= value out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "StartupBlockIOWeight= value out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->startup_blockio_weight = weight; @@ -945,7 +945,7 @@ int bus_cgroup_set_property( while ((r = sd_bus_message_read(message, "(st)", &path, &weight)) > 0) { if (!CGROUP_BLKIO_WEIGHT_IS_OK(weight) || weight == CGROUP_BLKIO_WEIGHT_INVALID) - return sd_bus_error_set_errnof(error, EINVAL, "BlockIODeviceWeight= out of range"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "BlockIODeviceWeight= out of range"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { CGroupBlockIODeviceWeight *a = NULL, *b; @@ -1032,7 +1032,7 @@ int bus_cgroup_set_property( if (r < 0) return r; if (v <= 0) - return sd_bus_error_set_errnof(error, EINVAL, "%s= is too small", name); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= is too small", name); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (streq(name, "MemoryLow")) @@ -1064,7 +1064,7 @@ int bus_cgroup_set_property( v = physical_memory_scale(raw, UINT32_MAX); if (v <= 0 || v == UINT64_MAX) - return sd_bus_error_set_errnof(error, EINVAL, "%s= is out of range", name); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= is out of range", name); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { const char *e; @@ -1094,7 +1094,7 @@ int bus_cgroup_set_property( if (r < 0) return r; if (limit <= 0) - return sd_bus_error_set_errnof(error, EINVAL, "%s= is too small", name); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= is too small", name); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->memory_limit = limit; @@ -1118,7 +1118,7 @@ int bus_cgroup_set_property( limit = physical_memory_scale(raw, UINT32_MAX); if (limit <= 0 || limit == UINT64_MAX) - return sd_bus_error_set_errnof(error, EINVAL, "%s= is out of range", name); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= is out of range", name); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->memory_limit = limit; @@ -1164,13 +1164,13 @@ int bus_cgroup_set_property( !startswith(path, "block-") && !startswith(path, "char-")) || strpbrk(path, WHITESPACE)) - return sd_bus_error_set_errnof(error, EINVAL, "DeviceAllow= requires device node"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "DeviceAllow= requires device node"); if (isempty(rwm)) rwm = "rwm"; if (!in_charset(rwm, "rwm")) - return sd_bus_error_set_errnof(error, EINVAL, "DeviceAllow= requires combination of rwm flags"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "DeviceAllow= requires combination of rwm flags"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { CGroupDeviceAllow *a = NULL, *b; @@ -1261,7 +1261,7 @@ int bus_cgroup_set_property( if (r < 0) return r; if (limit <= 0) - return sd_bus_error_set_errnof(error, EINVAL, "%s= is too small", name); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= is too small", name); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->tasks_max = limit; @@ -1285,7 +1285,7 @@ int bus_cgroup_set_property( limit = system_tasks_max_scale(raw, UINT32_MAX); if (limit <= 0 || limit >= UINT64_MAX) - return sd_bus_error_set_errnof(error, EINVAL, "%s= is out of range", name); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= is out of range", name); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { c->tasks_max = limit; @@ -1339,14 +1339,14 @@ int bus_cgroup_set_property( return r; if (!IN_SET(family, AF_INET, AF_INET6)) - return sd_bus_error_set_errnof(error, EINVAL, "%s= expects IPv4 or IPv6 addresses only.", name); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= expects IPv4 or IPv6 addresses only.", name); r = sd_bus_message_read_array(message, 'y', &ap, &an); if (r < 0) return r; if (an != FAMILY_ADDRESS_SIZE(family)) - return sd_bus_error_set_errnof(error, EINVAL, "IP address has wrong size for family (%s, expected %zu, got %zu)", + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "IP address has wrong size for family (%s, expected %zu, got %zu)", af_to_name(family), FAMILY_ADDRESS_SIZE(family), an); r = sd_bus_message_read(message, "u", &prefixlen); @@ -1354,7 +1354,7 @@ int bus_cgroup_set_property( return r; if (prefixlen > FAMILY_ADDRESS_SIZE(family)*8) - return sd_bus_error_set_errnof(error, EINVAL, "Prefix length %" PRIu32 " too large for address family %s.", prefixlen, af_to_name(family)); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Prefix length %" PRIu32 " too large for address family %s.", prefixlen, af_to_name(family)); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { IPAddressAccessItem *item; diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 593094ee33..f08d3186de 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -253,7 +253,7 @@ static int bus_service_set_transient_property( return r; if (!path_is_absolute(path)) - return sd_bus_error_set_errnof(error, EINVAL, "Path %s is not absolute.", path); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not absolute.", path); r = sd_bus_message_read_strv(message, &argv); if (r < 0) diff --git a/src/locale/localed.c b/src/locale/localed.c index 1f52c0a789..3e3f03e046 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -381,7 +381,7 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro if ((keymap && (!filename_is_valid(keymap) || !string_is_safe(keymap))) || (keymap_toggle && (!filename_is_valid(keymap_toggle) || !string_is_safe(keymap_toggle)))) - return sd_bus_error_set_errnof(error, -EINVAL, "Received invalid keymap data"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Received invalid keymap data"); r = bus_verify_polkit_async( m, @@ -560,7 +560,7 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err (model && !string_is_safe(model)) || (variant && !string_is_safe(variant)) || (options && !string_is_safe(options))) - return sd_bus_error_set_errnof(error, -EINVAL, "Received invalid keyboard data"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Received invalid keyboard data"); r = bus_verify_polkit_async( m, From 721060d4469d4d9f4466d6fe707f94a2c3256fe9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 22 Nov 2017 16:34:56 +0100 Subject: [PATCH 13/33] core: allow changing the description for units dynamically at runtime This makes "systemctl set-property foobar.service Description=waldo" work. See: #7245 --- src/core/dbus-unit.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 5d4bc28cde..cdab461d58 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1315,7 +1315,7 @@ int bus_unit_queue_job( return sd_bus_reply_method_return(message, "o", path); } -static int bus_unit_set_transient_property( +static int bus_unit_set_live_property( Unit *u, const char *name, sd_bus_message *message, @@ -1328,6 +1328,9 @@ static int bus_unit_set_transient_property( assert(name); assert(message); + /* Handles setting properties both "live" (i.e. at any time during runtime), and during creation (for transient + * units that are being created). */ + if (streq(name, "Description")) { const char *d; @@ -1344,8 +1347,28 @@ static int bus_unit_set_transient_property( } return 1; + } - } else if (streq(name, "DefaultDependencies")) { + return 0; +} + +static int bus_unit_set_transient_property( + Unit *u, + const char *name, + sd_bus_message *message, + UnitWriteFlags flags, + sd_bus_error *error) { + + int r; + + assert(u); + assert(name); + assert(message); + + /* Handles settings when transient units are created. This settings cannot be altered anymore after the unit + * has been created. */ + + if (streq(name, "DefaultDependencies")) { int b; r = sd_bus_message_read(message, "b", &b); @@ -1583,8 +1606,11 @@ int bus_unit_set_properties( r = UNIT_VTABLE(u)->bus_set_property(u, name, message, f, error); if (r == 0 && u->transient && u->load_state == UNIT_STUB) r = bus_unit_set_transient_property(u, name, message, f, error); + if (r == 0) + r = bus_unit_set_live_property(u, name, message, f, error); if (r < 0) return r; + if (r == 0) return sd_bus_error_setf(error, SD_BUS_ERROR_PROPERTY_READ_ONLY, "Cannot set property %s, or unknown property.", name); From e9f4aabd80e7a8551fb7c328e9a36aaf9ce131cc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Nov 2017 17:36:30 +0100 Subject: [PATCH 14/33] systemctl: reword "needs-reload" message a bit This message is displayed either when the unit file itself is newer than what is loaded, but also when any of the drop-ins is newer. Say so in the message, in order not to confuse the user unnecessarily. --- src/systemctl/systemctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 0d67905cdc..e52b4e32be 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2466,7 +2466,7 @@ static int need_daemon_reload(sd_bus *bus, const char *unit) { static void warn_unit_file_changed(const char *name) { assert(name); - log_warning("%sWarning:%s %s changed on disk. Run 'systemctl%s daemon-reload' to reload units.", + log_warning("%sWarning:%s The unit file, source configuration file or drop-ins of %s changed on disk. Run 'systemctl%s daemon-reload' to reload units.", ansi_highlight_red(), ansi_normal(), name, From fbc58835235f8a7a045be2831fe0a8d11f7d7bea Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Nov 2017 17:37:49 +0100 Subject: [PATCH 15/33] path-lookup: LookupPathsFlags are a flags type, hence define it like one We shouldn't rely on C's incremental assignment of values of enums for bit fields. That'll work only between the first two flags, but for everything following will break horrible. Hence, let's avoid any ambiguity here, and let's clearly define the flags as shifts of 1. --- src/shared/path-lookup.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/path-lookup.h b/src/shared/path-lookup.h index 8ea263e473..bcf9ca4de6 100644 --- a/src/shared/path-lookup.h +++ b/src/shared/path-lookup.h @@ -28,8 +28,8 @@ typedef struct LookupPaths LookupPaths; #include "macro.h" typedef enum LookupPathsFlags { - LOOKUP_PATHS_EXCLUDE_GENERATED = 1, - LOOKUP_PATHS_TEMPORARY_GENERATED, + LOOKUP_PATHS_EXCLUDE_GENERATED = 1 << 0, + LOOKUP_PATHS_TEMPORARY_GENERATED = 1 << 1, } LookupPathsFlags; struct LookupPaths { From 0126c8f3f68899d429d4a77ee1092c9b7f21aad6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Nov 2017 17:39:25 +0100 Subject: [PATCH 16/33] core: minor simplification --- src/core/unit.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index 90a09ce659..4d0cd6e65d 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4325,8 +4325,8 @@ int unit_write_settingf(Unit *u, UnitWriteFlags flags, const char *name, const c } int unit_make_transient(Unit *u) { + _cleanup_free_ char *path = NULL; FILE *f; - char *path; assert(u); @@ -4342,18 +4342,14 @@ int unit_make_transient(Unit *u) { RUN_WITH_UMASK(0022) { f = fopen(path, "we"); - if (!f) { - free(path); + if (!f) return -errno; - } } - if (u->transient_file) - fclose(u->transient_file); + safe_fclose(u->transient_file); u->transient_file = f; - free(u->fragment_path); - u->fragment_path = path; + free_and_replace(u->fragment_path, path); u->source_path = mfree(u->source_path); u->dropin_paths = strv_free(u->dropin_paths); From 45a7b16bae0dbda31caa54c608e72428cacdb6f4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Nov 2017 17:39:53 +0100 Subject: [PATCH 17/33] core: don't reference rescue/emergency targets in --user mode They are only defined for system mode, hence let's not check for them in --user mode. Follow-up for #7433 --- src/core/manager.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 0e21e14bec..ccc3e254fd 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3546,14 +3546,16 @@ ManagerState manager_state(Manager *m) { if (u && unit_active_or_pending(u)) return MANAGER_STOPPING; - /* Are the rescue or emergency targets active or queued? If so we are in maintenance state */ - u = manager_get_unit(m, SPECIAL_RESCUE_TARGET); - if (u && unit_active_or_pending(u)) - return MANAGER_MAINTENANCE; + if (MANAGER_IS_SYSTEM(m)) { + /* Are the rescue or emergency targets active or queued? If so we are in maintenance state */ + u = manager_get_unit(m, SPECIAL_RESCUE_TARGET); + if (u && unit_active_or_pending(u)) + return MANAGER_MAINTENANCE; - u = manager_get_unit(m, SPECIAL_EMERGENCY_TARGET); - if (u && unit_active_or_pending(u)) - return MANAGER_MAINTENANCE; + u = manager_get_unit(m, SPECIAL_EMERGENCY_TARGET); + if (u && unit_active_or_pending(u)) + return MANAGER_MAINTENANCE; + } /* Are there any failed units? If so, we are in degraded mode */ if (set_size(m->failed_units) > 0) From 45639f1be5f5aeea3c468abe69e38f68d52e39ee Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Nov 2017 17:45:58 +0100 Subject: [PATCH 18/33] core: never remove "transient" and "control" directories from unit search path This changes the unit search path logic to never drop the transient and control directories from the unit search path. This is necessary as we add new entries to both during runtime, due to the "systemctl set-property" and transient unit logic. Previously, the "transient" directory was created during early boot to deal with this, but the "control" directories were not covered like that. Creating the control directories early at boot is not possible however, as /etc might be read-only then, and we do define a persistent control directory. Hence, let's create these dirs on-demand when we need them, and make sure the search path clean-up logic never drops them from the search path even if they are initially missing. (Also, always create these paths properly labelled) --- src/core/manager.c | 7 ------- src/core/unit.c | 4 +++- src/shared/path-lookup.c | 8 ++++++++ 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index ccc3e254fd..be44ab3778 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1336,13 +1336,6 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { if (r < 0) return r; - /* Make sure the transient directory always exists, so that it remains - * in the search path */ - r = mkdir_p_label(m->lookup_paths.transient, 0755); - if (r < 0) - return log_error_errno(r, "Failed to create transient generator directory \"%s\": %m", - m->lookup_paths.transient); - dual_timestamp_get(m->timestamps + MANAGER_TIMESTAMP_GENERATORS_START); r = manager_run_generators(m); dual_timestamp_get(m->timestamps + MANAGER_TIMESTAMP_GENERATORS_FINISH); diff --git a/src/core/unit.c b/src/core/unit.c index 4d0cd6e65d..a12d24b90b 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4285,7 +4285,7 @@ int unit_write_setting(Unit *u, UnitWriteFlags flags, const char *name, const ch if (r < 0) return r; - (void) mkdir_p(p, 0755); + (void) mkdir_p_label(p, 0755); r = write_string_file_atomic_label(q, wrapped); if (r < 0) return r; @@ -4333,6 +4333,8 @@ int unit_make_transient(Unit *u) { if (!UNIT_VTABLE(u)->can_transient) return -EOPNOTSUPP; + (void) mkdir_p_label(u->manager->lookup_paths.transient, 0755); + path = strjoin(u->manager->lookup_paths.transient, "/", u->id); if (!path) return -ENOMEM; diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index b40887bfbd..57e0757529 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -736,6 +736,14 @@ int lookup_paths_reduce(LookupPaths *p) { struct stat st; unsigned k; + /* Never strip the transient and control directories from the path */ + if (path_equal_ptr(p->search_path[c], p->transient) || + path_equal_ptr(p->search_path[c], p->persistent_control) || + path_equal_ptr(p->search_path[c], p->runtime_control)) { + c++; + continue; + } + if (p->root_dir) r = lstat(p->search_path[c], &st); else From f6c66be1dc0396f406deb76b35a8c1bfacbbc965 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Nov 2017 17:58:34 +0100 Subject: [PATCH 19/33] core: open up all ExecXYZ= fields of service units to transient units Fixes: #7400 --- src/core/dbus-service.c | 12 +++--- src/shared/bus-unit-util.c | 88 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 5 deletions(-) diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index f08d3186de..8121765dad 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -93,6 +93,7 @@ static int bus_service_set_transient_property( UnitWriteFlags flags, sd_bus_error *error) { + ServiceExecCommand ci; int r; assert(s); @@ -236,7 +237,7 @@ static int bus_service_set_transient_property( return 1; - } else if (streq(name, "ExecStart")) { + } else if ((ci = service_exec_command_from_string(name)) >= 0) { unsigned n = 0; r = sd_bus_message_enter_container(message, 'a', "(sasb)"); @@ -286,7 +287,7 @@ static int bus_service_set_transient_property( c->flags = b ? EXEC_COMMAND_IGNORE_FAILURE : 0; path_kill_slashes(c->path); - exec_command_append_list(&s->exec_command[SERVICE_EXEC_START], c); + exec_command_append_list(&s->exec_command[ci], c); } n++; @@ -306,7 +307,7 @@ static int bus_service_set_transient_property( size_t size = 0; if (n == 0) - s->exec_command[SERVICE_EXEC_START] = exec_command_free_list(s->exec_command[SERVICE_EXEC_START]); + s->exec_command[ci] = exec_command_free_list(s->exec_command[ci]); f = open_memstream(&buf, &size); if (!f) @@ -314,7 +315,7 @@ static int bus_service_set_transient_property( fputs_unlocked("ExecStart=\n", f); - LIST_FOREACH(command, c, s->exec_command[SERVICE_EXEC_START]) { + LIST_FOREACH(command, c, s->exec_command[ci]) { _cleanup_free_ char *a = NULL, *t = NULL; const char *p; @@ -326,7 +327,8 @@ static int bus_service_set_transient_property( if (!a) return -ENOMEM; - fprintf(f, "ExecStart=%s@%s %s\n", + fprintf(f, "%s=%s@%s %s\n", + name, c->flags & EXEC_COMMAND_IGNORE_FAILURE ? "-" : "", p, a); diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 78b9a68348..5e88866dc7 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -1205,6 +1205,94 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen return r; r = sd_bus_message_close_container(m); + + } else if (STR_IN_SET(field, "ExecStartPre", "ExecStart", "ExecStartPost", + "ExecReload", "ExecStop", "ExecStopPost")) { + + bool ignore_failure = false, explicit_path = false, done = false; + _cleanup_strv_free_ char **l = NULL; + _cleanup_free_ char *path = NULL; + + do { + switch (*eq) { + + case '-': + if (ignore_failure) + done = true; + else { + ignore_failure = true; + eq++; + } + break; + + case '@': + if (explicit_path) + done = true; + else { + explicit_path = true; + eq++; + } + break; + + case '+': + case '!': + /* The bus API doesn't support +, ! and !! currently, unfortunately. :-( */ + log_error("Sorry, but +, ! and !! are currently not supported for transient services."); + return -EOPNOTSUPP; + + default: + done = true; + break; + } + } while(!done); + + if (explicit_path) { + r = extract_first_word(&eq, &path, NULL, EXTRACT_QUOTES|EXTRACT_CUNESCAPE); + if (r < 0) + return log_error_errno(r, "Failed to parse path: %m"); + } + + r = strv_split_extract(&l, eq, NULL, EXTRACT_QUOTES|EXTRACT_CUNESCAPE); + if (r < 0) + return log_error_errno(r, "Failed to parse command line: %m"); + + r = sd_bus_message_open_container(m, 'v', "a(sasb)"); + if (r < 0) + return r; + + r = sd_bus_message_open_container(m, 'a', "(sasb)"); + if (r < 0) + return r; + + if (strv_length(l) > 0) { + + r = sd_bus_message_open_container(m, 'r', "sasb"); + if (r < 0) + return r; + + r = sd_bus_message_append(m, "s", path ?: l[0]); + if (r < 0) + return r; + + r = sd_bus_message_append_strv(m, l); + if (r < 0) + return r; + + r = sd_bus_message_append(m, "b", ignore_failure); + if (r < 0) + return r; + + r = sd_bus_message_close_container(m); + if (r < 0) + return r; + } + + r = sd_bus_message_close_container(m); + if (r < 0) + return r; + + r = sd_bus_message_close_container(m); + } else { log_error("Unknown assignment %s.", assignment); return -EINVAL; From 91d2f19594d5b0aa5cf011a3dba6dd706696764b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Nov 2017 20:26:02 +0100 Subject: [PATCH 20/33] documentation: add document listing all unit file settings also supported for transient units Unfortunately, there are far too many properties currently not accessible to transient units. The first step to opening them all up is to figure out which ones they are. --- TRANSIENT-SETTINGS.md | 447 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 447 insertions(+) create mode 100644 TRANSIENT-SETTINGS.md diff --git a/TRANSIENT-SETTINGS.md b/TRANSIENT-SETTINGS.md new file mode 100644 index 0000000000..17fe0604ec --- /dev/null +++ b/TRANSIENT-SETTINGS.md @@ -0,0 +1,447 @@ +# What settings are currently available for transient units? + +Our intention is to make all settings that are available as unit file settings +also available for transient units, through the D-Bus API. At the moment, some +unit types (socket, swap, path) are not supported at all via unit types, but +most others are pretty well supported, with some notable omissions. + +The lists below contain all settings currently available in unit files. The +ones currently available in transient units are prefixed with `✓`. + +## Generic Unit Settings + +Only the most important generic unit settings are available for transient units. + +``` +✓ Description= + Documentation= + SourcePath= +✓ Requires= +✓ Requisite= +✓ Wants= +✓ BindsTo= +✓ Conflicts= +✓ Before= +✓ After= +✓ OnFailure= +✓ PropagatesReloadTo= +✓ ReloadPropagatedFrom= +✓ PartOf= + JoinsNamespaceOf= + RequiresMountsFor= + StopWhenUnneeded= + RefuseManualStart= + RefuseManualStop= + AllowIsolate= +✓ DefaultDependencies= + OnFailureJobMode= + OnFailureIsolate= + IgnoreOnIsolate= + JobTimeoutSec= + JobRunningTimeoutSec= + JobTimeoutAction= + JobTimeoutRebootArgument= + StartLimitIntervalSec=SECONDS + StartLimitBurst=UNSIGNED + StartLimitAction=ACTION +✓ FailureAction= +✓ SuccessAction= +✓ AddRef= + RebootArgument=STRING + ConditionPathExists= + ConditionPathExistsGlob= + ConditionPathIsDirectory= + ConditionPathIsSymbolicLink= + ConditionPathIsMountPoint= + ConditionPathIsReadWrite= + ConditionDirectoryNotEmpty= + ConditionFileNotEmpty= + ConditionFileIsExecutable= + ConditionNeedsUpdate= + ConditionFirstBoot= + ConditionKernelCommandLine= + ConditionArchitecture= + ConditionVirtualization= + ConditionSecurity= + ConditionCapability= + ConditionHost= + ConditionACPower= + ConditionUser= + ConditionGroup= + AssertPathExists= + AssertPathExistsGlob= + AssertPathIsDirectory= + AssertPathIsSymbolicLink= + AssertPathIsMountPoint= + AssertPathIsReadWrite= + AssertDirectoryNotEmpty= + AssertFileNotEmpty= + AssertFileIsExecutable= + AssertNeedsUpdate= + AssertFirstBoot= + AssertKernelCommandLine= + AssertArchitecture= + AssertVirtualization= + AssertSecurity= + AssertCapability= + AssertHost= + AssertACPower= + AssertUser= + AssertGroup= +✓ CollectMode= +``` + +## Execution-Related Settings + +All execution-related settings are available for transient units. + +``` +✓ WorkingDirectory= +✓ RootDirectory= +✓ RootImage= +✓ User= +✓ Group= +✓ SupplementaryGroups= +✓ Nice= +✓ OOMScoreAdjust= +✓ IOSchedulingClass= +✓ IOSchedulingPriority= +✓ CPUSchedulingPolicy= +✓ CPUSchedulingPriority= +✓ CPUSchedulingResetOnFork= +✓ CPUAffinity= +✓ UMask= +✓ Environment= +✓ EnvironmentFile= +✓ PassEnvironment= +✓ UnsetEnvironment= +✓ DynamicUser= +✓ RemoveIPC= +✓ StandardInput= +✓ StandardOutput= +✓ StandardError= +✓ StandardInputText= +✓ StandardInputData= +✓ TTYPath= +✓ TTYReset= +✓ TTYVHangup= +✓ TTYVTDisallocate= +✓ SyslogIdentifier= +✓ SyslogFacility= +✓ SyslogLevel= +✓ SyslogLevelPrefix= +✓ LogLevelMax= +✓ LogExtraFields= +✓ SecureBits= +✓ CapabilityBoundingSet= +✓ AmbientCapabilities= +✓ TimerSlackNSec= +✓ NoNewPrivileges= +✓ KeyringMode= +✓ SystemCallFilter= +✓ SystemCallArchitectures= +✓ SystemCallErrorNumber= +✓ MemoryDenyWriteExecute= +✓ RestrictNamespaces= +✓ RestrictRealtime= +✓ RestrictAddressFamilies= +✓ LockPersonality= +✓ LimitCPU= +✓ LimitFSIZE= +✓ LimitDATA= +✓ LimitSTACK= +✓ LimitCORE= +✓ LimitRSS= +✓ LimitNOFILE= +✓ LimitAS= +✓ LimitNPROC= +✓ LimitMEMLOCK= +✓ LimitLOCKS= +✓ LimitSIGPENDING= +✓ LimitMSGQUEUE= +✓ LimitNICE= +✓ LimitRTPRIO= +✓ LimitRTTIME= +✓ ReadWritePaths= +✓ ReadOnlyPaths= +✓ InaccessiblePaths= +✓ BindPaths= +✓ BindReadOnlyPaths= +✓ PrivateTmp= +✓ PrivateDevices= +✓ ProtectKernelTunables= +✓ ProtectKernelModules= +✓ ProtectControlGroups= +✓ PrivateNetwork= +✓ PrivateUsers= +✓ ProtectSystem= +✓ ProtectHome= +✓ MountFlags= +✓ MountAPIVFS= +✓ Personality= +✓ RuntimeDirectoryPreserve= +✓ RuntimeDirectoryMode= +✓ RuntimeDirectory= +✓ StateDirectoryMode= +✓ StateDirectory= +✓ CacheDirectoryMode= +✓ CacheDirectory= +✓ LogsDirectoryMode= +✓ LogsDirectory= +✓ ConfigurationDirectoryMode= +✓ ConfigurationDirectory= +✓ PAMName= +✓ IgnoreSIGPIPE= +✓ UtmpIdentifier= +✓ UtmpMode= +✓ SELinuxContext= +✓ SmackProcessLabel= +✓ AppArmorProfile= +✓ Slice= +``` + +## Resource Control Settings + +All cgroup/resource control settings are available for transient units + +``` +✓ CPUAccounting= +✓ CPUWeight= +✓ StartupCPUWeight= +✓ CPUShares= +✓ StartupCPUShares= +✓ CPUQuota= +✓ MemoryAccounting= +✓ MemoryLow= +✓ MemoryHigh= +✓ MemoryMax= +✓ MemorySwapMax= +✓ MemoryLimit= +✓ DeviceAllow= +✓ DevicePolicy= +✓ IOAccounting= +✓ IOWeight= +✓ StartupIOWeight= +✓ IODeviceWeight= +✓ IOReadBandwidthMax= +✓ IOWriteBandwidthMax= +✓ IOReadIOPSMax= +✓ IOWriteIOPSMax= +✓ BlockIOAccounting= +✓ BlockIOWeight= +✓ StartupBlockIOWeight= +✓ BlockIODeviceWeight= +✓ BlockIOReadBandwidth= +✓ BlockIOWriteBandwidth= +✓ TasksAccounting= +✓ TasksMax= +✓ Delegate= +✓ IPAccounting= +✓ IPAddressAllow= +✓ IPAddressDeny= +``` + +## Process Killing Settings + +All process killing settings are available for transient units: + +``` +✓ SendSIGKILL= +✓ SendSIGHUP= +✓ KillMode= +✓ KillSignal= +``` + +## Service Unit Settings + +Only the most important service settings are available for transient units. + +``` + PIDFile= +✓ ExecStartPre= +✓ ExecStart= +✓ ExecStartPost= +✓ ExecReload= +✓ ExecStop= +✓ ExecStopPost= + RestartSec= + TimeoutStartSec= + TimeoutStopSec= + TimeoutSec= +✓ RuntimeMaxSec= + WatchdogSec= +✓ Type= +✓ Restart= + PermissionsStartOnly= + RootDirectoryStartOnly= +✓ RemainAfterExit= + GuessMainPID= + RestartPreventExitStatus= + RestartForceExitStatus= + SuccessExitStatus= +✓ NonBlocking= + BusName= +✓ FileDescriptorStoreMax= +✓ NotifyAccess= + Sockets= + USBFunctionDescriptors= + USBFunctionStrings= +``` + +## Mount Unit Settings + +Only the most important mount unit settings are currently available to transient units: + +``` +✓ What= + Where= +✓ Options= +✓ Type= + TimeoutSec= + DirectoryMode= + SloppyOptions= + LazyUnmount= + ForceUnmount= +``` + +## Automount Unit Settings + +Only one automount unit setting is currently available to transient units: + +``` + Where= + DirectoryMode= +✓ TimeoutIdleSec= +``` + +## Timer Unit Settings + +Most timer unit settings are available to transient units. + +``` +✓ OnCalendar= +✓ OnActiveSec= +✓ OnBootSec= +✓ OnStartupSec= +✓ OnUnitActiveSec= +✓ OnUnitInactiveSec= + Persistent= +✓ WakeSystem= +✓ RemainAfterElapse= +✓ AccuracySec= +✓ RandomizedDelaySec= + Unit= +``` + +## Slice Unit Settings + +Slice units are fully supported as transient units, but they have no settings +of their own beyond the generic unit and resource control settings. + +## Scope Unit Settings + +Scope units are fully supported as transient units (in fact they only exist as +such), but they have no settings of their own beyond the generic unit and +resource control settings. + +## Socket Unit Settings + +Socket units are currently not available at all as transient units: + +``` + ListenStream= + ListenDatagram= + ListenSequentialPacket= + ListenFIFO= + ListenNetlink= + ListenSpecial= + ListenMessageQueue= + ListenUSBFunction= + SocketProtocol= + BindIPv6Only= + Backlog= + BindToDevice= + ExecStartPre= + ExecStartPost= + ExecStopPre= + ExecStopPost= + TimeoutSec= + SocketUser= + SocketGroup= + SocketMode= + DirectoryMode= + Accept= + Writable= + MaxConnections= + MaxConnectionsPerSource= + KeepAlive= + KeepAliveTimeSec= + KeepAliveIntervalSec= + KeepAliveProbes= + DeferAcceptSec= + NoDelay= + Priority= + ReceiveBuffer= + SendBuffer= + IPTOS= + IPTTL= + Mark= + PipeSize= + FreeBind= + Transparent= + Broadcast= + PassCredentials= + PassSecurity= + TCPCongestion= + ReusePort= + MessageQueueMaxMessages= + MessageQueueMessageSize= + RemoveOnStop= + Symlinks= + FileDescriptorName= + Service= + TriggerLimitIntervalSec= + TriggerLimitBurst= + SmackLabel= + SmackLabelIPIn= + SmackLabelIPOut= + SELinuxContextFromNet= +``` + +## Swap Unit Settings + +Swap units are currently not available at all as transient units: + +``` + What= + Priority= + Options= + TimeoutSec= +``` + +## Path Unit Settings + +Path units are currently not available at all as transient units: + +``` + PathExists= + PathExistsGlob= + PathChanged= + PathModified= + DirectoryNotEmpty= + Unit= + MakeDirectory= + DirectoryMode= +``` + +## Install Section + +The `[Install]` section is currently not available at all for transient units, and it probably doesn't even make sense. + +``` + Alias= + WantedBy= + RequiredBy= + Also= + DefaultInstance= +``` From 7d7a99ac9f2c4b06a11fbe3138e2bb24011c3b89 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 Nov 2017 16:06:39 +0100 Subject: [PATCH 21/33] fileio: document why fileio-label.c and fileio.c are two different modules --- src/basic/fileio-label.h | 4 ++++ src/basic/fileio.h | 13 +++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/basic/fileio-label.h b/src/basic/fileio-label.h index 630ee55bec..0adb895236 100644 --- a/src/basic/fileio-label.h +++ b/src/basic/fileio-label.h @@ -25,6 +25,10 @@ #include "fileio.h" +/* These functions are split out of fileio.h (and not for examplement just as flags to the functions they wrap) in + * order to optimize linking: This way, -lselinux is needed only for the callers of these functions that need selinux, + * but not for all */ + int write_string_file_atomic_label_ts(const char *fn, const char *line, struct timespec *ts); static inline int write_string_file_atomic_label(const char *fn, const char *line) { return write_string_file_atomic_label_ts(fn, line, NULL); diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 3d785eae75..c283f41f09 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -30,11 +30,16 @@ #include "time-util.h" typedef enum { - WRITE_STRING_FILE_CREATE = 1<<0, - WRITE_STRING_FILE_ATOMIC = 1<<1, - WRITE_STRING_FILE_AVOID_NEWLINE = 1<<2, + WRITE_STRING_FILE_CREATE = 1<<0, + WRITE_STRING_FILE_ATOMIC = 1<<1, + WRITE_STRING_FILE_AVOID_NEWLINE = 1<<2, WRITE_STRING_FILE_VERIFY_ON_FAILURE = 1<<3, - WRITE_STRING_FILE_SYNC = 1<<4, + WRITE_STRING_FILE_SYNC = 1<<4, + + /* And before you wonder, why write_string_file_atomic_label_ts() is a separate function instead of just one + more flag here: it's about linking: we don't want to pull -lselinux into all users of write_string_file() + and friends. */ + } WriteStringFileFlags; int write_string_stream_ts(FILE *f, const char *line, WriteStringFileFlags flags, struct timespec *ts); From 50fb00b707ab54e498e7624c4a5d0d8cc2c2e7e3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Nov 2017 21:24:20 +0100 Subject: [PATCH 22/33] core: use safe_fclose() where we can --- src/core/unit.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index a12d24b90b..1bbe5fdded 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -571,8 +571,7 @@ void unit_free(Unit *u) { if (!u) return; - if (u->transient_file) - fclose(u->transient_file); + u->transient_file = safe_fclose(u->transient_file); if (!MANAGER_IS_RELOADING(u->manager)) unit_remove_transient(u); @@ -1505,9 +1504,7 @@ int unit_load(Unit *u) { if (r < 0) goto fail; - fclose(u->transient_file); - u->transient_file = NULL; - + u->transient_file = safe_fclose(u->transient_file); u->fragment_mtime = now(CLOCK_REALTIME); } From 7ae8162f29f3ba70c8a4f343ffdb6e77122ce625 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 28 Nov 2017 21:25:49 +0100 Subject: [PATCH 23/33] tests: fix description of TEST-14-MACHINE-ID It used the same description as TEST-01, and that's hardly helpful. --- test/TEST-14-MACHINE-ID/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/TEST-14-MACHINE-ID/test.sh b/test/TEST-14-MACHINE-ID/test.sh index b932060bc2..7342645bc5 100755 --- a/test/TEST-14-MACHINE-ID/test.sh +++ b/test/TEST-14-MACHINE-ID/test.sh @@ -2,7 +2,7 @@ # -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*- # ex: ts=8 sw=4 sts=4 et filetype=sh set -e -TEST_DESCRIPTION="Basic systemd setup" +TEST_DESCRIPTION="/etc/machine-id testing" TEST_NO_NSPAWN=1 SKIP_INITRD=yes . $TEST_BASE_DIR/test-functions From 5716965132bfad3c897b11799e478820a9587a45 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Nov 2017 11:33:26 +0100 Subject: [PATCH 24/33] bus-unit-util: properly serialize EnvironmentFile= empty assignment In that case we need to generate an empty array. --- src/shared/bus-unit-util.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 5e88866dc7..0cc3f991b3 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -134,9 +134,12 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen } else if (streq(field, "EnvironmentFile")) { - r = sd_bus_message_append(m, "sv", "EnvironmentFiles", "a(sb)", 1, - eq[0] == '-' ? eq + 1 : eq, - eq[0] == '-'); + if (isempty(eq)) + r = sd_bus_message_append(m, "sv", "EnvironmentFiles", "a(sb)", 0); + else + r = sd_bus_message_append(m, "sv", "EnvironmentFiles", "a(sb)", 1, + eq[0] == '-' ? eq + 1 : eq, + eq[0] == '-'); goto finish; } else if (STR_IN_SET(field, "AccuracySec", "RandomizedDelaySec", "RuntimeMaxSec")) { From 7b943bb7e3e2fad3481c7191edbfaeef0680b196 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Nov 2017 12:40:22 +0100 Subject: [PATCH 25/33] tree-wide: use strv_isempty() instead of strv_length() == 0 It's a lot faster in many cases, since it's O(1) rather than O(n). --- coccinelle/isempty.cocci | 15 +++++++++++++++ src/core/dbus-execute.c | 14 +++++++------- src/test/test-path-lookup.c | 2 +- src/tmpfiles/tmpfiles.c | 2 +- src/udev/collect/collect.c | 2 +- 5 files changed, 25 insertions(+), 10 deletions(-) create mode 100644 coccinelle/isempty.cocci diff --git a/coccinelle/isempty.cocci b/coccinelle/isempty.cocci new file mode 100644 index 0000000000..1374ee40d7 --- /dev/null +++ b/coccinelle/isempty.cocci @@ -0,0 +1,15 @@ +@@ +expression s; +@@ +- strv_length(s) == 0 ++ strv_isempty(s) +@@ +expression s; +@@ +- strlen(s) == 0 ++ isempty(s) +@@ +expression s; +@@ +- strlen_ptr(s) == 0 ++ isempty(s) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 412be2a6c4..14fb46ec59 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1108,7 +1108,7 @@ int bus_exec_context_set_transient_property( } if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - if (strv_length(l) == 0) { + if (strv_isempty(l)) { c->supplementary_groups = strv_free(c->supplementary_groups); unit_write_settingf(u, flags, name, "%s=", name); } else { @@ -1360,7 +1360,7 @@ int bus_exec_context_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *joined = NULL; - if (strv_length(l) == 0) { + if (strv_isempty(l)) { c->syscall_whitelist = false; c->syscall_filter = hashmap_free(c->syscall_filter); } else { @@ -1433,7 +1433,7 @@ int bus_exec_context_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *joined = NULL; - if (strv_length(l) == 0) + if (strv_isempty(l)) c->syscall_archs = set_free(c->syscall_archs); else { char **s; @@ -1506,7 +1506,7 @@ int bus_exec_context_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *joined = NULL; - if (strv_length(l) == 0) { + if (strv_isempty(l)) { c->address_families_whitelist = false; c->address_families = set_free(c->address_families); } else { @@ -2073,7 +2073,7 @@ int bus_exec_context_set_transient_property( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid environment block."); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - if (strv_length(l) == 0) { + if (strv_isempty(l)) { c->environment = strv_free(c->environment); unit_write_setting(u, flags, name, "Environment="); } else { @@ -2109,7 +2109,7 @@ int bus_exec_context_set_transient_property( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid UnsetEnvironment= list."); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - if (strv_length(l) == 0) { + if (strv_isempty(l)) { c->unset_environment = strv_free(c->unset_environment); unit_write_setting(u, flags, name, "UnsetEnvironment="); } else { @@ -2316,7 +2316,7 @@ int bus_exec_context_set_transient_property( else /* "InaccessiblePaths" */ dirs = &c->inaccessible_paths; - if (strv_length(l) == 0) { + if (strv_isempty(l)) { *dirs = strv_free(*dirs); unit_write_settingf(u, flags, name, "%s=", name); } else { diff --git a/src/test/test-path-lookup.c b/src/test/test-path-lookup.c index 94e5d62e64..834b061980 100644 --- a/src/test/test-path-lookup.c +++ b/src/test/test-path-lookup.c @@ -47,7 +47,7 @@ static void test_paths(UnitFileScope scope) { assert_se(strv_length(lp_with_env.search_path) == 1); assert_se(streq(lp_with_env.search_path[0], systemd_unit_path)); assert_se(lookup_paths_reduce(&lp_with_env) >= 0); - assert_se(strv_length(lp_with_env.search_path) == 0); + assert_se(strv_isempty(lp_with_env.search_path)); assert_se(rm_rf(template, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0); } diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index ad89b46f01..c29087b9d0 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1788,7 +1788,7 @@ static bool should_include_path(const char *path) { /* no matches, so we should include this path only if we * have no whitelist at all */ - if (strv_length(arg_include_prefixes) == 0) + if (strv_isempty(arg_include_prefixes)) return true; log_debug("Entry \"%s\" does not match any include prefix, skipping.", path); diff --git a/src/udev/collect/collect.c b/src/udev/collect/collect.c index 14e97e6f48..3e278bd637 100644 --- a/src/udev/collect/collect.c +++ b/src/udev/collect/collect.c @@ -162,7 +162,7 @@ static int checkout(int fd) if (ptr) { *ptr = '\0'; ptr++; - if (!strlen(word)) + if (isempty(word)) continue; if (debug) From a2d61f07463410e8ef7127b6e7c194d2bb04247a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Nov 2017 13:05:24 +0100 Subject: [PATCH 26/33] coccinelle: fix IN_SET/!IN_SET scripts, and apply some changes it found IN_SET only works for constant values, hence clarify that. Moreover, we declared a statement "s" we never made use of. Drop it. Also, for both scripts, let's support 10 items. More causes spatch to die with "Stack overflow" for me. --- coccinelle/in_set.cocci | 59 +++++++++++------ coccinelle/not_in_set.cocci | 127 +++++------------------------------- src/udev/udevd.c | 2 +- 3 files changed, 57 insertions(+), 131 deletions(-) diff --git a/coccinelle/in_set.cocci b/coccinelle/in_set.cocci index f5ddd3d334..12d5475fd9 100644 --- a/coccinelle/in_set.cocci +++ b/coccinelle/in_set.cocci @@ -1,35 +1,54 @@ @@ expression e; -identifier n1, n2, n3, n4, n5, n6; -statement s; +constant n0, n1, n2, n3, n4, n5, n6, n7, n8, n9; @@ -- e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 -+ IN_SET(e, n1, n2, n3, n4, n5, n6) +- e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 || e == n7 || e == n8 || e == n9 ++ IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9) @@ expression e; -identifier n1, n2, n3, n4, n5; -statement s; +constant n0, n1, n2, n3, n4, n5, n6, n7, n8; @@ -- e == n1 || e == n2 || e == n3 || e == n4 || e == n5 -+ IN_SET(e, n1, n2, n3, n4, n5) +- e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 || e == n7 || e == n8 ++ IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8) @@ expression e; -identifier n1, n2, n3, n4; -statement s; +constant n0, n1, n2, n3, n4, n5, n6, n7; @@ -- e == n1 || e == n2 || e == n3 || e == n4 -+ IN_SET(e, n1, n2, n3, n4) +- e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 || e == n7 ++ IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7) @@ expression e; -identifier n1, n2, n3; -statement s; +constant n0, n1, n2, n3, n4, n5, n6; @@ -- e == n1 || e == n2 || e == n3 -+ IN_SET(e, n1, n2, n3) +- e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 ++ IN_SET(e, n0, n1, n2, n3, n4, n5, n6) @@ expression e; -identifier n, p; -statement s; +constant n0, n1, n2, n3, n4, n5; @@ -- e == n || e == p -+ IN_SET(e, n, p) +- e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 ++ IN_SET(e, n0, n1, n2, n3, n4, n5) +@@ +expression e; +constant n0, n1, n2, n3, n4; +@@ +- e == n0 || e == n1 || e == n2 || e == n3 || e == n4 ++ IN_SET(e, n0, n1, n2, n3, n4) +@@ +expression e; +constant n0, n1, n2, n3; +@@ +- e == n0 || e == n1 || e == n2 || e == n3 ++ IN_SET(e, n0, n1, n2, n3) +@@ +expression e; +constant n0, n1, n2; +@@ +- e == n0 || e == n1 || e == n2 ++ IN_SET(e, n0, n1, n2) +@@ +expression e; +constant n0, n1; +@@ +- e == n0 || e == n1 ++ IN_SET(e, n0, n1) diff --git a/coccinelle/not_in_set.cocci b/coccinelle/not_in_set.cocci index e6bcf47893..7cf98500cd 100644 --- a/coccinelle/not_in_set.cocci +++ b/coccinelle/not_in_set.cocci @@ -1,147 +1,54 @@ @@ expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15, n16, n17, n18, n19, n20; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 && e != n11 && e != n12 && e != n13 && e != n14 && e != n15 && e != n16 && e != n17 && e != n18 && e != n19 && e != n20 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15, n16, n17, n18, n19, n20) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15, n16, n17, n18, n19; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 && e != n11 && e != n12 && e != n13 && e != n14 && e != n15 && e != n16 && e != n17 && e != n18 && e != n19 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15, n16, n17, n18, n19) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15, n16, n17, n18; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 && e != n11 && e != n12 && e != n13 && e != n14 && e != n15 && e != n16 && e != n17 && e != n18 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15, n16, n17, n18) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15, n16, n17; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 && e != n11 && e != n12 && e != n13 && e != n14 && e != n15 && e != n16 && e != n17 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15, n16, n17) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15, n16; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 && e != n11 && e != n12 && e != n13 && e != n14 && e != n15 && e != n16 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15, n16) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 && e != n11 && e != n12 && e != n13 && e != n14 && e != n15 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14, n15) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 && e != n11 && e != n12 && e != n13 && e != n14 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 && e != n11 && e != n12 && e != n13 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 && e != n11 && e != n12 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 && e != n11 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10; -statement s; -@@ -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 && e != n10 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10) -@@ -expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8, n9; -statement s; +constant n0, n1, n2, n3, n4, n5, n6, n7, n8, n9; @@ - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9) @@ expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7, n8; -statement s; +constant n0, n1, n2, n3, n4, n5, n6, n7, n8; @@ - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8) @@ expression e; -identifier n0, n1, n2, n3, n4, n5, n6, n7; -statement s; +constant n0, n1, n2, n3, n4, n5, n6, n7; @@ - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7) @@ expression e; -identifier n0, n1, n2, n3, n4, n5, n6; -statement s; +constant n0, n1, n2, n3, n4, n5, n6; @@ - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6) @@ expression e; -identifier n0, n1, n2, n3, n4, n5; -statement s; +constant n0, n1, n2, n3, n4, n5; @@ - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 + !IN_SET(e, n0, n1, n2, n3, n4, n5) @@ expression e; -identifier n1, n2, n3, n4, n5; -statement s; +constant n0, n1, n2, n3, n4; @@ -- e != n1 && e != n2 && e != n3 && e != n4 && e != n5 -+ !IN_SET(e, n1, n2, n3, n4, n5) +- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 ++ !IN_SET(e, n0, n1, n2, n3, n4) @@ expression e; -identifier n1, n2, n3, n4; -statement s; +constant n0, n1, n2, n3; @@ -- e != n1 && e != n2 && e != n3 && e != n4 -+ !IN_SET(e, n1, n2, n3, n4) +- e != n0 && e != n1 && e != n2 && e != n3 ++ !IN_SET(e, n0, n1, n2, n3) @@ expression e; -identifier n1, n2, n3, n4; -statement s; +constant n0, n1, n2; @@ -- e != n1 && e != n2 && e != n3 && e != n4 -+ !IN_SET(e, n1, n2, n3, n4) +- e != n0 && e != n1 && e != n2 ++ !IN_SET(e, n0, n1, n2) @@ expression e; -identifier n1, n2, n3; -statement s; +constant n0, n1; @@ -- e != n1 && e != n2 && e != n3 -+ !IN_SET(e, n1, n2, n3) -@@ -expression e; -identifier n, p; -statement s; -@@ -- e != n && e != p -+ !IN_SET(e, n, p) +- e != n0 && e != n1 ++ !IN_SET(e, n0, n1) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index be76857454..e2c7f99cd4 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1715,7 +1715,7 @@ int main(int argc, char *argv[]) { by PID1. otherwise we are not guaranteed to have a dedicated cgroup */ r = cg_pid_get_path(SYSTEMD_CGROUP_CONTROLLER, 0, &cgroup); if (r < 0) { - if (r == -ENOENT || r == -ENOMEDIUM) + if (IN_SET(r, -ENOENT, -ENOMEDIUM)) log_debug_errno(r, "did not find dedicated cgroup: %m"); else log_warning_errno(r, "failed to get cgroup: %m"); From 2d0bc6845005e222b8b48c31fdeb8b56a6590f18 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Nov 2017 19:49:05 +0100 Subject: [PATCH 27/33] coccinelle: add a run-coccinelle.sh script that runs all scripts One day we should start running something like this as part of CI so that non-well-formed commits are not even accepted... --- coccinelle/run-coccinelle.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100755 coccinelle/run-coccinelle.sh diff --git a/coccinelle/run-coccinelle.sh b/coccinelle/run-coccinelle.sh new file mode 100755 index 0000000000..fe3aeb68ce --- /dev/null +++ b/coccinelle/run-coccinelle.sh @@ -0,0 +1,11 @@ +#!/bin/bash -e + +for SCRIPT in *.cocci ; do + [ "$SCRIPT" = "empty-if.cocci" ] && continue + echo "--x-- Processing $SCRIPT --x--" + TMPFILE=`mktemp` + spatch --sp-file $SCRIPT --dir $(pwd)/.. 2> "$TMPFILE" || cat "$TMPFILE" + rm "$TMPFILE" + echo "--x-- Processed $SCRIPT --x--" + echo "" +done From 421eaea0d1c33f638877bf737e5293aaaf3ce636 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Nov 2017 19:53:53 +0100 Subject: [PATCH 28/33] tty-ask-password-agent: drop NULL sentinel found by coccinelle --- src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 3824d2f5b5..1553655a28 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -721,7 +721,7 @@ static int ask_on_this_console(const char *tty, pid_t *pid, int argc, char *argv for (ac = 0; ac < argc; ac++) { if (streq(argv[ac], "--console")) { - argv[ac] = strjoina("--console=", tty, NULL); + argv[ac] = strjoina("--console=", tty); break; } } From 66a892ae3d5ebaa82298c40012b95534ba34cfb1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Nov 2017 19:55:01 +0100 Subject: [PATCH 29/33] core: accept MemorySwapMax= properties that are scaled, too Let's do what we already do for MemoryMax= and friends for MemorySwapMax= too. --- src/core/dbus-cgroup.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 6c999f681f..690e3a3ca2 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -1054,7 +1054,7 @@ int bus_cgroup_set_property( return 1; - } else if (STR_IN_SET(name, "MemoryLowScale", "MemoryHighScale", "MemoryMaxScale")) { + } else if (STR_IN_SET(name, "MemoryLowScale", "MemoryHighScale", "MemoryMaxScale", "MemorySwapMaxScale")) { uint32_t raw; uint64_t v; @@ -1077,7 +1077,9 @@ int bus_cgroup_set_property( c->memory_low = v; else if (streq(name, "MemoryHigh")) c->memory_high = v; - else + else if (streq(name, "MemorySwapMaxScale")) + c->memory_swap_max = v; + else /* MemoryMax */ c->memory_max = v; unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); @@ -1100,7 +1102,7 @@ int bus_cgroup_set_property( c->memory_limit = limit; unit_invalidate_cgroup(u, CGROUP_MASK_MEMORY); - if (limit == (uint64_t) -1) + if (limit == CGROUP_LIMIT_MAX) unit_write_setting(u, flags, name, "MemoryLimit=infinity"); else unit_write_settingf(u, flags, name, "MemoryLimit=%" PRIu64, limit); From 147fe484c178b6e4d60ee7c6982345dd51bf434d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Nov 2017 19:55:42 +0100 Subject: [PATCH 30/33] bus-unit-util: add proper MemorySwapMax= serialization Fixes: #7505 --- src/shared/bus-unit-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 0cc3f991b3..bd8a8ed344 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -186,7 +186,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_close_container(m); goto finish; - } else if (STR_IN_SET(field, "MemoryLow", "MemoryHigh", "MemoryMax", "MemoryLimit")) { + } else if (STR_IN_SET(field, "MemoryLow", "MemoryHigh", "MemoryMax", "MemorySwapMax", "MemoryLimit")) { uint64_t bytes; if (isempty(eq) || streq(eq, "infinity")) From f53bddf30879a3db9fb61b42b16ab6a3e280389e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Nov 2017 20:05:22 +0100 Subject: [PATCH 31/33] run: rearrange error handling when adding unit properties Let's always leave logging to the call that actually added the fields to the bus message. This way we don't get duplicate logging whenver bus_append_unit_property_assignment() ends up being called, which does all its logging on its own (and probably should do, as it can output much more precise errors). --- src/run/run.c | 101 ++++++++++++++++++++++++++------------------------ 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index f888fd96b1..db833c2903 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -467,12 +467,12 @@ static int transient_unit_set_properties(sd_bus_message *m, char **properties) { r = sd_bus_message_append(m, "(sv)", "Description", "s", arg_description); if (r < 0) - return r; + return bus_log_create_error(r); if (arg_aggressive_gc) { r = sd_bus_message_append(m, "(sv)", "CollectMode", "s", "inactive-or-failed"); if (r < 0) - return r; + return bus_log_create_error(r); } r = bus_append_unit_property_assignment_many(m, properties); @@ -487,27 +487,32 @@ static int transient_cgroup_set_properties(sd_bus_message *m) { assert(m); if (!isempty(arg_slice)) { - _cleanup_free_ char *slice; + _cleanup_free_ char *slice = NULL; r = unit_name_mangle_with_suffix(arg_slice, UNIT_NAME_NOGLOB, ".slice", &slice); if (r < 0) - return r; + return log_error_errno(r, "Failed to mangle name '%s': %m", arg_slice); r = sd_bus_message_append(m, "(sv)", "Slice", "s", slice); if (r < 0) - return r; + return bus_log_create_error(r); } return 0; } static int transient_kill_set_properties(sd_bus_message *m) { + int r; + assert(m); - if (arg_send_sighup) - return sd_bus_message_append(m, "(sv)", "SendSIGHUP", "b", arg_send_sighup); - else - return 0; + if (arg_send_sighup) { + r = sd_bus_message_append(m, "(sv)", "SendSIGHUP", "b", arg_send_sighup); + if (r < 0) + return bus_log_create_error(r); + } + + return 0; } static int transient_service_set_properties(sd_bus_message *m, char **argv, const char *pty_path) { @@ -531,37 +536,37 @@ static int transient_service_set_properties(sd_bus_message *m, char **argv, cons if (arg_wait || arg_stdio != ARG_STDIO_NONE) { r = sd_bus_message_append(m, "(sv)", "AddRef", "b", 1); if (r < 0) - return r; + return bus_log_create_error(r); } if (arg_remain_after_exit) { r = sd_bus_message_append(m, "(sv)", "RemainAfterExit", "b", arg_remain_after_exit); if (r < 0) - return r; + return bus_log_create_error(r); } if (arg_service_type) { r = sd_bus_message_append(m, "(sv)", "Type", "s", arg_service_type); if (r < 0) - return r; + return bus_log_create_error(r); } if (arg_exec_user) { r = sd_bus_message_append(m, "(sv)", "User", "s", arg_exec_user); if (r < 0) - return r; + return bus_log_create_error(r); } if (arg_exec_group) { r = sd_bus_message_append(m, "(sv)", "Group", "s", arg_exec_group); if (r < 0) - return r; + return bus_log_create_error(r); } if (arg_nice_set) { r = sd_bus_message_append(m, "(sv)", "Nice", "i", arg_nice); if (r < 0) - return r; + return bus_log_create_error(r); } if (pty_path) { @@ -572,7 +577,7 @@ static int transient_service_set_properties(sd_bus_message *m, char **argv, cons "StandardError", "s", "tty", "TTYPath", "s", pty_path); if (r < 0) - return r; + return bus_log_create_error(r); send_term = true; @@ -583,7 +588,7 @@ static int transient_service_set_properties(sd_bus_message *m, char **argv, cons "StandardOutputFileDescriptor", "h", STDOUT_FILENO, "StandardErrorFileDescriptor", "h", STDERR_FILENO); if (r < 0) - return r; + return bus_log_create_error(r); send_term = isatty(STDIN_FILENO) || isatty(STDOUT_FILENO) || isatty(STDERR_FILENO); } @@ -600,85 +605,85 @@ static int transient_service_set_properties(sd_bus_message *m, char **argv, cons "(sv)", "Environment", "as", 1, n); if (r < 0) - return r; + return bus_log_create_error(r); } } if (!strv_isempty(arg_environment)) { r = sd_bus_message_open_container(m, 'r', "sv"); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_append(m, "s", "Environment"); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_open_container(m, 'v', "as"); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_append_strv(m, arg_environment); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_close_container(m); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_close_container(m); if (r < 0) - return r; + return bus_log_create_error(r); } /* Exec container */ { r = sd_bus_message_open_container(m, 'r', "sv"); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_append(m, "s", "ExecStart"); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_open_container(m, 'v', "a(sasb)"); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_open_container(m, 'a', "(sasb)"); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_open_container(m, 'r', "sasb"); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_append(m, "s", argv[0]); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_append_strv(m, argv); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_append(m, "b", false); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_close_container(m); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_close_container(m); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_close_container(m); if (r < 0) - return r; + return bus_log_create_error(r); r = sd_bus_message_close_container(m); if (r < 0) - return r; + return bus_log_create_error(r); } return 0; @@ -703,7 +708,7 @@ static int transient_scope_set_properties(sd_bus_message *m) { r = sd_bus_message_append(m, "(sv)", "PIDs", "au", 1, (uint32_t) getpid_cached()); if (r < 0) - return r; + return bus_log_create_error(r); return 0; } @@ -720,42 +725,42 @@ static int transient_timer_set_properties(sd_bus_message *m) { /* Automatically clean up our transient timers */ r = sd_bus_message_append(m, "(sv)", "RemainAfterElapse", "b", false); if (r < 0) - return r; + return bus_log_create_error(r); if (arg_on_active) { r = sd_bus_message_append(m, "(sv)", "OnActiveSec", "t", arg_on_active); if (r < 0) - return r; + return bus_log_create_error(r); } if (arg_on_boot) { r = sd_bus_message_append(m, "(sv)", "OnBootSec", "t", arg_on_boot); if (r < 0) - return r; + return bus_log_create_error(r); } if (arg_on_startup) { r = sd_bus_message_append(m, "(sv)", "OnStartupSec", "t", arg_on_startup); if (r < 0) - return r; + return bus_log_create_error(r); } if (arg_on_unit_active) { r = sd_bus_message_append(m, "(sv)", "OnUnitActiveSec", "t", arg_on_unit_active); if (r < 0) - return r; + return bus_log_create_error(r); } if (arg_on_unit_inactive) { r = sd_bus_message_append(m, "(sv)", "OnUnitInactiveSec", "t", arg_on_unit_inactive); if (r < 0) - return r; + return bus_log_create_error(r); } if (arg_on_calendar) { r = sd_bus_message_append(m, "(sv)", "OnCalendar", "s", arg_on_calendar); if (r < 0) - return r; + return bus_log_create_error(r); } return 0; @@ -1018,7 +1023,7 @@ static int start_transient_service( r = transient_service_set_properties(m, argv, pty_path); if (r < 0) - return bus_log_create_error(r); + return r; r = sd_bus_message_close_container(m); if (r < 0) @@ -1217,7 +1222,7 @@ static int start_transient_scope( r = transient_scope_set_properties(m); if (r < 0) - return bus_log_create_error(r); + return r; r = sd_bus_message_close_container(m); if (r < 0) @@ -1398,7 +1403,7 @@ static int start_transient_timer( r = transient_timer_set_properties(m); if (r < 0) - return bus_log_create_error(r); + return r; r = sd_bus_message_close_container(m); if (r < 0) @@ -1423,7 +1428,7 @@ static int start_transient_timer( r = transient_service_set_properties(m, argv, NULL); if (r < 0) - return bus_log_create_error(r); + return r; r = sd_bus_message_close_container(m); if (r < 0) From c2288ade1e9eb9e3ec44937ea9de94f5df9403b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 29 Nov 2017 20:07:08 +0100 Subject: [PATCH 32/33] bus-unit-util: slightly tweak log output for unknown props Pretty: Unknown assignment Foo=bar. Prettier: Unknown assignment: Foo=bar --- src/shared/bus-unit-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index bd8a8ed344..8a07318455 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -1297,7 +1297,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_close_container(m); } else { - log_error("Unknown assignment %s.", assignment); + log_error("Unknown assignment: %s", assignment); return -EINVAL; } From c6009ff0290183d8882cc27e174d08a49f85a088 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 22 Nov 2017 13:56:10 +0100 Subject: [PATCH 33/33] update TODO --- TODO | 37 ++++--------------------------------- 1 file changed, 4 insertions(+), 33 deletions(-) diff --git a/TODO b/TODO index f7ed2627ba..42bc11703a 100644 --- a/TODO +++ b/TODO @@ -30,6 +30,10 @@ Features: systemd-journald writes to /var/log/journal, which could be useful when we doing disk usage calculations and so on. +* taint systemd if the overflowuid/overflowgid is not 65534 + +* deprecate PermissionsStartOnly= and RootDirectoryStartOnly= in favour of the ExecStart= prefix chars + * add a new RuntimeDirectoryPreserve= mode that defines a similar lifecycle for the runtime dir as we maintain for the fdstore: i.e. keep it around as long as the unit is running or has a job queued. @@ -67,9 +71,6 @@ Features: suitable for processing with rrdtool. Add bus API to access this data, and possibly implement a CPULoad property based on it. -* In journalctl add a way how "-o verbose" and suchlike can be tweaked to show - only a specific set of properties - * beef up pam_systemd to take unit file settings such as cgroups properties as parameters @@ -116,12 +117,6 @@ Features: taken if multiple dirs are configured. Maybe avoid setting the env vars in that case? -* introduce SuccessAction= that permits shutting down the system when a service - succeeds. This is useful to replace "ExecPost=/usr/bin/systemctl poweroff" and - similar constructs, which are frequently used. This is particularly nice for - implementation of a systemd.run= kernel command line option that runs some - command and immediately shuts down. - * expose IO accounting data on the bus, show it in systemd-run --wait and log about it in the resource log message @@ -145,10 +140,6 @@ Features: ReadWritePaths=:/var/lib/foobar -* sort generated hwdb files alphabetically when we import them, so that git - diffs remain minimal (in particular: the OUI databases we import are not - sorted, and not stable) - * maybe add call sd_journal_set_block_timeout() or so to set SO_SNDTIMEO for the sd-journal logging socket, and, if the timeout is set to 0, sets O_NONBLOCK on it. That way people can control if and when to block for @@ -208,17 +199,12 @@ Features: partition, that is mounted to / and is writable, and where the actual root's /usr is mounted into. -* machined: add apis to query /etc/machine-info data of a container - * .mount and .swap units: add Format=yes|no option that formats the partition before mounting/enabling it, implicitly * gpt-auto logic: support encrypted swap, add kernel cmdline option to force it, and honour a gpt bit about it, plus maybe a configuration file * drop nss-myhostname in favour of nss-resolve? -* drop internal dlopen() based nss-dns fallback in nss-resolve, and rely on the - external nsswitch.conf based one - * add a percentage syntax for TimeoutStopSec=, e.g. TimeoutStopSec=150%, and then use that for the setting used in user@.service. It should be understood relative to the configured default value. @@ -229,8 +215,6 @@ Features: * Permit masking specific netlink APIs with RestrictAddressFamily= -* nspawn: start UID allocation loop from hash of container name - * nspawn: support that /proc, /sys/, /dev are pre-mounted * define gpt header bits to select volatility mode @@ -268,8 +252,6 @@ Features: a user/group for a service only has to exist on the host for the right mapping to work. -* allow attaching additional journald log fields to cgroups - * add bus API for creating unit files in /etc, reusing the code for transient units * add bus API to remove unit files from /etc @@ -570,8 +552,6 @@ Features: * shutdown logging: store to EFI var, and store to USB stick? -* think about window-manager-run-as-user-service problem: exit 0 → activate shutdown.target; exit != 0 → restart service - * merge unit_kill_common() and unit_kill_context() * introduce ExecCondition= in services @@ -657,7 +637,6 @@ Features: - journald: when we drop syslog messages because the syslog socket is full, make sure to write how many messages are lost as first thing to syslog when it works again. - - journald: make sure ratelimit is actually really per-service with the new cgroup changes - change systemd-journal-flush into a service that stays around during boot, and causes the journal to be moved back to /run on shutdown, so that we do not keep /var busy. This needs to happen synchronously, @@ -686,7 +665,6 @@ Features: - add journalctl -H that talks via ssh to a remote peer and passes through binary logs data - add a version of --merge which also merges /var/log/journal/remote - - log accumulated resource usage after each service invocation - journalctl: -m should access container journals directly by enumerating them via machined, and also watch containers coming and going. Benefit: nspawn --ephemeral would start working nicely with the journal. @@ -697,7 +675,6 @@ Features: [Install] units of other units, unless those units are disabled - man: clarify that time-sync.target is not only sysv compat but also useful otherwise. Same for similar targets - document that service reload may be implemented as service reexec - - document in wiki how to map ical recurrence events to systemd timer unit calendar specifications - add a man page containing packaging guidelines and recommending usage of things like Documentation=, PrivateTmp=, PrivateNetwork= and ReadOnlyDirectories=/etc /usr. - document systemd-journal-flush.service properly - documentation: recommend to connect the timer units of a service to the service via Also= in [Install] @@ -715,7 +692,6 @@ Features: - add new command to systemctl: "systemctl system-reexec" which reexecs as many daemons as virtually possible - systemctl enable: fail if target to alias into does not exist? maybe show how many units are enabled afterwards? - systemctl: "Journal has been rotated since unit was started." message is misleading - - better error message if you run systemctl without systemd running - systemctl status output should include list of triggering units and their status * unit install: @@ -758,8 +734,6 @@ Features: - should send out sd_notify("WATCHDOG=1") messages - optionally automatically add FORWARD rules to iptables whenever nspawn is running, remove them when shut down. - - Improve error message when --bind= is used on a non-existing source - directory - maybe make copying of /etc/resolv.conf optional, and skip it if --read-only is used @@ -846,7 +820,6 @@ Features: * write blog stories about: - hwdb: what belongs into it, lsusb - enabling dbus services - - status update - how to make changes to sysctl and sysfs attributes - remote access - how to pass throw-away units to systemd, or dynamically change properties of existing units @@ -1001,8 +974,6 @@ Regularly: * check for strerror(r) instead of strerror(-r) -* Use PR_SET_PROCTITLE_AREA if it becomes available in the kernel - * pahole * set_put(), hashmap_put() return values check. i.e. == 0 does not free()!