From 216e8bbe342160af60fa20a70b95c1bef8639695 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Oct 2018 14:51:58 +0900 Subject: [PATCH 1/9] udevd: explicitly set default value of global variables --- src/udev/udevd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 2664c8475e..ecec6cafb8 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -62,8 +62,8 @@ static bool arg_debug = false; static int arg_daemonize = false; static int arg_resolve_names = 1; -static unsigned arg_children_max; -static int arg_exec_delay; +static unsigned arg_children_max = 0; +static int arg_exec_delay = 0; static usec_t arg_event_timeout_usec = 180 * USEC_PER_SEC; static usec_t arg_event_timeout_warn_usec = 180 * USEC_PER_SEC / 3; From 6b92f42934d0198253c11cedd7dc3239ecaa5bf0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Oct 2018 14:56:12 +0900 Subject: [PATCH 2/9] udevd: use parse_sec() to parse --exec-delay option --- src/udev/udev-event.c | 8 ++++---- src/udev/udev.h | 4 ++-- src/udev/udevd.c | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index c1d0b3662b..d5666c1079 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -42,7 +42,7 @@ typedef struct Spawn { size_t result_len; } Spawn; -struct udev_event *udev_event_new(sd_device *dev, int exec_delay, sd_netlink *rtnl) { +struct udev_event *udev_event_new(sd_device *dev, usec_t exec_delay_usec, sd_netlink *rtnl) { struct udev_event *event; assert(dev); @@ -54,7 +54,7 @@ struct udev_event *udev_event_new(sd_device *dev, int exec_delay, sd_netlink *rt *event = (struct udev_event) { .dev = sd_device_ref(dev), .birth_usec = now(CLOCK_MONOTONIC), - .exec_delay = exec_delay, + .exec_delay_usec = exec_delay_usec, .rtnl = sd_netlink_ref(rtnl), }; @@ -896,9 +896,9 @@ void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, usec_ if (builtin_cmd >= 0 && builtin_cmd < _UDEV_BUILTIN_MAX) udev_builtin_run(event->dev, builtin_cmd, command, false); else { - if (event->exec_delay > 0) { + if (event->exec_delay_usec > 0) { log_debug("delay execution of '%s'", command); - sleep(event->exec_delay); + (void) usleep(event->exec_delay_usec); } udev_event_spawn(event, timeout_usec, timeout_warn_usec, false, command, NULL, 0); diff --git a/src/udev/udev.h b/src/udev/udev.h index 162859ab40..fb2c6813ef 100644 --- a/src/udev/udev.h +++ b/src/udev/udev.h @@ -30,7 +30,7 @@ struct udev_event { gid_t gid; Hashmap *seclabel_list; Hashmap *run_list; - int exec_delay; + usec_t exec_delay_usec; usec_t birth_usec; sd_netlink *rtnl; unsigned builtin_run; @@ -59,7 +59,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event int udev_rules_apply_static_dev_perms(struct udev_rules *rules); /* udev-event.c */ -struct udev_event *udev_event_new(sd_device *dev, int exec_delay, sd_netlink *rtnl); +struct udev_event *udev_event_new(sd_device *dev, usec_t exec_delay_usec, sd_netlink *rtnl); struct udev_event *udev_event_free(struct udev_event *event); ssize_t udev_event_apply_format(struct udev_event *event, const char *src, char *dest, size_t size, diff --git a/src/udev/udevd.c b/src/udev/udevd.c index ecec6cafb8..c97e873942 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -63,7 +63,7 @@ static bool arg_debug = false; static int arg_daemonize = false; static int arg_resolve_names = 1; static unsigned arg_children_max = 0; -static int arg_exec_delay = 0; +static usec_t arg_exec_delay_usec = 0; static usec_t arg_event_timeout_usec = 180 * USEC_PER_SEC; static usec_t arg_event_timeout_warn_usec = 180 * USEC_PER_SEC / 3; @@ -409,7 +409,7 @@ static void worker_spawn(Manager *manager, struct event *event) { assert(dev); log_debug("seq %llu running", udev_device_get_seqnum(dev)); - udev_event = udev_event_new(dev->device, arg_exec_delay, rtnl); + udev_event = udev_event_new(dev->device, arg_exec_delay_usec, rtnl); if (!udev_event) { r = -ENOMEM; goto out; @@ -1479,7 +1479,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat if (proc_cmdline_value_missing(key, value)) return 0; - r = safe_atoi(value, &arg_exec_delay); + r = parse_sec(value, &arg_exec_delay_usec); } else if (startswith(key, "udev.")) log_warning("Unknown udev kernel command line option \"%s\"", key); @@ -1549,9 +1549,9 @@ static int parse_argv(int argc, char *argv[]) { log_warning("Invalid --children-max ignored: %s", optarg); break; case 'e': - r = safe_atoi(optarg, &arg_exec_delay); + r = parse_sec(optarg, &arg_exec_delay_usec); if (r < 0) - log_warning("Invalid --exec-delay ignored: %s", optarg); + log_warning_errno(r, "Failed to parse --exec-delay= value '%s', ignoring: %m", optarg); break; case 't': r = safe_atou64(optarg, &arg_event_timeout_usec); From c4d44cba4d9bd9d92c86e06f21d5936cca1b8c16 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Oct 2018 15:30:51 +0900 Subject: [PATCH 3/9] udev: introduce enum ResolveNameTiming for --resolve-names argument --- src/test/test-udev.c | 2 +- src/udev/udev-rules.c | 25 ++++++++++++++++++------- src/udev/udev.h | 13 ++++++++++++- src/udev/udevadm-test.c | 15 +++++---------- src/udev/udevd.c | 26 ++++++++++++-------------- 5 files changed, 48 insertions(+), 33 deletions(-) diff --git a/src/test/test-udev.c b/src/test/test-udev.c index 102da4adc6..7c70336e1b 100644 --- a/src/test/test-udev.c +++ b/src/test/test-udev.c @@ -84,7 +84,7 @@ int main(int argc, char *argv[]) { action = argv[1]; devpath = argv[2]; - rules = udev_rules_new(1); + rules = udev_rules_new(RESOLVE_NAME_EARLY); const char *syspath = strjoina("/sys", devpath); r = device_new_from_synthetic_event(&dev, syspath, action); diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index ad4b32abea..c8b905a873 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -30,6 +30,7 @@ #include "stat-util.h" #include "stdio-util.h" #include "strbuf.h" +#include "string-table.h" #include "string-util.h" #include "strv.h" #include "sysctl-util.h" @@ -57,7 +58,7 @@ static const char* const rules_dirs[] = { struct udev_rules { usec_t dirs_ts_usec; - int resolve_names; + ResolveNameTiming resolve_name_timing; /* every key in the rules file becomes a token */ struct token *tokens; @@ -1335,10 +1336,10 @@ static void add_rule(struct udev_rules *rules, char *line, uid = strtoul(value, &endptr, 10); if (endptr[0] == '\0') rule_add_key(&rule_tmp, TK_A_OWNER_ID, op, NULL, &uid); - else if (rules->resolve_names > 0 && strchr("$%", value[0]) == NULL) { + else if (rules->resolve_name_timing == RESOLVE_NAME_EARLY && strchr("$%", value[0]) == NULL) { uid = add_uid(rules, value); rule_add_key(&rule_tmp, TK_A_OWNER_ID, op, NULL, &uid); - } else if (rules->resolve_names >= 0) + } else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER) rule_add_key(&rule_tmp, TK_A_OWNER, op, value, NULL); rule_tmp.rule.rule.can_set_name = true; @@ -1353,10 +1354,10 @@ static void add_rule(struct udev_rules *rules, char *line, gid = strtoul(value, &endptr, 10); if (endptr[0] == '\0') rule_add_key(&rule_tmp, TK_A_GROUP_ID, op, NULL, &gid); - else if ((rules->resolve_names > 0) && strchr("$%", value[0]) == NULL) { + else if ((rules->resolve_name_timing == RESOLVE_NAME_EARLY) && strchr("$%", value[0]) == NULL) { gid = add_gid(rules, value); rule_add_key(&rule_tmp, TK_A_GROUP_ID, op, NULL, &gid); - } else if (rules->resolve_names >= 0) + } else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER) rule_add_key(&rule_tmp, TK_A_GROUP, op, value, NULL); rule_tmp.rule.rule.can_set_name = true; @@ -1512,18 +1513,20 @@ static int parse_file(struct udev_rules *rules, const char *filename) { return 0; } -struct udev_rules *udev_rules_new(int resolve_names) { +struct udev_rules *udev_rules_new(ResolveNameTiming resolve_name_timing) { struct udev_rules *rules; struct token end_token; char **files, **f; int r; + assert(resolve_name_timing >= 0 && resolve_name_timing < _RESOLVE_NAME_TIMING_MAX); + rules = new(struct udev_rules, 1); if (!rules) return NULL; *rules = (struct udev_rules) { - .resolve_names = resolve_names, + .resolve_name_timing = resolve_name_timing, }; /* init token array and string buffer */ @@ -2598,3 +2601,11 @@ finish: return 0; } + +static const char* const resolve_name_timing_table[_RESOLVE_NAME_TIMING_MAX] = { + [RESOLVE_NAME_NEVER] = "never", + [RESOLVE_NAME_LATE] = "late", + [RESOLVE_NAME_EARLY] = "early", +}; + +DEFINE_STRING_TABLE_LOOKUP(resolve_name_timing, ResolveNameTiming); diff --git a/src/udev/udev.h b/src/udev/udev.h index fb2c6813ef..5c6ca6bf9f 100644 --- a/src/udev/udev.h +++ b/src/udev/udev.h @@ -48,9 +48,17 @@ struct udev_event { bool run_final; }; +typedef enum ResolveNameTiming { + RESOLVE_NAME_NEVER, + RESOLVE_NAME_LATE, + RESOLVE_NAME_EARLY, + _RESOLVE_NAME_TIMING_MAX, + _RESOLVE_NAME_TIMING_INVALID = -1, +} ResolveNameTiming; + /* udev-rules.c */ struct udev_rules; -struct udev_rules *udev_rules_new(int resolve_names); +struct udev_rules *udev_rules_new(ResolveNameTiming resolve_name_timing); struct udev_rules *udev_rules_unref(struct udev_rules *rules); bool udev_rules_check_timestamp(struct udev_rules *rules); int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event, @@ -58,6 +66,9 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event Hashmap *properties_list); int udev_rules_apply_static_dev_perms(struct udev_rules *rules); +ResolveNameTiming resolve_name_timing_from_string(const char *s) _pure_; +const char *resolve_name_timing_to_string(ResolveNameTiming i) _const_; + /* udev-event.c */ struct udev_event *udev_event_new(sd_device *dev, usec_t exec_delay_usec, sd_netlink *rtnl); struct udev_event *udev_event_free(struct udev_event *event); diff --git a/src/udev/udevadm-test.c b/src/udev/udevadm-test.c index cfaaf03db9..dd0fc4f91f 100644 --- a/src/udev/udevadm-test.c +++ b/src/udev/udevadm-test.c @@ -22,7 +22,7 @@ #include "udevadm.h" static const char *arg_action = "add"; -static int arg_resolve_names = 1; +static ResolveNameTiming arg_resolve_name_timing = RESOLVE_NAME_EARLY; static char arg_syspath[UTIL_PATH_SIZE] = {}; static int help(void) { @@ -55,14 +55,9 @@ static int parse_argv(int argc, char *argv[]) { arg_action = optarg; break; case 'N': - if (streq (optarg, "early")) { - arg_resolve_names = 1; - } else if (streq (optarg, "late")) { - arg_resolve_names = 0; - } else if (streq (optarg, "never")) { - arg_resolve_names = -1; - } else { - log_error("resolve-names must be early, late or never"); + arg_resolve_name_timing = resolve_name_timing_from_string(optarg); + if (arg_resolve_name_timing < 0) { + log_error("--resolve-names= must be early, late or never"); return -EINVAL; } break; @@ -115,7 +110,7 @@ int test_main(int argc, char *argv[], void *userdata) { udev_builtin_init(); - rules = udev_rules_new(arg_resolve_names); + rules = udev_rules_new(arg_resolve_name_timing); if (!rules) { log_error("Failed to read udev rules."); r = -ENOMEM; diff --git a/src/udev/udevd.c b/src/udev/udevd.c index c97e873942..4f93ec4c9a 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -61,7 +61,7 @@ static bool arg_debug = false; static int arg_daemonize = false; -static int arg_resolve_names = 1; +static ResolveNameTiming arg_resolve_name_timing = RESOLVE_NAME_EARLY; static unsigned arg_children_max = 0; static usec_t arg_exec_delay_usec = 0; static usec_t arg_event_timeout_usec = 180 * USEC_PER_SEC; @@ -851,7 +851,7 @@ static void event_queue_start(Manager *manager) { udev_builtin_init(); if (!manager->rules) { - manager->rules = udev_rules_new(arg_resolve_names); + manager->rules = udev_rules_new(arg_resolve_name_timing); if (!manager->rules) return; } @@ -1565,18 +1565,16 @@ static int parse_argv(int argc, char *argv[]) { case 'D': arg_debug = true; break; - case 'N': - if (streq(optarg, "early")) { - arg_resolve_names = 1; - } else if (streq(optarg, "late")) { - arg_resolve_names = 0; - } else if (streq(optarg, "never")) { - arg_resolve_names = -1; - } else { - log_error("resolve-names must be early, late or never"); - return 0; - } + case 'N': { + ResolveNameTiming t; + + t = resolve_name_timing_from_string(optarg); + if (t < 0) + log_warning("Invalid --resolve-names= value '%s', ignoring.", optarg); + else + arg_resolve_name_timing = t; break; + } case 'h': return help(); case 'V': @@ -1611,7 +1609,7 @@ static int manager_new(Manager **ret, int fd_ctrl, int fd_uevent, const char *cg udev_builtin_init(); - manager->rules = udev_rules_new(arg_resolve_names); + manager->rules = udev_rules_new(arg_resolve_name_timing); if (!manager->rules) return log_error_errno(ENOMEM, "error reading rules"); From 389f9bf2cf32df8235959a3693e0f9b3379d53ec Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Oct 2018 18:13:55 +0900 Subject: [PATCH 4/9] udev: include error cause of parsing --children-max option in log message --- src/udev/udevd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 4f93ec4c9a..2564295ef0 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1546,7 +1546,7 @@ static int parse_argv(int argc, char *argv[]) { case 'c': r = safe_atou(optarg, &arg_children_max); if (r < 0) - log_warning("Invalid --children-max ignored: %s", optarg); + log_warning_errno(r, "Failed to parse --children-max= value '%s', ignoring: %m", optarg); break; case 'e': r = parse_sec(optarg, &arg_exec_delay_usec); From 9d9264ba39f797d20100c8acfda3df895ab5aaa2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Oct 2018 18:06:44 +0900 Subject: [PATCH 5/9] udev: use parse_sec() to parse --event-timeout option --- src/udev/udevd.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 2564295ef0..12b3298a06 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1461,11 +1461,9 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat if (proc_cmdline_value_missing(key, value)) return 0; - r = safe_atou64(value, &arg_event_timeout_usec); - if (r >= 0) { - arg_event_timeout_usec *= USEC_PER_SEC; - arg_event_timeout_warn_usec = (arg_event_timeout_usec / 3) ? : 1; - } + r = parse_sec(value, &arg_event_timeout_usec); + if (r >= 0) + arg_event_timeout_warn_usec = DIV_ROUND_UP(arg_event_timeout_usec, 3); } else if (proc_cmdline_key_streq(key, "udev.children_max")) { @@ -1554,13 +1552,11 @@ static int parse_argv(int argc, char *argv[]) { log_warning_errno(r, "Failed to parse --exec-delay= value '%s', ignoring: %m", optarg); break; case 't': - r = safe_atou64(optarg, &arg_event_timeout_usec); + r = parse_sec(optarg, &arg_event_timeout_usec); if (r < 0) - log_warning("Invalid --event-timeout ignored: %s", optarg); - else { - arg_event_timeout_usec *= USEC_PER_SEC; - arg_event_timeout_warn_usec = (arg_event_timeout_usec / 3) ? : 1; - } + log_warning_errno(r, "Failed to parse --event-timeout= value '%s', ignoring: %m", optarg); + + arg_event_timeout_warn_usec = DIV_ROUND_UP(arg_event_timeout_usec, 3); break; case 'D': arg_debug = true; From 46f0fbd8fd8ed1428b6f038b88375935e722eeaa Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 29 Oct 2018 16:50:39 +0900 Subject: [PATCH 6/9] udev: drop util_log_priority() and use log_level_from_string() The function util_log_priority() is almost same as log_level_from_string(). The difference between them is only that util_log_priority() accepts such that '3 hogehoge'. --- src/libudev/libudev-private.h | 1 - src/libudev/libudev-util.c | 16 ---------------- src/udev/udevadm-control.c | 15 ++++++--------- src/udev/udevd.c | 3 ++- 4 files changed, 8 insertions(+), 27 deletions(-) diff --git a/src/libudev/libudev-private.h b/src/libudev/libudev-private.h index bf06e3add6..fdf87acf9e 100644 --- a/src/libudev/libudev-private.h +++ b/src/libudev/libudev-private.h @@ -99,7 +99,6 @@ void udev_list_entry_set_num(struct udev_list_entry *list_entry, int num); #define UTIL_NAME_SIZE 512 #define UTIL_LINE_SIZE 16384 #define UDEV_ALLOWED_CHARS_INPUT "/ $%?," -int util_log_priority(const char *priority); size_t util_path_encode(const char *src, char *dest, size_t size); int util_replace_whitespace(const char *str, char *to, size_t len); int util_replace_chars(char *str, const char *white); diff --git a/src/libudev/libudev-util.c b/src/libudev/libudev-util.c index 3bb27fbc4e..df5223e5a9 100644 --- a/src/libudev/libudev-util.c +++ b/src/libudev/libudev-util.c @@ -12,7 +12,6 @@ #include "MurmurHash2.h" #include "device-nodes.h" #include "libudev-private.h" -#include "syslog-util.h" #include "utf8.h" /** @@ -84,21 +83,6 @@ int util_resolve_subsys_kernel(const char *string, return 0; } -int util_log_priority(const char *priority) { - char *endptr; - int prio; - - prio = strtoul(priority, &endptr, 10); - if (endptr[0] == '\0' || isspace(endptr[0])) { - if (prio >= 0 && prio <= 7) - return prio; - else - return -ERANGE; - } - - return log_level_from_string(priority); -} - size_t util_path_encode(const char *src, char *dest, size_t size) { size_t i, j; diff --git a/src/udev/udevadm-control.c b/src/udev/udevadm-control.c index 22ac4962ed..d9320418cf 100644 --- a/src/udev/udevadm-control.c +++ b/src/udev/udevadm-control.c @@ -19,9 +19,9 @@ #include #include -#include "libudev-private.h" #include "parse-util.h" #include "process-util.h" +#include "syslog-util.h" #include "time-util.h" #include "udevadm.h" #include "udev-ctrl.h" @@ -84,18 +84,15 @@ int control_main(int argc, char *argv[], void *userdata) { if (r < 0) return r; break; - case 'l': { - int i; + case 'l': + r = log_level_from_string(optarg); + if (r < 0) + return log_error_errno(r, "Failed to parse log priority '%s': %m", optarg); - i = util_log_priority(optarg); - if (i < 0) - return log_error_errno(i, "invalid number '%s'", optarg); - - r = udev_ctrl_send_set_log_level(uctrl, i, timeout); + r = udev_ctrl_send_set_log_level(uctrl, r, timeout); if (r < 0) return r; break; - } case 's': r = udev_ctrl_send_stop_exec_queue(uctrl, timeout); if (r < 0) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 12b3298a06..1a643d5309 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -51,6 +51,7 @@ #include "signal-util.h" #include "socket-util.h" #include "string-util.h" +#include "syslog-util.h" #include "terminal-util.h" #include "udev-builtin.h" #include "udev-ctrl.h" @@ -1452,7 +1453,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat if (proc_cmdline_value_missing(key, value)) return 0; - r = util_log_priority(value); + r = log_level_from_string(value); if (r >= 0) log_set_max_level(r); From c52cff074831c6ca76f20f9c972283c78d8843be Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Oct 2018 17:39:53 +0900 Subject: [PATCH 7/9] udev: handle sd_is_socket() failure --- src/udev/udevd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 1a643d5309..cc3fdd8593 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1368,14 +1368,14 @@ static int listen_fds(int *rctrl, int *rnetlink) { return n; for (fd = SD_LISTEN_FDS_START; fd < n + SD_LISTEN_FDS_START; fd++) { - if (sd_is_socket(fd, AF_LOCAL, SOCK_SEQPACKET, -1)) { + if (sd_is_socket(fd, AF_LOCAL, SOCK_SEQPACKET, -1) > 0) { if (ctrl_fd >= 0) return -EINVAL; ctrl_fd = fd; continue; } - if (sd_is_socket(fd, AF_NETLINK, SOCK_RAW, -1)) { + if (sd_is_socket(fd, AF_NETLINK, SOCK_RAW, -1) > 0) { if (netlink_fd >= 0) return -EINVAL; netlink_fd = fd; From c4b69e990f962128cc6975e36e91e9ad838fa2c4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Oct 2018 17:41:17 +0900 Subject: [PATCH 8/9] udev: drop redundant initializations for file descriptors As udev_ctrl_new_from_fd() or udev_monitor_new_from_netlink_fd() creates fd if negative fd is passed. --- src/udev/udevd.c | 74 ++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 50 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index cc3fdd8593..922542f0f6 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1356,12 +1356,12 @@ static int on_post(sd_event_source *s, void *userdata) { return 1; } -static int listen_fds(int *rctrl, int *rnetlink) { +static int listen_fds(int *ret_ctrl, int *ret_netlink) { int ctrl_fd = -1, netlink_fd = -1; - int fd, n, r; + int fd, n; - assert(rctrl); - assert(rnetlink); + assert(ret_ctrl); + assert(ret_netlink); n = sd_listen_fds(true); if (n < 0) @@ -1385,50 +1385,8 @@ static int listen_fds(int *rctrl, int *rnetlink) { return -EINVAL; } - if (ctrl_fd < 0) { - _cleanup_(udev_ctrl_unrefp) struct udev_ctrl *ctrl = NULL; - - ctrl = udev_ctrl_new(); - if (!ctrl) - return log_error_errno(EINVAL, "error initializing udev control socket"); - - r = udev_ctrl_enable_receiving(ctrl); - if (r < 0) - return log_error_errno(EINVAL, "error binding udev control socket"); - - fd = udev_ctrl_get_fd(ctrl); - if (fd < 0) - return log_error_errno(EIO, "could not get ctrl fd"); - - ctrl_fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); - if (ctrl_fd < 0) - return log_error_errno(errno, "could not dup ctrl fd: %m"); - } - - if (netlink_fd < 0) { - _cleanup_(udev_monitor_unrefp) struct udev_monitor *monitor = NULL; - - monitor = udev_monitor_new_from_netlink(NULL, "kernel"); - if (!monitor) - return log_error_errno(EINVAL, "error initializing netlink socket"); - - (void) udev_monitor_set_receive_buffer_size(monitor, 128 * 1024 * 1024); - - r = udev_monitor_enable_receiving(monitor); - if (r < 0) - return log_error_errno(EINVAL, "error binding netlink socket"); - - fd = udev_monitor_get_fd(monitor); - if (fd < 0) - return log_error_errno(netlink_fd, "could not get uevent fd: %m"); - - netlink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 3); - if (netlink_fd < 0) - return log_error_errno(errno, "could not dup netlink fd: %m"); - } - - *rctrl = ctrl_fd; - *rnetlink = netlink_fd; + *ret_ctrl = ctrl_fd; + *ret_netlink = netlink_fd; return 0; } @@ -1593,8 +1551,6 @@ static int manager_new(Manager **ret, int fd_ctrl, int fd_uevent, const char *cg int r, fd_worker; assert(ret); - assert(fd_ctrl >= 0); - assert(fd_uevent >= 0); manager = new0(Manager, 1); if (!manager) @@ -1618,10 +1574,28 @@ static int manager_new(Manager **ret, int fd_ctrl, int fd_uevent, const char *cg if (!manager->ctrl) return log_error_errno(EINVAL, "error taking over udev control socket"); + r = udev_ctrl_enable_receiving(manager->ctrl); + if (r < 0) + return log_error_errno(r, "Failed to bind udev control socket: %m"); + + fd_ctrl = udev_ctrl_get_fd(manager->ctrl); + if (fd_ctrl < 0) + return log_error_errno(fd_ctrl, "Failed to get udev control fd: %m"); + manager->monitor = udev_monitor_new_from_netlink_fd(NULL, "kernel", fd_uevent); if (!manager->monitor) return log_error_errno(EINVAL, "error taking over netlink socket"); + (void) udev_monitor_set_receive_buffer_size(manager->monitor, 128 * 1024 * 1024); + + r = udev_monitor_enable_receiving(manager->monitor); + if (r < 0) + return log_error_errno(r, "Failed to bind netlink socket; %m"); + + fd_uevent = udev_monitor_get_fd(manager->monitor); + if (fd_uevent < 0) + return log_error_errno(fd_uevent, "Failed to get uevent fd: %m"); + /* unnamed socket from workers to the main daemon */ r = socketpair(AF_LOCAL, SOCK_DGRAM|SOCK_CLOEXEC, 0, manager->worker_watch); if (r < 0) From 6f19b42f24b7aea227094c6d06b1cef8053ce8cb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Oct 2018 18:18:35 +0900 Subject: [PATCH 9/9] udev: use structured initializer at one more place --- src/udev/udevd.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 922542f0f6..05030dc81e 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1552,13 +1552,15 @@ static int manager_new(Manager **ret, int fd_ctrl, int fd_uevent, const char *cg assert(ret); - manager = new0(Manager, 1); + manager = new(Manager, 1); if (!manager) return log_oom(); - manager->fd_inotify = -1; - manager->worker_watch[WRITE_END] = -1; - manager->worker_watch[READ_END] = -1; + *manager = (Manager) { + .fd_inotify = -1, + .worker_watch = { -1, -1 }, + .cgroup = cgroup, + }; udev_builtin_init(); @@ -1566,10 +1568,6 @@ static int manager_new(Manager **ret, int fd_ctrl, int fd_uevent, const char *cg if (!manager->rules) return log_error_errno(ENOMEM, "error reading rules"); - LIST_HEAD_INIT(manager->events); - - manager->cgroup = cgroup; - manager->ctrl = udev_ctrl_new_from_fd(fd_ctrl); if (!manager->ctrl) return log_error_errno(EINVAL, "error taking over udev control socket");