udevadm: several cleanups for command line option parsers (#36241)

man/udevadm.xml

@@ -897,13 +897,11 @@
           <term><option>-N</option></term>
           <term><option>--resolve-names=<constant>early</constant>|<constant>late</constant>|<constant>never</constant></option></term>
           <listitem>
-            <para>Specify when udevadm should resolve names of users
+            <para>Specify when <command>udevadm</command> should resolve names of users and groups specified
-            and groups.  When set to <constant>early</constant> (the
+            in udev rules. When set to <constant>early</constant> (the default), names will be resolved when
-            default), names will be resolved when the rules are
+            the rules are parsed. When set to <constant>late</constant>, names will be resolved during the
-            parsed. When set to <constant>late</constant>, names will
+            event being processed. When set to <constant>never</constant>, names will never be resolved and
-            be resolved for every event. When set to
+            relevant udev rules will be ignored.</para>
-            <constant>never</constant>, names will never be resolved
-            and all devices will be owned by root.</para>
 
             <xi:include href="version-info.xml" xpointer="v209"/>
           </listitem>
@@ -989,13 +987,14 @@
       <variablelist>
         <varlistentry>
           <term><option>-N</option></term>
-          <term><option>--resolve-names=<constant>early</constant>|<constant>never</constant></option></term>
+          <term><option>--resolve-names=<constant>early</constant>|<constant>late</constant>|<constant>never</constant></option></term>
           <listitem>
-            <para>Specify when udevadm should resolve names of users
+            <para>Specify when <command>udevadm</command> should resolve names of users and groups specified
-            and groups.  When set to <constant>early</constant> (the
+            in udev rules. When set to <constant>early</constant> (the default), names will be resolved when
-            default), names will be resolved when the rules are
+            the rules are parsed. When set to <constant>late</constant>, names will not be verified, as
-            parsed. When set to <constant>never</constant>, names will
+            <command>systemd-udevd</command> resolves names during each event being processed. When set to
-            never be resolved.</para>
+            <constant>never</constant>, names will never be resolved and relevant rules will be ignored.
+            </para>
 
             <xi:include href="version-info.xml" xpointer="v254"/>
           </listitem>

shell-completion/bash/udevadm

@@ -258,7 +258,7 @@ _udevadm() {
                         comps=$( udevadm test --action help )
                         ;;
                     -N|--resolve-names)
-                        comps='early late never'
+                        comps=$( udevadm test --resolve-names help )
                         ;;
                     -D|--extra-rules-dir)
                         comps=''
@@ -304,7 +304,7 @@ _udevadm() {
             if __contains_word "$prev" ${OPTS[VERIFY_ARG]}; then
                 case $prev in
                     -N|--resolve-names)
-                        comps='early never'
+                        comps=$( udevadm test --resolve-names help )
                         ;;
                     --root)
                         comps=''

shell-completion/zsh/_udevadm

@@ -88,7 +88,8 @@ _udevadm_test(){
         '(-)'{-h,--help}'[Show help]' \
         '(-)'{-V,--version}'[Show package version]' \
         '--action=[The action string.]:actions:(add change remove move online offline bind unbind)' \
-        '--subsystem=[The subsystem string.]' \
+        '(-N --resolve-names)'{-N,--resolve-names=}'[When to resolve names.]:resolve:(early late never)' \
+       '--subsystem=[The subsystem string.]' \
         '(-D --extra-rules-dir=)'{-D,--extra-rules-dir=}'[Also load rules from the directory.]' \
         '(-v --verbose)'{-v,--verbose}'[Show verbose logs.]' \
         '*::devpath:_files -P /sys/ -W /sys'
@@ -119,7 +120,7 @@ _udevadm_verify(){
     _arguments \
         '(- *)'{-h,--help}'[Show help]' \
         '(- *)'{-V,--version}'[Show package version]' \
-        '(-N --resolve-names)'{-N+,--resolve-names=}'[When to resolve names.]:resolve:(early never)' \
+        '(-N --resolve-names)'{-N,--resolve-names=}'[When to resolve names.]:resolve:(early late never)' \
         '--root=[Operate on catalog hierarchy under specified directory]:directories:_directories' \
         --no-summary'[Do not show summary.]' \
         --no-style'[Ignore style issues.]' \

src/libsystemd/sd-device/device-private.c

@@ -961,7 +961,3 @@ static const char* const device_action_table[_SD_DEVICE_ACTION_MAX] = {
 };
 
 DEFINE_STRING_TABLE_LOOKUP(device_action, sd_device_action_t);
-
-void dump_device_action_table(void) {
-        DUMP_STRING_TABLE(device_action, sd_device_action_t, _SD_DEVICE_ACTION_MAX);
-}

src/libsystemd/sd-device/device-private.h

@@ -77,4 +77,3 @@ int device_read_uevent_file(sd_device *device);
 int device_set_action(sd_device *device, sd_device_action_t a);
 sd_device_action_t device_action_from_string(const char *s) _pure_;
 const char* device_action_to_string(sd_device_action_t a) _const_;
-void dump_device_action_table(void);

src/udev/udevadm-test-builtin.c

@@ -45,10 +45,8 @@ static int parse_argv(int argc, char *argv[]) {
                 switch (c) {
                 case 'a':
                         r = parse_device_action(optarg, &arg_action);
-                        if (r < 0)
+                        if (r <= 0)
-                                return log_error_errno(r, "Invalid action '%s'", optarg);
+                                return r;
-                        if (r == 0)
-                                return 0;
                         break;
                 case 'V':
                         return print_version();

src/udev/udevadm-test.c

@@ -60,16 +60,13 @@ static int parse_argv(int argc, char *argv[]) {
                 switch (c) {
                 case 'a':
                         r = parse_device_action(optarg, &arg_action);
-                        if (r < 0)
+                        if (r <= 0)
-                                return log_error_errno(r, "Invalid action '%s'", optarg);
+                                return r;
-                        if (r == 0)
-                                return 0;
                         break;
                 case 'N':
-                        arg_resolve_name_timing = resolve_name_timing_from_string(optarg);
+                        r = parse_resolve_name_timing(optarg, &arg_resolve_name_timing);
-                        if (arg_resolve_name_timing < 0)
+                        if (r <= 0)
-                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                return r;
-                                                       "--resolve-names= must be early, late or never");
                         break;
                 case 'D': {
                         _cleanup_free_ char *p = NULL;

src/udev/udevadm-trigger.c

@@ -369,10 +369,8 @@ int trigger_main(int argc, char *argv[], void *userdata) {
                         break;
                 case 'c':
                         r = parse_device_action(optarg, &action);
-                        if (r < 0)
+                        if (r <= 0)
-                                return log_error_errno(r, "Unknown action '%s'", optarg);
+                                return r;
-                        if (r == 0)
-                                return 0;
                         break;
                 case 's':
                         r = sd_device_enumerator_add_match_subsystem(e, optarg, true);

src/udev/udevadm-util.c

@@ -10,7 +10,9 @@
 #include "constants.h"
 #include "device-private.h"
 #include "path-util.h"
+#include "string-table.h"
 #include "udev-ctrl.h"
+#include "udev-rules.h"
 #include "udev-varlink.h"
 #include "udevadm-util.h"
 #include "unit-name.h"
@@ -110,22 +112,34 @@ int find_device_with_action(const char *id, sd_device_action_t action, sd_device
         return 0;
 }
 
-int parse_device_action(const char *str, sd_device_action_t *action) {
-        sd_device_action_t a;
 
+int parse_device_action(const char *str, sd_device_action_t *ret) {
         assert(str);
-        assert(action);
 
-        if (streq(str, "help")) {
+        if (streq(str, "help"))
-                dump_device_action_table();
+                return DUMP_STRING_TABLE(device_action, sd_device_action_t, _SD_DEVICE_ACTION_MAX);
-                return 0;
-        }
 
-        a = device_action_from_string(str);
+        sd_device_action_t a = device_action_from_string(str);
         if (a < 0)
-                return a;
+                return log_error_errno(a, "Invalid action '%s'.", str);
 
-        *action = a;
+        if (ret)
+                *ret = a;
+        return 1;
+}
+
+int parse_resolve_name_timing(const char *str, ResolveNameTiming *ret) {
+        assert(str);
+
+        if (streq(str, "help"))
+                return DUMP_STRING_TABLE(resolve_name_timing, ResolveNameTiming, _RESOLVE_NAME_TIMING_MAX);
+
+        ResolveNameTiming v = resolve_name_timing_from_string(optarg);
+        if (v < 0)
+                return log_error_errno(v, "--resolve-names= must be 'early', 'late', or 'never'.");
+
+        if (ret)
+                *ret = v;
         return 1;
 }
 

src/udev/udevadm-util.h

@@ -3,8 +3,11 @@
 
 #include "sd-device.h"
 
+#include "udev-def.h"
+
 int find_device(const char *id, const char *prefix, sd_device **ret);
 int find_device_with_action(const char *id, sd_device_action_t action, sd_device **ret);
-int parse_device_action(const char *str, sd_device_action_t *action);
+int parse_device_action(const char *str, sd_device_action_t *ret);
+int parse_resolve_name_timing(const char *str, ResolveNameTiming *ret);
 int udev_ping(usec_t timeout, bool ignore_connection_failure);
 int search_rules_files(char * const *a, const char *root, char ***ret);

src/udev/udevadm-verify.c

@@ -36,7 +36,7 @@ static int help(void) {
                "\n%sVerify udev rules files.%s\n\n"
                "  -h --help                            Show this help\n"
                "  -V --version                         Show package version\n"
-               "  -N --resolve-names=early|never       When to resolve names\n"
+               "  -N --resolve-names=early|late|never  When to resolve names\n"
                "     --root=PATH                       Operate on an alternate filesystem root\n"
                "     --no-summary                      Do not show summary\n"
                "     --no-style                        Ignore style issues\n"
@@ -77,17 +77,9 @@ static int parse_argv(int argc, char *argv[]) {
                 case 'V':
                         return print_version();
                 case 'N':
-                        arg_resolve_name_timing = resolve_name_timing_from_string(optarg);
+                        r = parse_resolve_name_timing(optarg, &arg_resolve_name_timing);
-                        if (arg_resolve_name_timing < 0)
+                        if (r <= 0)
-                                return log_error_errno(arg_resolve_name_timing,
+                                return r;
-                                                       "--resolve-names= takes \"early\" or \"never\"");
-                        /*
-                         * In the verifier "late" has the effect of "never",
-                         * and "never" would generate irrelevant diagnostics,
-                         * so map "never" to "late".
-                         */
-                        if (arg_resolve_name_timing == RESOLVE_NAME_NEVER)
-                                arg_resolve_name_timing = RESOLVE_NAME_LATE;
                         break;
                 case ARG_ROOT:
                         r = parse_path_argument(optarg, /* suppress_root= */ true, &arg_root);

test/meson.build

@@ -136,7 +136,7 @@ if want_tests != 'false'
         test('udev-rules-check',
              exe,
              suite : 'udev',
-             args : ['verify', '--resolve-names=never', all_rules])
+             args : ['verify', '--resolve-names=late', all_rules])
 endif
 
 ############################################################

test/units/TEST-17-UDEV.11.sh

@@ -103,6 +103,8 @@ assert_0 -h
 assert_0 --help
 assert_0 -V
 assert_0 --version
+assert_0 -N help
+assert_0 --resolve-names help
 
 # unrecognized option '--unknown'
 assert_1 --unknown
@@ -112,7 +114,7 @@ assert_1 -N
 assert_1 -N now
 # option '--resolve-names' requires an argument
 assert_1 --resolve-names
-# --resolve-names= takes "early" or "never"
+# --resolve-names= takes "early", "late", or "never"
 assert_1 --resolve-names=now
 # Failed to parse rules file ./nosuchfile: No such file or directory
 assert_1 ./nosuchfile