parse-argument: make parse_tristate_argument() do something useful

I expressed the issue I have with parse_tristate_argument()
in #37751: it doesn't add any value to direct use of parse_tristate();
on the contrary, it doesn't support means to reset the arg to "auto"/-1 state.
The mere reason it existed is that we need a int type ret param.

Since the previous attempt to address this mess failed, let's
try to make the function more useful by making it accept "auto".
I figure this is useful on its own.

As requested in
https://github.com/systemd/systemd/pull/40652#discussion_r2831833996,
the function name is suffixed with _with_auto() to establish
that "auto" is handled internally.

diff --git a/man/bootctl.xml b/man/bootctl.xml
index 0c46edb6105927cbb1411a460f0793369dd2f9de..5f53023c83d27aeebab5fbf5a72f0624d8f389a0 100644 (file)
--- a/man/bootctl.xml
+++ b/man/bootctl.xml
@@ -407,10 +407,11 @@
       </varlistentry>
 
       <varlistentry>
-        <term><option>--variables=yes|no</option></term>
+        <term><option>--variables=yes|no|auto</option></term>
         <listitem><para>Controls whether to touch the firmware's boot loader list stored in EFI variables,
-        and other EFI variables. If not specified defaults to no when execution in a container runtime is
-        detected, yes otherwise.</para>
+        and other EFI variables. If not specified or set to <option>auto</option>, EFI variables will not
+        be modified when execution in a container runtime is detected. <option>yes</option> may be used to
+        explicit override the check.</para>
 
         <xi:include href="version-info.xml" xpointer="v258"/></listitem>
       </varlistentry>

diff --git a/man/run0.xml b/man/run0.xml
index 831b63a541d263010ea9d36d3b685867e0cc8b83..186383dbbe172eb3bc0949aa205f895eae2c5595 100644 (file)
--- a/man/run0.xml
+++ b/man/run0.xml
@@ -255,8 +255,9 @@
         <term><option>--lightweight=<replaceable>BOOLEAN</replaceable></option></term>
 
         <listitem><para>Controls whether to activate the per-user service manager for the target user. By
-        default if the target user is <literal>root</literal> or a system user the per-user service manager
-        is not activated as effect of the <command>run0</command> invocation, otherwise it is.</para>
+        default (unset or set to <option>auto</option>), if the target user is <literal>root</literal> or
+        a system user the per-user service manager is not activated as effect of the <command>run0</command>
+        invocation, otherwise it is.</para>
 
         <para>This ultimately controls the <varname>$XDG_SESSION_CLASS</varname> environment variable
         <citerefentry><refentrytitle>pam_systemd</refentrytitle><manvolnum>8</manvolnum></citerefentry>

diff --git a/man/systemd-dissect.xml b/man/systemd-dissect.xml
index 9368d90a5283aa55bc0d5a3581a582adb8af7f93..b4ff042bb9df058ecd55517beb8deb6f731fca5f 100644 (file)
--- a/man/systemd-dissect.xml
+++ b/man/systemd-dissect.xml
@@ -555,10 +555,10 @@
         <term><option>--copy-ownership=</option></term>
 
         <listitem><para>Controls whether file ownership (user and group) is preserved when copying files
-        with <option>--copy-from</option> or <option>--copy-to</option>. Takes a boolean. If
-        <literal>yes</literal>, ownership is always preserved. If <literal>no</literal>, ownership is never
-        preserved and the current user's UID/GID is used instead. If not specified, ownership is preserved
-        when copying directory trees, but not when copying individual regular files.
+        with <option>--copy-from</option> or <option>--copy-to</option>. Takes a boolean, or <option>auto</option>.
+        If <literal>yes</literal>, ownership is always preserved. If <literal>no</literal>, ownership is never
+        preserved and the current user's UID/GID is used instead. If not specified or <option>auto</option>,
+        ownership is preserved when copying directory trees, but not when copying individual regular files.
         </para>
 
         <xi:include href="version-info.xml" xpointer="v260"/></listitem>

diff --git a/src/bootctl/bootctl.c b/src/bootctl/bootctl.c
index 7e59fd016814706912b76d2f313f4d8810ad7c00..b69720d66c69d97a795d6be744320100541e2514 100644 (file)
--- a/src/bootctl/bootctl.c
+++ b/src/bootctl/bootctl.c
@@ -521,7 +521,7 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_VARIABLES:
-                        r = parse_tristate_argument("--variables=", optarg, &arg_touch_variables);
+                        r = parse_tristate_argument_with_auto("--variables=", optarg, &arg_touch_variables);
                         if (r < 0)
                                 return r;
                         break;

diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c
index 33ddb53d0a607a21185cba631bbb49e106bcab5f..250be1af3d140ba9690150d9e181e5bfca7a60c9 100644 (file)
--- a/src/dissect/dissect.c
+++ b/src/dissect/dissect.c
@@ -637,7 +637,7 @@ static int parse_argv(int argc, char *argv[]) {
                 }
 
                 case ARG_COPY_OWNERSHIP:
-                        r = parse_tristate_argument("--copy-ownership=", optarg, &arg_copy_ownership);
+                        r = parse_tristate_argument_with_auto("--copy-ownership=", optarg, &arg_copy_ownership);
                         if (r < 0)
                                 return r;
                         break;

diff --git a/src/run/run.c b/src/run/run.c
index e614f27c288d193169a349b964b5af56393a14fb..f1bbbe1f393c2dafd582b46addac272c2a0fda70 100644 (file)
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -1083,7 +1083,7 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) {
                         break;
 
                 case ARG_LIGHTWEIGHT:
-                        r = parse_tristate_argument("--lightweight=", optarg, &arg_lightweight);
+                        r = parse_tristate_argument_with_auto("--lightweight=", optarg, &arg_lightweight);
                         if (r < 0)
                                 return r;
                         break;

diff --git a/src/shared/parse-argument.c b/src/shared/parse-argument.c
index f8cd7f30ff7470872252aa621d4537e4536a67d8..39e5328e7037ef5761ae6df3c978a8a9647149a0 100644 (file)
--- a/src/shared/parse-argument.c
+++ b/src/shared/parse-argument.c
@@ -20,10 +20,12 @@ int parse_boolean_argument(const char *optname, const char *s, bool *ret) {
 
         /* Returns the result through *ret and the return value. */
 
+        assert(optname);
+
         if (s) {
                 r = parse_boolean(s);
                 if (r < 0)
-                        return log_error_errno(r, "Failed to parse boolean argument to %s: %s.", optname, s);
+                        return log_error_errno(r, "Failed to parse boolean argument to '%s': %s", optname, s);
 
                 if (ret)
                         *ret = r;
@@ -36,24 +38,20 @@ int parse_boolean_argument(const char *optname, const char *s, bool *ret) {
         }
 }
 
-int parse_tristate_argument(const char *optname, const char *s, int *ret) {
+int parse_tristate_argument_with_auto(const char *optname, const char *s, int *ret) {
         int r;
 
-        if (s) {
-                r = parse_boolean(s);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to parse boolean argument to %s: %s.", optname, s);
-
-                if (ret)
-                        *ret = r;
+        assert(optname);
+        assert(s); /* We refuse NULL optarg here, since that would be ambiguous on cmdline:
+                      for --enable-a[=BOOL], --enable-a is intuitively interpreted as true rather than "auto"
+                      (parse_boolean_argument() does exactly that). IOW, tristate options should require
+                      arguments. */
 
-                return r;
-        } else {
-                if (ret)
-                        *ret = -1;
+        r = parse_tristate_full(s, "auto", ret);
+        if (r < 0)
+                return log_error_errno(r, "Failed to parse tristate argument to '%s': %s", optname, s);
 
-                return 0;
-        }
+        return 0;
 }
 
 int parse_json_argument(const char *s, sd_json_format_flags_t *ret) {

diff --git a/src/shared/parse-argument.h b/src/shared/parse-argument.h
index 75098b8eb1aa9513c7d2f13f08a9b7403ebb1bd5..0df7fc3b5e0ac9df04bd8a9ded2f5899661e56f0 100644 (file)
--- a/src/shared/parse-argument.h
+++ b/src/shared/parse-argument.h
@@ -4,7 +4,7 @@
 #include "shared-forward.h"
 
 int parse_boolean_argument(const char *optname, const char *s, bool *ret);
-int parse_tristate_argument(const char *optname, const char *s, int *ret);
+int parse_tristate_argument_with_auto(const char *optname, const char *s, int *ret);
 int parse_json_argument(const char *s, sd_json_format_flags_t *ret);
 int parse_path_argument(const char *path, bool suppress_root, char **arg);
 int parse_signal_argument(const char *s, int *ret);

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 01b5517d142c2cba7733e8047d203f37066611af..382a50e312322cd08d9c222caece3693f3475d1b 100644 (file)
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -830,9 +830,9 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_CHECK_INHIBITORS:
-                        r = parse_tristate_full(optarg, "auto", &arg_check_inhibitors);
+                        r = parse_tristate_argument_with_auto("--check-inhibitors=", optarg, &arg_check_inhibitors);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to parse --check-inhibitors= argument: %s", optarg);
+                                return r;
                         break;
 
                 case ARG_PLAIN: