From 97d5f09bd2ff622575a4b5c0c4c254517a040b13 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 12 Aug 2023 15:20:59 +0800 Subject: [PATCH 1/5] proc-cmdline: introduce PROC_CMDLINE_TRUE_WHEN_MISSING --- src/basic/proc-cmdline.c | 19 +++++++++++-------- src/basic/proc-cmdline.h | 1 + src/test/test-proc-cmdline.c | 1 + 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 95130a36be..c666736627 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -197,9 +197,9 @@ static int proc_cmdline_parse_strv(char **args, proc_cmdline_parse_t parse_item, assert(parse_item); - /* The PROC_CMDLINE_VALUE_OPTIONAL flag doesn't really make sense for proc_cmdline_parse(), let's - * make this clear. */ - assert(!FLAGS_SET(flags, PROC_CMDLINE_VALUE_OPTIONAL)); + /* The PROC_CMDLINE_VALUE_OPTIONAL and PROC_CMDLINE_TRUE_WHEN_MISSING flags don't really make sense + * for proc_cmdline_parse(), let's make this clear. */ + assert(!(flags & (PROC_CMDLINE_VALUE_OPTIONAL|PROC_CMDLINE_TRUE_WHEN_MISSING))); STRV_FOREACH(word, args) { char *key, *value; @@ -351,6 +351,9 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val * * In all three cases, > 0 is returned if the key is found, 0 if not. */ + /* PROC_CMDLINE_TRUE_WHEN_MISSING doesn't really make sense for proc_cmdline_get_key(). */ + assert(!FLAGS_SET(flags, PROC_CMDLINE_TRUE_WHEN_MISSING)); + if (isempty(key)) return -EINVAL; @@ -398,11 +401,11 @@ int proc_cmdline_get_bool(const char *key, ProcCmdlineFlags flags, bool *ret) { assert(ret); - r = proc_cmdline_get_key(key, flags | PROC_CMDLINE_VALUE_OPTIONAL, &v); + r = proc_cmdline_get_key(key, (flags & ~PROC_CMDLINE_TRUE_WHEN_MISSING) | PROC_CMDLINE_VALUE_OPTIONAL, &v); if (r < 0) return r; if (r == 0) { /* key not specified at all */ - *ret = false; + *ret = FLAGS_SET(flags, PROC_CMDLINE_TRUE_WHEN_MISSING); return 0; } @@ -456,9 +459,9 @@ int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { int r, ret = 0; va_list ap; - /* The PROC_CMDLINE_VALUE_OPTIONAL flag doesn't really make sense for proc_cmdline_get_key_many(), let's make - * this clear. */ - assert(!FLAGS_SET(flags, PROC_CMDLINE_VALUE_OPTIONAL)); + /* The PROC_CMDLINE_VALUE_OPTIONAL and PROC_CMDLINE_TRUE_WHEN_MISSING flags don't really make sense + * for proc_cmdline_get_key_many, let's make this clear. */ + assert(!(flags & (PROC_CMDLINE_VALUE_OPTIONAL|PROC_CMDLINE_TRUE_WHEN_MISSING))); /* This call may clobber arguments on failure! */ diff --git a/src/basic/proc-cmdline.h b/src/basic/proc-cmdline.h index babdcbeeaa..9502fb8d43 100644 --- a/src/basic/proc-cmdline.h +++ b/src/basic/proc-cmdline.h @@ -10,6 +10,7 @@ typedef enum ProcCmdlineFlags { PROC_CMDLINE_VALUE_OPTIONAL = 1 << 1, /* the value is optional (for boolean switches that can omit the value) */ PROC_CMDLINE_RD_STRICT = 1 << 2, /* ignore this in the initrd */ PROC_CMDLINE_IGNORE_EFI_OPTIONS = 1 << 3, /* don't check systemd's private EFI variable */ + PROC_CMDLINE_TRUE_WHEN_MISSING = 1 << 4, /* default to true when the key is missing for bool */ } ProcCmdlineFlags; typedef int (*proc_cmdline_parse_t)(const char *key, const char *value, void *data); diff --git a/src/test/test-proc-cmdline.c b/src/test/test-proc-cmdline.c index 3614ebe6f4..9920462ad5 100644 --- a/src/test/test-proc-cmdline.c +++ b/src/test/test-proc-cmdline.c @@ -180,6 +180,7 @@ TEST(proc_cmdline_get_bool) { assert_se(proc_cmdline_get_bool("", /* flags = */ 0, &value) == -EINVAL); assert_se(proc_cmdline_get_bool("abc", /* flags = */ 0, &value) == 0 && value == false); + assert_se(proc_cmdline_get_bool("unspecified", PROC_CMDLINE_TRUE_WHEN_MISSING, &value) == 0 && value == true); assert_se(proc_cmdline_get_bool("foo_bar", /* flags = */ 0, &value) > 0 && value == true); assert_se(proc_cmdline_get_bool("foo-bar", /* flags = */ 0, &value) > 0 && value == true); assert_se(proc_cmdline_get_bool("bar-waldo", /* flags = */ 0, &value) > 0 && value == true); From 60499ed1c85412b521d55ebd55ca81601f03e1da Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 12 Aug 2023 15:22:06 +0800 Subject: [PATCH 2/5] proc-cmdline: do assert on flags earlier --- src/basic/proc-cmdline.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index c666736627..48036b959c 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -197,10 +197,6 @@ static int proc_cmdline_parse_strv(char **args, proc_cmdline_parse_t parse_item, assert(parse_item); - /* The PROC_CMDLINE_VALUE_OPTIONAL and PROC_CMDLINE_TRUE_WHEN_MISSING flags don't really make sense - * for proc_cmdline_parse(), let's make this clear. */ - assert(!(flags & (PROC_CMDLINE_VALUE_OPTIONAL|PROC_CMDLINE_TRUE_WHEN_MISSING))); - STRV_FOREACH(word, args) { char *key, *value; @@ -226,6 +222,10 @@ int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineF assert(parse_item); + /* The PROC_CMDLINE_VALUE_OPTIONAL and PROC_CMDLINE_TRUE_WHEN_MISSING flags don't really make sense + * for proc_cmdline_parse(), let's make this clear. */ + assert(!(flags & (PROC_CMDLINE_VALUE_OPTIONAL|PROC_CMDLINE_TRUE_WHEN_MISSING))); + /* We parse the EFI variable first, because later settings have higher priority. */ if (!FLAGS_SET(flags, PROC_CMDLINE_IGNORE_EFI_OPTIONS)) { From 9d7b189bf37c5b99dbf202af787a067078af3bdd Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 10 Aug 2023 23:25:17 +0800 Subject: [PATCH 3/5] fstab-util: make use of PROC_CMDLINE_TRUE_WHEN_MISSING --- src/shared/fstab-util.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/shared/fstab-util.c b/src/shared/fstab-util.c index fed0e1113f..55e76b6e16 100644 --- a/src/shared/fstab-util.c +++ b/src/shared/fstab-util.c @@ -19,7 +19,7 @@ bool fstab_enabled_full(int enabled) { static int cached = -1; - bool val; + bool val = true; /* If nothing specified or the check fails, then defaults to true. */ int r; /* If 'enabled' is non-negative, then update the cache with it. */ @@ -29,12 +29,11 @@ bool fstab_enabled_full(int enabled) { if (cached >= 0) return cached; - r = proc_cmdline_get_bool("fstab", PROC_CMDLINE_STRIP_RD_PREFIX, &val); + r = proc_cmdline_get_bool("fstab", PROC_CMDLINE_STRIP_RD_PREFIX|PROC_CMDLINE_TRUE_WHEN_MISSING, &val); if (r < 0) - log_debug_errno(r, "Failed to parse kernel command line, ignoring: %m"); + log_debug_errno(r, "Failed to parse fstab= kernel command line option, ignoring: %m"); - /* If nothing specified, then defaults to true. */ - return (cached = r > 0 ? val : true); + return (cached = val); } int fstab_has_fstype(const char *fstype) { From ca05933a1bcf5d22bd019834ecaeab094b4c8d66 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 10 Aug 2023 23:25:44 +0800 Subject: [PATCH 4/5] battery-check: use proc_cmdline_get_bool --- src/battery-check/battery-check.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/src/battery-check/battery-check.c b/src/battery-check/battery-check.c index c3091d3474..3430d2c8a9 100644 --- a/src/battery-check/battery-check.c +++ b/src/battery-check/battery-check.c @@ -88,23 +88,6 @@ static int plymouth_send_message(const char *mode, const char *message) { return 0; } -static int parse_proc_cmdline_item(const char *key, const char *value, void *data) { - int r; - - assert(key); - - if (streq(key, "systemd.battery-check")) { - - r = value ? parse_boolean(value) : 1; - if (r < 0) - log_warning_errno(r, "Failed to parse %s switch, ignoring: %s", key, value); - else - arg_doit = r; - } - - return 0; -} - static int parse_argv(int argc, char * argv[]) { enum { @@ -153,9 +136,9 @@ static int run(int argc, char *argv[]) { log_setup(); - r = proc_cmdline_parse(parse_proc_cmdline_item, NULL, PROC_CMDLINE_STRIP_RD_PREFIX); + r = proc_cmdline_get_bool("systemd.battery-check", PROC_CMDLINE_STRIP_RD_PREFIX|PROC_CMDLINE_TRUE_WHEN_MISSING, &arg_doit); if (r < 0) - log_warning_errno(r, "Failed to parse kernel command line, ignoring: %m"); + log_warning_errno(r, "Failed to parse systemd.battery-check= kernel command line option, ignoring: %m"); r = parse_argv(argc, argv); if (r <= 0) From 022a19c9f69ae7d5fdc283711d111886047d4285 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 10 Aug 2023 23:25:59 +0800 Subject: [PATCH 5/5] reboot-util: cache the result of shall_restore_state --- src/backlight/backlight.c | 2 +- src/rfkill/rfkill.c | 2 +- src/shared/reboot-util.c | 16 ++++++++++------ src/shared/reboot-util.h | 2 +- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index a384208879..24459628fa 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -528,7 +528,7 @@ static int run(int argc, char *argv[]) { _cleanup_free_ char *value = NULL; bool clamp; - if (shall_restore_state() == 0) + if (!shall_restore_state()) return 0; if (validate_device(device) == 0) diff --git a/src/rfkill/rfkill.c b/src/rfkill/rfkill.c index 9631513628..c4e93737f5 100644 --- a/src/rfkill/rfkill.c +++ b/src/rfkill/rfkill.c @@ -140,7 +140,7 @@ static int load_state(Context *c, const struct rfkill_event *event) { assert(c->rfkill_fd >= 0); assert(event); - if (shall_restore_state() == 0) + if (!shall_restore_state()) return 0; r = determine_state_file(event, &state_file); diff --git a/src/shared/reboot-util.c b/src/shared/reboot-util.c index 75910ede44..62ff697fe2 100644 --- a/src/shared/reboot-util.c +++ b/src/shared/reboot-util.c @@ -109,15 +109,19 @@ int reboot_with_parameter(RebootFlags flags) { return log_full_errno(flags & REBOOT_LOG ? LOG_ERR : LOG_DEBUG, errno, "Failed to reboot: %m"); } -int shall_restore_state(void) { - bool ret; +bool shall_restore_state(void) { + static int cached = -1; + bool b = true; /* If nothing specified or the check fails, then defaults to true. */ int r; - r = proc_cmdline_get_bool("systemd.restore_state", /* flags = */ 0, &ret); - if (r < 0) - return r; + if (cached >= 0) + return cached; - return r > 0 ? ret : true; + r = proc_cmdline_get_bool("systemd.restore_state", PROC_CMDLINE_TRUE_WHEN_MISSING, &b); + if (r < 0) + log_debug_errno(r, "Failed to parse systemd.restore_state= kernel command line option, ignoring: %m"); + + return (cached = b); } static int xen_kexec_loaded(void) { diff --git a/src/shared/reboot-util.h b/src/shared/reboot-util.h index 9e6366206e..ccd15c7b35 100644 --- a/src/shared/reboot-util.h +++ b/src/shared/reboot-util.h @@ -12,6 +12,6 @@ typedef enum RebootFlags { int read_reboot_parameter(char **parameter); int reboot_with_parameter(RebootFlags flags); -int shall_restore_state(void); +bool shall_restore_state(void); bool kexec_loaded(void);