bootctl: rework setting of menu timeout variables

menu-force and menu-hidden were added in 97f077df052c75224dcc73375bfaaa69af6a1c26,
menu-disable was added in 6efdd7fec5106205240332bd3b7fd2f93d4d9d4c, a year later.
So we can assume that if the feature flag is set, the other string values are
supported too. The comment that there's no way check that was added later in
5b45fad4fcfa2dd81f25b13fe8d7717f62fa5843, but it was incorrect even at that
time.

Fixes https://github.com/systemd/systemd/issues/39167. As described in the
issue, we documented various string values in the BLI, but bootctl didn't use
the string values. At the time menu-force and menu-hidden were added, using
numerical values for compatibility made sense. But that stopped being needed
when a string value that didn't have a strictly equivalent numerical value and
a feature flag were added.

When converting a large number to menu-force, message is downgraded to debug,
since the severity of the issue is very minor. Debug messages are added in
other places when the requested setting is modified too.

diff --git a/src/bootctl/bootctl-set-efivar.c b/src/bootctl/bootctl-set-efivar.c
index b102d6600ea8b9ea60239fecd221eab421a116d7..360664b879f75a22a85dc8c57876041af095532f 100644 (file)
--- a/src/bootctl/bootctl-set-efivar.c
+++ b/src/bootctl/bootctl-set-efivar.c
@@ -15,51 +15,69 @@
 #include "virt.h"
 
 static int parse_timeout(const char *arg1, char16_t **ret_timeout, size_t *ret_timeout_size) {
-        char utf8[DECIMAL_STR_MAX(usec_t)];
-        char16_t *encoded;
+        char buf[DECIMAL_STR_MAX(usec_t)];
         usec_t timeout;
-        bool menu_disabled = false;
+        uint64_t loader_features = 0;
         int r;
 
         assert(arg1);
         assert(ret_timeout);
         assert(ret_timeout_size);
 
-        assert_cc(STRLEN("menu-disabled") < ELEMENTSOF(utf8));
+        assert_cc(STRLEN("menu-force") < ELEMENTSOF(buf));
+        assert_cc(STRLEN("menu-hidden") < ELEMENTSOF(buf));
+        assert_cc(STRLEN("menu-disabled") < ELEMENTSOF(buf));
 
-        /* Note: Since there is no way to query if the bootloader supports the string tokens, we explicitly
-         * set their numerical value(s) instead. This means that some of the sd-boot internal ABI has leaked
-         * although the ship has sailed and the side-effects are self-contained.
+        /* Use feature EFI_LOADER_FEATURE_MENU_DISABLE as a mark that the boot loader supports the other
+         * string values too. When unsupported, convert to the timeout with the closest meaning.
          */
-        if (streq(arg1, "menu-force"))
-                timeout = USEC_INFINITY;
-        else if (streq(arg1, "menu-hidden"))
-                timeout = 0;
-        else if (streq(arg1, "menu-disabled")) {
-                uint64_t loader_features = 0;
 
+        if (streq(arg1, "menu-force")) {
                 (void) efi_loader_get_features(&loader_features);
+
                 if (!(loader_features & EFI_LOADER_FEATURE_MENU_DISABLE)) {
-                        if (arg_graceful() == ARG_GRACEFUL_NO)
-                                return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Loader does not support 'menu-disabled'.");
+                        log_debug("Using maximum timeout instead of '%s'.", arg1);
+                        timeout = USEC_INFINITY;
+                        arg1 = NULL;
+                }
+
+        } else if (streq(arg1, "menu-hidden")) {
+                (void) efi_loader_get_features(&loader_features);
 
-                        log_warning("Loader does not support 'menu-disabled', setting anyway.");
+                if (!(loader_features & EFI_LOADER_FEATURE_MENU_DISABLE)) {
+                        log_debug("Using zero timeout instead of '%s'.", arg1);
+                        timeout = 0;
+                        arg1 = NULL;  /* replace the arg by printed timeout value later */
                 }
-                menu_disabled = true;
+
+        } else if (streq(arg1, "menu-disabled")) {
+                (void) efi_loader_get_features(&loader_features);
+
+                if (!(loader_features & EFI_LOADER_FEATURE_MENU_DISABLE)) {
+                        if (arg_graceful() == ARG_GRACEFUL_NO)
+                                return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
+                                                       "Loader does not support '%s'.", arg1);
+                        log_warning("Using zero timeout instead of '%s'.", arg1);
+                        timeout = 0;
+                        arg1 = NULL;
+                }
+
         } else {
                 r = parse_time(arg1, &timeout, USEC_PER_SEC);
                 if (r < 0)
                         return log_error_errno(r, "Failed to parse timeout '%s': %m", arg1);
-                if (timeout != USEC_INFINITY && timeout > UINT32_MAX * USEC_PER_SEC)
-                        log_warning("Timeout is too long and will be treated as 'menu-force' instead.");
+
+                assert_cc(USEC_INFINITY > UINT32_MAX * USEC_PER_SEC);
+                if (timeout > UINT32_MAX * USEC_PER_SEC && timeout != USEC_INFINITY)
+                        log_debug("Timeout is too long and will be clamped to maximum timeout.");
+
+                arg1 = NULL;
         }
 
-        if (menu_disabled)
-                xsprintf(utf8, "menu-disabled");
-        else
-                xsprintf(utf8, USEC_FMT, MIN(timeout / USEC_PER_SEC, UINT32_MAX));
+        if (!arg1)
+                xsprintf(buf, USEC_FMT, MIN(timeout / USEC_PER_SEC, UINT32_MAX));
 
-        encoded = utf8_to_utf16(utf8, SIZE_MAX);
+        char16_t *encoded = utf8_to_utf16(arg1 ?: buf, SIZE_MAX);
         if (!encoded)
                 return log_oom();