Merge pull request #10603 from yuwata/udevd-parser

udevd: several cleanups for parsing options

6 files changed, 107 insertions, 126 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-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 162859ab40..5c6ca6bf9f 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;
@@ -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,8 +66,11 @@ 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, 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/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 <string.h>
 #include <unistd.h>
 
-#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;
-
-                        i = util_log_priority(optarg);
-                        if (i < 0)
-                                return log_error_errno(i, "invalid number '%s'", optarg);
+                case 'l':
+                        r = log_level_from_string(optarg);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to parse log priority '%s': %m", 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/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 2664c8475e..05030dc81e 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"
@@ -61,9 +62,9 @@
 
 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 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;
 static usec_t arg_event_timeout_warn_usec = 180 * USEC_PER_SEC / 3;
 
@@ -409,7 +410,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;
@@ -851,7 +852,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;
         }
@@ -1355,26 +1356,26 @@ 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)
                 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;
@@ -1384,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;
 }
@@ -1452,7 +1411,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);
 
@@ -1461,11 +1420,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")) {
 
@@ -1479,7 +1436,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);
@@ -1546,37 +1503,33 @@ 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 = 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);
+                        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;
                         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':
@@ -1598,35 +1551,49 @@ 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);
+        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();
 
-        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");
 
-        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");
 
+        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)