From 9b65fb8f34cf69869454d31abad671c475807c35 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Tue, 29 Apr 2025 10:09:11 +0200 Subject: [PATCH 1/8] udevadm-trigger: add vertical space to separate cases --- src/udev/udevadm-trigger.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index 860335c8d5..27d3d140d8 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -282,12 +282,15 @@ int trigger_main(int argc, char *argv[], void *userdata) { case 'v': arg_verbose = true; break; + case 'n': arg_dry_run = true; break; + case 'q': arg_quiet = true; break; + case 't': if (streq(optarg, "devices")) device_type = TYPE_DEVICES; @@ -298,21 +301,25 @@ int trigger_main(int argc, char *argv[], void *userdata) { else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown type --type=%s", optarg); break; + case 'c': r = parse_device_action(optarg, &action); if (r <= 0) return r; break; + case 's': r = sd_device_enumerator_add_match_subsystem(e, optarg, true); if (r < 0) return log_error_errno(r, "Failed to add subsystem match '%s': %m", optarg); break; + case 'S': r = sd_device_enumerator_add_match_subsystem(e, optarg, false); if (r < 0) return log_error_errno(r, "Failed to add negative subsystem match '%s': %m", optarg); break; + case 'a': buf = keyval(optarg, &key, &val); if (!buf) @@ -321,6 +328,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to add sysattr match '%s=%s': %m", key, val); break; + case 'A': buf = keyval(optarg, &key, &val); if (!buf) @@ -329,6 +337,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to add negative sysattr match '%s=%s': %m", key, val); break; + case 'p': buf = keyval(optarg, &key, &val); if (!buf) @@ -337,16 +346,19 @@ int trigger_main(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to add property match '%s=%s': %m", key, val); break; + case 'g': r = sd_device_enumerator_add_match_tag(e, optarg); if (r < 0) return log_error_errno(r, "Failed to add tag match '%s': %m", optarg); break; + case 'y': r = sd_device_enumerator_add_match_sysname(e, optarg); if (r < 0) return log_error_errno(r, "Failed to add sysname match '%s': %m", optarg); break; + case 'b': { _cleanup_(sd_device_unrefp) sd_device *dev = NULL; @@ -359,11 +371,13 @@ int trigger_main(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to add parent match '%s': %m", optarg); break; } + case ARG_INCLUDE_PARENTS: r = sd_device_enumerator_add_all_parents(e); if (r < 0) return log_error_errno(r, "Failed to always include all parents: %m"); break; + case 'w': arg_settle = true; break; @@ -408,18 +422,23 @@ int trigger_main(int argc, char *argv[], void *userdata) { } break; } + case ARG_INITIALIZED_MATCH: case ARG_INITIALIZED_NOMATCH: r = device_enumerator_add_match_is_initialized(e, c == ARG_INITIALIZED_MATCH ? MATCH_INITIALIZED_YES : MATCH_INITIALIZED_NO); if (r < 0) return log_error_errno(r, "Failed to set initialized filter: %m"); break; + case 'V': return print_version(); + case 'h': return help(); + case '?': return -EINVAL; + default: assert_not_reached(); } From 5f0ebd97547f8d6aea1948302e2b3c7055364353 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Tue, 29 Apr 2025 10:36:28 +0200 Subject: [PATCH 2/8] udevadm-trigger: split arg. parsing into a separate function --- src/udev/udevadm-trigger.c | 350 +++++++++++++++++++++++++------------ 1 file changed, 238 insertions(+), 112 deletions(-) diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index 27d3d140d8..5a45a93ac9 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -23,11 +23,46 @@ #include "udevadm-util.h" #include "virt.h" +typedef enum { + TYPE_DEVICES, + TYPE_SUBSYSTEMS, + TYPE_ALL, +} ScanType; + static bool arg_verbose = false; static bool arg_dry_run = false; static bool arg_quiet = false; static bool arg_uuid = false; static bool arg_settle = false; +static ScanType arg_scan_type = TYPE_DEVICES; +static sd_device_action_t arg_action = SD_DEVICE_CHANGE; +static char **arg_devices = NULL; +static char **arg_attr_match = NULL; +static char **arg_attr_nomatch = NULL; +static char **arg_name_match = NULL; +static char **arg_parent_match = NULL; +static char **arg_property_match = NULL; +static char **arg_subsystem_match = NULL; +static char **arg_subsystem_nomatch = NULL; +static char **arg_sysname_match = NULL; +static char **arg_tag_match = NULL; +static char **arg_prioritized_subsystems = NULL; +static int arg_initialized_match = -1; +static bool arg_include_parents = false; +static bool arg_ping = false; +static usec_t arg_ping_timeout_usec = 5 * USEC_PER_SEC; + +STATIC_DESTRUCTOR_REGISTER(arg_devices, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_attr_match, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_attr_nomatch, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_name_match, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_parent_match, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_property_match, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_subsystem_match, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_subsystem_nomatch, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_sysname_match, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_tag_match, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_prioritized_subsystems, strv_freep); static int exec_list( sd_device_enumerator *e, @@ -174,6 +209,131 @@ static char* keyval(const char *str, const char **key, const char **val) { return buf; } +static int add_device_match(sd_device_enumerator *e, const char *s, const char *prefix) { + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + int r; + + assert(e); + assert(s); + + r = find_device(s, prefix, &dev); + if (r < 0) + return log_error_errno(r, "Failed to open the device '%s': %m", s); + + r = device_enumerator_add_match_parent_incremental(e, dev); + if (r < 0) + return log_error_errno(r, "Failed to add parent match '%s': %m", s); + + return 0; +} + +static int setup_matches(sd_device_enumerator *e) { + int r; + + assert(e); + + STRV_FOREACH(d, arg_devices) { + r = add_device_match(e, *d, /* prefix = */ NULL); + if (r < 0) + return r; + } + + STRV_FOREACH(n, arg_name_match) { + r = add_device_match(e, *n, "/dev/"); + if (r < 0) + return r; + } + + STRV_FOREACH(p, arg_parent_match) { + r = add_device_match(e, *p, "/sys/"); + if (r < 0) + return r; + } + + STRV_FOREACH(s, arg_subsystem_match) { + r = sd_device_enumerator_add_match_subsystem(e, *s, /* match= */ true); + if (r < 0) + return log_error_errno(r, "Failed to add subsystem match '%s': %m", *s); + } + + STRV_FOREACH(s, arg_subsystem_nomatch) { + r = sd_device_enumerator_add_match_subsystem(e, *s, /* match= */ false); + if (r < 0) + return log_error_errno(r, "Failed to add negative subsystem match '%s': %m", *s); + } + + STRV_FOREACH(a, arg_attr_match) { + _cleanup_free_ char *buf = NULL; + const char *k, *v; + + buf = keyval(*a, &k, &v); + if (!buf) + return log_oom(); + + r = sd_device_enumerator_add_match_sysattr(e, k, v, /* match= */ true); + if (r < 0) + return log_error_errno(r, "Failed to add sysattr match '%s=%s': %m", k, v); + } + + STRV_FOREACH(a, arg_attr_nomatch) { + _cleanup_free_ char *buf = NULL; + const char *k, *v; + + buf = keyval(*a, &k, &v); + if (!buf) + return log_oom(); + + r = sd_device_enumerator_add_match_sysattr(e, k, v, /* match= */ false); + if (r < 0) + return log_error_errno(r, "Failed to add negative sysattr match '%s=%s': %m", k, v); + } + + STRV_FOREACH(p, arg_property_match) { + _cleanup_free_ char *buf = NULL; + const char *k, *v; + + buf = keyval(*p, &k, &v); + if (!buf) + return log_oom(); + + r = sd_device_enumerator_add_match_property(e, k, v); + if (r < 0) + return log_error_errno(r, "Failed to add property match '%s=%s': %m", k, v); + } + + STRV_FOREACH(t, arg_tag_match) { + r = sd_device_enumerator_add_match_tag(e, *t); + if (r < 0) + return log_error_errno(r, "Failed to add tag match '%s': %m", *t); + } + + STRV_FOREACH(s, arg_sysname_match) { + r = sd_device_enumerator_add_match_sysname(e, *s); + if (r < 0) + return log_error_errno(r, "Failed to add sysname match '%s': %m", *s); + } + + STRV_FOREACH(p, arg_prioritized_subsystems) { + r = device_enumerator_add_prioritized_subsystem(e, *p); + if (r < 0) + return log_error_errno(r, "Failed to add prioritized subsystem '%s': %m", *p); + } + + if (arg_initialized_match != -1) { + r = device_enumerator_add_match_is_initialized(e, arg_initialized_match); + if (r < 0) + return log_error_errno(r, "Failed to set initialized filter: %m"); + } + + if (arg_include_parents) { + r = sd_device_enumerator_add_all_parents(e); + if (r < 0) + return log_error_errno(r, "Failed to always include all parents: %m"); + } + + return 0; +} + static int help(void) { printf("%s trigger [OPTIONS] DEVPATH\n\n" "Request events from the kernel.\n\n" @@ -210,7 +370,7 @@ static int help(void) { return 0; } -int trigger_main(int argc, char *argv[], void *userdata) { +static int parse_argv(int argc, char *argv[]) { enum { ARG_NAME = 0x100, ARG_PING, @@ -247,37 +407,13 @@ int trigger_main(int argc, char *argv[], void *userdata) { { "prioritized-subsystem", required_argument, NULL, ARG_PRIORITIZED_SUBSYSTEM }, {} }; - enum { - TYPE_DEVICES, - TYPE_SUBSYSTEMS, - TYPE_ALL, - } device_type = TYPE_DEVICES; - sd_device_action_t action = SD_DEVICE_CHANGE; - _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; - _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *m = NULL; - _cleanup_(sd_event_unrefp) sd_event *event = NULL; - _cleanup_set_free_ Set *settle_ids = NULL; - usec_t ping_timeout_usec = 5 * USEC_PER_SEC; - bool ping = false; + int c, r; - if (running_in_chroot() > 0) { - log_info("Running in chroot, ignoring request."); - return 0; - } - - r = sd_device_enumerator_new(&e); - if (r < 0) - return r; - - r = sd_device_enumerator_allow_uninitialized(e); - if (r < 0) - return r; + assert(argc >= 0); + assert(argv); while ((c = getopt_long(argc, argv, "vnqt:c:s:S:a:A:p:g:y:b:wVh", options, NULL)) >= 0) { - _cleanup_free_ char *buf = NULL; - const char *key, *val; - switch (c) { case 'v': arg_verbose = true; @@ -293,112 +429,87 @@ int trigger_main(int argc, char *argv[], void *userdata) { case 't': if (streq(optarg, "devices")) - device_type = TYPE_DEVICES; + arg_scan_type = TYPE_DEVICES; else if (streq(optarg, "subsystems")) - device_type = TYPE_SUBSYSTEMS; + arg_scan_type = TYPE_SUBSYSTEMS; else if (streq(optarg, "all")) - device_type = TYPE_ALL; + arg_scan_type = TYPE_ALL; else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown type --type=%s", optarg); break; case 'c': - r = parse_device_action(optarg, &action); + r = parse_device_action(optarg, &arg_action); if (r <= 0) return r; break; case 's': - r = sd_device_enumerator_add_match_subsystem(e, optarg, true); + r = strv_extend(&arg_subsystem_match, optarg); if (r < 0) - return log_error_errno(r, "Failed to add subsystem match '%s': %m", optarg); + return log_oom(); break; case 'S': - r = sd_device_enumerator_add_match_subsystem(e, optarg, false); + r = strv_extend(&arg_subsystem_nomatch, optarg); if (r < 0) - return log_error_errno(r, "Failed to add negative subsystem match '%s': %m", optarg); + return log_oom(); break; case 'a': - buf = keyval(optarg, &key, &val); - if (!buf) - return log_oom(); - r = sd_device_enumerator_add_match_sysattr(e, key, val, true); + r = strv_extend(&arg_attr_match, optarg); if (r < 0) - return log_error_errno(r, "Failed to add sysattr match '%s=%s': %m", key, val); + return log_oom(); break; case 'A': - buf = keyval(optarg, &key, &val); - if (!buf) - return log_oom(); - r = sd_device_enumerator_add_match_sysattr(e, key, val, false); + r = strv_extend(&arg_attr_nomatch, optarg); if (r < 0) - return log_error_errno(r, "Failed to add negative sysattr match '%s=%s': %m", key, val); + return log_oom(); break; case 'p': - buf = keyval(optarg, &key, &val); - if (!buf) - return log_oom(); - r = sd_device_enumerator_add_match_property(e, key, val); + r = strv_extend(&arg_property_match, optarg); if (r < 0) - return log_error_errno(r, "Failed to add property match '%s=%s': %m", key, val); + return log_oom(); break; case 'g': - r = sd_device_enumerator_add_match_tag(e, optarg); + r = strv_extend(&arg_tag_match, optarg); if (r < 0) - return log_error_errno(r, "Failed to add tag match '%s': %m", optarg); + return log_oom(); break; case 'y': - r = sd_device_enumerator_add_match_sysname(e, optarg); + r = strv_extend(&arg_sysname_match, optarg); if (r < 0) - return log_error_errno(r, "Failed to add sysname match '%s': %m", optarg); + return log_oom(); break; - case 'b': { - _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - - r = find_device(optarg, "/sys", &dev); + case 'b': + r = strv_extend(&arg_parent_match, optarg); if (r < 0) - return log_error_errno(r, "Failed to open the device '%s': %m", optarg); - - r = device_enumerator_add_match_parent_incremental(e, dev); - if (r < 0) - return log_error_errno(r, "Failed to add parent match '%s': %m", optarg); + return log_oom(); break; - } case ARG_INCLUDE_PARENTS: - r = sd_device_enumerator_add_all_parents(e); - if (r < 0) - return log_error_errno(r, "Failed to always include all parents: %m"); + arg_include_parents = true; break; case 'w': arg_settle = true; break; - case ARG_NAME: { - _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - - r = find_device(optarg, "/dev", &dev); + case ARG_NAME: + r = strv_extend(&arg_name_match, optarg); if (r < 0) - return log_error_errno(r, "Failed to open the device '%s': %m", optarg); - - r = device_enumerator_add_match_parent_incremental(e, dev); - if (r < 0) - return log_error_errno(r, "Failed to add parent match '%s': %m", optarg); + return log_oom(); break; - } case ARG_PING: - ping = true; + arg_ping = true; if (optarg) { - r = parse_sec(optarg, &ping_timeout_usec); + r = parse_sec(optarg, &arg_ping_timeout_usec); if (r < 0) log_error_errno(r, "Failed to parse timeout value '%s', ignoring: %m", optarg); } @@ -408,26 +519,18 @@ int trigger_main(int argc, char *argv[], void *userdata) { arg_uuid = true; break; - case ARG_PRIORITIZED_SUBSYSTEM: { - _cleanup_strv_free_ char **subsystems = NULL; - - subsystems = strv_split(optarg, ","); - if (!subsystems) - return log_error_errno(r, "Failed to parse prioritized subsystem '%s': %m", optarg); - - STRV_FOREACH(p, subsystems) { - r = device_enumerator_add_prioritized_subsystem(e, *p); - if (r < 0) - return log_error_errno(r, "Failed to add prioritized subsystem '%s': %m", *p); - } + case ARG_PRIORITIZED_SUBSYSTEM: + r = strv_split_and_extend(&arg_prioritized_subsystems, optarg, ",", /* filter_duplicates= */ false); + if (r < 0) + return log_oom(); break; - } case ARG_INITIALIZED_MATCH: + arg_initialized_match = MATCH_INITIALIZED_YES; + break; + case ARG_INITIALIZED_NOMATCH: - r = device_enumerator_add_match_is_initialized(e, c == ARG_INITIALIZED_MATCH ? MATCH_INITIALIZED_YES : MATCH_INITIALIZED_NO); - if (r < 0) - return log_error_errno(r, "Failed to set initialized filter: %m"); + arg_initialized_match = MATCH_INITIALIZED_NO; break; case 'V': @@ -444,25 +547,48 @@ int trigger_main(int argc, char *argv[], void *userdata) { } } - if (ping) { - r = udev_ping(ping_timeout_usec, /* ignore_connection_failure = */ false); + r = strv_extend_strv(&arg_devices, argv + optind, /* filter_duplicates= */ false); + if (r < 0) + return log_error_errno(r, "Failed to build argument list: %m"); + + return 1; +} + +int trigger_main(int argc, char *argv[], void *userdata) { + _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *m = NULL; + _cleanup_(sd_event_unrefp) sd_event *event = NULL; + _cleanup_set_free_ Set *settle_ids = NULL; + int r; + + if (running_in_chroot() > 0) { + log_info("Running in chroot, ignoring request."); + return 0; + } + + r = parse_argv(argc, argv); + if (r <= 0) + return r; + + r = sd_device_enumerator_new(&e); + if (r < 0) + return r; + + r = sd_device_enumerator_allow_uninitialized(e); + if (r < 0) + return r; + + r = setup_matches(e); + if (r < 0) + return r; + + if (arg_ping) { + r = udev_ping(arg_ping_timeout_usec, /* ignore_connection_failure = */ false); if (r < 0) return r; assert(r > 0); } - for (; optind < argc; optind++) { - _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - - r = find_device(argv[optind], NULL, &dev); - if (r < 0) - return log_error_errno(r, "Failed to open the device '%s': %m", argv[optind]); - - r = device_enumerator_add_match_parent_incremental(e, dev); - if (r < 0) - return log_error_errno(r, "Failed to add parent match '%s': %m", argv[optind]); - } - if (arg_settle) { r = sd_event_default(&event); if (r < 0) @@ -485,7 +611,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to start device monitor: %m"); } - switch (device_type) { + switch (arg_scan_type) { case TYPE_SUBSYSTEMS: r = device_enumerator_scan_subsystems(e); if (r < 0) @@ -505,7 +631,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { assert_not_reached(); } - r = exec_list(e, action, settle_ids); + r = exec_list(e, arg_action, settle_ids); if (r < 0) return r; From f10206a88ebacb3178eb677acc8021acf4f71c89 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Tue, 29 Apr 2025 14:22:15 +0200 Subject: [PATCH 3/8] udevadm: move key/value parsing func. to udevadm-util.c --- src/udev/udevadm-info.c | 22 ---------------------- src/udev/udevadm-util.c | 22 ++++++++++++++++++++++ src/udev/udevadm-util.h | 1 + 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c index ea66a2ad22..fd2df5b273 100644 --- a/src/udev/udevadm-info.c +++ b/src/udev/udevadm-info.c @@ -875,28 +875,6 @@ static int ensure_device_enumerator(sd_device_enumerator **e) { return 0; } -static int parse_key_value_argument(const char *s, char **key, char **value) { - _cleanup_free_ char *k = NULL, *v = NULL; - int r; - - assert(s); - assert(key); - assert(value); - - r = extract_many_words(&s, "=", EXTRACT_DONT_COALESCE_SEPARATORS, &k, &v); - if (r < 0) - return log_error_errno(r, "Failed to parse key/value pair %s: %m", s); - if (r < 2) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Missing '=' in key/value pair %s.", s); - - if (!filename_is_valid(k)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "%s is not a valid key name", k); - - free_and_replace(*key, k); - free_and_replace(*value, v); - return 0; -} - int info_main(int argc, char *argv[], void *userdata) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; _cleanup_strv_free_ char **devices = NULL; diff --git a/src/udev/udevadm-util.c b/src/udev/udevadm-util.c index 5bede0e4da..0b18e2add9 100644 --- a/src/udev/udevadm-util.c +++ b/src/udev/udevadm-util.c @@ -145,6 +145,28 @@ int parse_resolve_name_timing(const char *str, ResolveNameTiming *ret) { return 1; } +int parse_key_value_argument(const char *s, char **key, char **value) { + _cleanup_free_ char *k = NULL, *v = NULL; + int r; + + assert(s); + assert(key); + assert(value); + + r = extract_many_words(&s, "=", EXTRACT_DONT_COALESCE_SEPARATORS, &k, &v); + if (r < 0) + return log_error_errno(r, "Failed to parse key/value pair %s: %m", s); + if (r < 2) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Missing '=' in key/value pair %s.", s); + + if (!filename_is_valid(k)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "%s is not a valid key name", k); + + free_and_replace(*key, k); + free_and_replace(*value, v); + return 0; +} + static int udev_ping_via_ctrl(usec_t timeout_usec, bool ignore_connection_failure) { _cleanup_(udev_ctrl_unrefp) UdevCtrl *uctrl = NULL; int r; diff --git a/src/udev/udevadm-util.h b/src/udev/udevadm-util.h index f9a3b99fd7..8a615b9339 100644 --- a/src/udev/udevadm-util.h +++ b/src/udev/udevadm-util.h @@ -10,5 +10,6 @@ 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 *ret); int parse_resolve_name_timing(const char *str, ResolveNameTiming *ret); +int parse_key_value_argument(const char *s, char **key, char **value); int udev_ping(usec_t timeout, bool ignore_connection_failure); int search_rules_files(char * const *a, const char *root, char ***ret); From 9925193a768d5b975497988a0d3f180b927c6282 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Tue, 29 Apr 2025 14:29:59 +0200 Subject: [PATCH 4/8] udevadm: allow key without a value --- src/udev/udevadm-info.c | 4 ++-- src/udev/udevadm-util.c | 4 ++-- src/udev/udevadm-util.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c index fd2df5b273..1838d12f5f 100644 --- a/src/udev/udevadm-info.c +++ b/src/udev/udevadm-info.c @@ -1054,7 +1054,7 @@ int info_main(int argc, char *argv[], void *userdata) { if (r < 0) return r; - r = parse_key_value_argument(optarg, &k, &v); + r = parse_key_value_argument(optarg, /* require_value= */ true, &k, &v); if (r < 0) return r; @@ -1072,7 +1072,7 @@ int info_main(int argc, char *argv[], void *userdata) { if (r < 0) return r; - r = parse_key_value_argument(optarg, &k, &v); + r = parse_key_value_argument(optarg, /* require_value= */ true, &k, &v); if (r < 0) return r; diff --git a/src/udev/udevadm-util.c b/src/udev/udevadm-util.c index 0b18e2add9..e3ea2879cf 100644 --- a/src/udev/udevadm-util.c +++ b/src/udev/udevadm-util.c @@ -145,7 +145,7 @@ int parse_resolve_name_timing(const char *str, ResolveNameTiming *ret) { return 1; } -int parse_key_value_argument(const char *s, char **key, char **value) { +int parse_key_value_argument(const char *s, bool require_value, char **key, char **value) { _cleanup_free_ char *k = NULL, *v = NULL; int r; @@ -156,7 +156,7 @@ int parse_key_value_argument(const char *s, char **key, char **value) { r = extract_many_words(&s, "=", EXTRACT_DONT_COALESCE_SEPARATORS, &k, &v); if (r < 0) return log_error_errno(r, "Failed to parse key/value pair %s: %m", s); - if (r < 2) + if (require_value && r < 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Missing '=' in key/value pair %s.", s); if (!filename_is_valid(k)) diff --git a/src/udev/udevadm-util.h b/src/udev/udevadm-util.h index 8a615b9339..c75a2c3930 100644 --- a/src/udev/udevadm-util.h +++ b/src/udev/udevadm-util.h @@ -10,6 +10,6 @@ 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 *ret); int parse_resolve_name_timing(const char *str, ResolveNameTiming *ret); -int parse_key_value_argument(const char *s, char **key, char **value); +int parse_key_value_argument(const char *s, bool require_value, char **key, char **value); int udev_ping(usec_t timeout, bool ignore_connection_failure); int search_rules_files(char * const *a, const char *root, char ***ret); From f849e852214e9a834e2952a9c7f36b1e365506e9 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Tue, 29 Apr 2025 14:34:44 +0200 Subject: [PATCH 5/8] test: extend udevadm coverage test a bit --- test/units/TEST-17-UDEV.sanity-check.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/units/TEST-17-UDEV.sanity-check.sh b/test/units/TEST-17-UDEV.sanity-check.sh index 125a5946f7..fe81bb52bc 100755 --- a/test/units/TEST-17-UDEV.sanity-check.sh +++ b/test/units/TEST-17-UDEV.sanity-check.sh @@ -246,6 +246,10 @@ udevadm trigger -s net udevadm trigger -S net udevadm trigger -a subsystem=net udevadm trigger --attr-match hello=world +udevadm trigger -a subsystem +udevadm trigger -A subsystem=net +udevadm trigger --attr-nomatch hello=world +udevadm trigger -A subsystem udevadm trigger -p DEVNAME=null udevadm trigger --property-match HELLO=world udevadm trigger -g systemd From a07f40e26434287b081f3b7f688062a2ed9a3b1a Mon Sep 17 00:00:00 2001 From: David Tardon Date: Tue, 29 Apr 2025 13:57:10 +0200 Subject: [PATCH 6/8] udevadm-trigger: replace home-made key/value splitting function --- src/udev/udevadm-trigger.c | 46 ++++++++++---------------------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index 5a45a93ac9..b7eed70e70 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -190,25 +190,6 @@ static int device_monitor_handler(sd_device_monitor *m, sd_device *dev, void *us return 0; } -static char* keyval(const char *str, const char **key, const char **val) { - char *buf, *pos; - - buf = strdup(str); - if (!buf) - return NULL; - - pos = strchr(buf, '='); - if (pos) { - pos[0] = 0; - pos++; - } - - *key = buf; - *val = pos; - - return buf; -} - static int add_device_match(sd_device_enumerator *e, const char *s, const char *prefix) { _cleanup_(sd_device_unrefp) sd_device *dev = NULL; int r; @@ -263,12 +244,11 @@ static int setup_matches(sd_device_enumerator *e) { } STRV_FOREACH(a, arg_attr_match) { - _cleanup_free_ char *buf = NULL; - const char *k, *v; + _cleanup_free_ char *k = NULL, *v = NULL; - buf = keyval(*a, &k, &v); - if (!buf) - return log_oom(); + r = parse_key_value_argument(*a, /* require_value= */ false, &k, &v); + if (r < 0) + return r; r = sd_device_enumerator_add_match_sysattr(e, k, v, /* match= */ true); if (r < 0) @@ -276,12 +256,11 @@ static int setup_matches(sd_device_enumerator *e) { } STRV_FOREACH(a, arg_attr_nomatch) { - _cleanup_free_ char *buf = NULL; - const char *k, *v; + _cleanup_free_ char *k = NULL, *v = NULL; - buf = keyval(*a, &k, &v); - if (!buf) - return log_oom(); + r = parse_key_value_argument(*a, /* require_value= */ false, &k, &v); + if (r < 0) + return r; r = sd_device_enumerator_add_match_sysattr(e, k, v, /* match= */ false); if (r < 0) @@ -289,12 +268,11 @@ static int setup_matches(sd_device_enumerator *e) { } STRV_FOREACH(p, arg_property_match) { - _cleanup_free_ char *buf = NULL; - const char *k, *v; + _cleanup_free_ char *k = NULL, *v = NULL; - buf = keyval(*p, &k, &v); - if (!buf) - return log_oom(); + r = parse_key_value_argument(*p, /* require_value= */ true, &k, &v); + if (r < 0) + return r; r = sd_device_enumerator_add_match_property(e, k, v); if (r < 0) From bc995d6764091b29e851ed72f21874f93b11455b Mon Sep 17 00:00:00 2001 From: David Tardon Date: Tue, 29 Apr 2025 14:02:37 +0200 Subject: [PATCH 7/8] udevadm-trigger: add prefix to enumeration values --- src/udev/udevadm-trigger.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index b7eed70e70..9163824bef 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -24,9 +24,9 @@ #include "virt.h" typedef enum { - TYPE_DEVICES, - TYPE_SUBSYSTEMS, - TYPE_ALL, + SCAN_TYPE_DEVICES, + SCAN_TYPE_SUBSYSTEMS, + SCAN_TYPE_ALL, } ScanType; static bool arg_verbose = false; @@ -34,7 +34,7 @@ static bool arg_dry_run = false; static bool arg_quiet = false; static bool arg_uuid = false; static bool arg_settle = false; -static ScanType arg_scan_type = TYPE_DEVICES; +static ScanType arg_scan_type = SCAN_TYPE_DEVICES; static sd_device_action_t arg_action = SD_DEVICE_CHANGE; static char **arg_devices = NULL; static char **arg_attr_match = NULL; @@ -407,11 +407,11 @@ static int parse_argv(int argc, char *argv[]) { case 't': if (streq(optarg, "devices")) - arg_scan_type = TYPE_DEVICES; + arg_scan_type = SCAN_TYPE_DEVICES; else if (streq(optarg, "subsystems")) - arg_scan_type = TYPE_SUBSYSTEMS; + arg_scan_type = SCAN_TYPE_SUBSYSTEMS; else if (streq(optarg, "all")) - arg_scan_type = TYPE_ALL; + arg_scan_type = SCAN_TYPE_ALL; else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown type --type=%s", optarg); break; @@ -590,17 +590,17 @@ int trigger_main(int argc, char *argv[], void *userdata) { } switch (arg_scan_type) { - case TYPE_SUBSYSTEMS: + case SCAN_TYPE_SUBSYSTEMS: r = device_enumerator_scan_subsystems(e); if (r < 0) return log_error_errno(r, "Failed to scan subsystems: %m"); break; - case TYPE_DEVICES: + case SCAN_TYPE_DEVICES: r = device_enumerator_scan_devices(e); if (r < 0) return log_error_errno(r, "Failed to scan devices: %m"); break; - case TYPE_ALL: + case SCAN_TYPE_ALL: r = device_enumerator_scan_devices_and_subsystems(e); if (r < 0) return log_error_errno(r, "Failed to scan devices and subsystems: %m"); From a63822a7e338014e3305699a85104777e9272d6d Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 7 May 2025 16:11:38 +0200 Subject: [PATCH 8/8] udevadm-trigger: use string table to parse scan type --- src/udev/udevadm-trigger.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index 9163824bef..94a3a4037a 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -17,6 +17,7 @@ #include "process-util.h" #include "set.h" #include "static-destruct.h" +#include "string-table.h" #include "string-util.h" #include "strv.h" #include "udevadm.h" @@ -27,6 +28,8 @@ typedef enum { SCAN_TYPE_DEVICES, SCAN_TYPE_SUBSYSTEMS, SCAN_TYPE_ALL, + _SCAN_TYPE_MAX, + _SCAN_TYPE_INVALID = -EINVAL, } ScanType; static bool arg_verbose = false; @@ -64,6 +67,14 @@ STATIC_DESTRUCTOR_REGISTER(arg_sysname_match, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_tag_match, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_prioritized_subsystems, strv_freep); +static const char *scan_type_table[_SCAN_TYPE_MAX] = { + [SCAN_TYPE_DEVICES] = "devices", + [SCAN_TYPE_SUBSYSTEMS] = "subsystems", + [SCAN_TYPE_ALL] = "all", +}; + +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(scan_type, ScanType); + static int exec_list( sd_device_enumerator *e, sd_device_action_t action, @@ -406,13 +417,8 @@ static int parse_argv(int argc, char *argv[]) { break; case 't': - if (streq(optarg, "devices")) - arg_scan_type = SCAN_TYPE_DEVICES; - else if (streq(optarg, "subsystems")) - arg_scan_type = SCAN_TYPE_SUBSYSTEMS; - else if (streq(optarg, "all")) - arg_scan_type = SCAN_TYPE_ALL; - else + arg_scan_type = scan_type_from_string(optarg); + if (arg_scan_type < 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown type --type=%s", optarg); break;