[systemd-commits] 12 commits - src/libudev src/test src/udev

Tom Gundersen tomegun at kemper.freedesktop.org
Tue Sep 16 03:14:22 PDT 2014


 src/libudev/libudev-private.h |    3 
 src/libudev/libudev-util.c    |  105 ---------------
 src/test/test-udev.c          |    2 
 src/udev/udev-node.c          |    3 
 src/udev/udev-rules.c         |   49 ++++++-
 src/udev/udevd.c              |  286 +++++++++++++++++++++---------------------
 6 files changed, 188 insertions(+), 260 deletions(-)

New commits:
commit 85639427b3a3014adf934ee94c86d91f3aaf4cfb
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Sep 15 16:36:07 2014 +0200

    udev: rules - add missing whitespace to log message

diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
index ce4d173..2a691f6 100644
--- a/src/udev/udev-rules.c
+++ b/src/udev/udev-rules.c
@@ -1072,8 +1072,7 @@ static int add_rule(struct udev_rules *rules, char *line,
                                 _cleanup_free_ char *tmp;
 
                                 tmp = cescape(buf);
-                                log_error("invalid key/value pair in file %s on line %u,"
-                                          "starting at character %tu ('%s')\n",
+                                log_error("invalid key/value pair in file %s on line %u, starting at character %tu ('%s')\n",
                                           filename, lineno, linepos - line + 1, tmp);
                                 if (linepos[1] == '#')
                                         log_error("hint: comments can only start at beginning of line");

commit f1e8664e4a86f9b9b8d8a001d886d69f1ac42e9b
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Sep 15 14:41:30 2014 +0200

    udevd: use safe_ato*() in place of strto*()

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index e8b3602..cd51793 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -986,11 +986,20 @@ static void kernel_cmdline_options(struct udev *udev) {
                         log_set_max_level(prio);
                         udev_set_log_priority(udev, prio);
                 } else if (startswith(opt, "udev.children-max=")) {
-                        arg_children_max = strtoul(opt + 18, NULL, 0);
+                        r = safe_atoi(opt + 18, &arg_children_max);
+                        if (r < 0)
+                                log_warning("Invalid udev.children-max ignored: %s", opt + 18);
                 } else if (startswith(opt, "udev.exec-delay=")) {
-                        arg_exec_delay = strtoul(opt + 16, NULL, 0);
+                        r = safe_atoi(opt + 16, &arg_exec_delay);
+                        if (r < 0)
+                                log_warning("Invalid udev.exec-delay ignored: %s", opt + 16);
                 } else if (startswith(opt, "udev.event-timeout=")) {
-                        arg_event_timeout_usec = strtoul(opt + 16, NULL, 0) * USEC_PER_SEC;
+                        r = safe_atou64(opt + 16, &arg_event_timeout_usec);
+                        if (r < 0) {
+                                log_warning("Invalid udev.event-timeout ignored: %s", opt + 16);
+                                break;
+                        }
+                        arg_event_timeout_usec *= USEC_PER_SEC;
                         arg_event_timeout_warn_usec = (arg_event_timeout_usec / 3) ? : 1;
                 }
 
@@ -1031,6 +1040,7 @@ static int parse_argv(int argc, char *argv[]) {
         assert(argv);
 
         while ((c = getopt_long(argc, argv, "c:de:DtN:hV", options, NULL)) >= 0) {
+                int r;
 
                 switch (c) {
 
@@ -1038,13 +1048,16 @@ static int parse_argv(int argc, char *argv[]) {
                         arg_daemonize = true;
                         break;
                 case 'c':
-                        arg_children_max = strtoul(optarg, NULL, 0);
+                        safe_atoi(optarg, &arg_children_max);
                         break;
                 case 'e':
-                        arg_exec_delay = strtoul(optarg, NULL, 0);
+                        safe_atoi(optarg, &arg_exec_delay);
                         break;
                 case 't':
-                        arg_event_timeout_usec = strtoul(optarg, NULL, 0) * USEC_PER_SEC;
+                        r = safe_atou64(optarg, &arg_event_timeout_usec);
+                        if (r < 0)
+                                break;
+                        arg_event_timeout_usec *= USEC_PER_SEC;
                         arg_event_timeout_warn_usec = (arg_event_timeout_usec / 3) ? : 1;
                         break;
                 case 'D':

commit ba7408a6e9fa2f79f7e69052de78f82e12c613f6
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Sep 15 14:21:00 2014 +0200

    udev: util - use log_level_from_string()

diff --git a/src/libudev/libudev-util.c b/src/libudev/libudev-util.c
index a7125fa..f3fdf3b 100644
--- a/src/libudev/libudev-util.c
+++ b/src/libudev/libudev-util.c
@@ -162,13 +162,8 @@ int util_log_priority(const char *priority)
         prio = strtol(priority, &endptr, 10);
         if (endptr[0] == '\0' || isspace(endptr[0]))
                 return prio;
-        if (startswith(priority, "err"))
-                return LOG_ERR;
-        if (startswith(priority, "info"))
-                return LOG_INFO;
-        if (startswith(priority, "debug"))
-                return LOG_DEBUG;
-        return 0;
+
+        return log_level_from_string(priority);
 }
 
 size_t util_path_encode(const char *src, char *dest, size_t size)

commit 37d522746b67fda0d52111364d81358ce560bcf7
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Sep 15 14:20:32 2014 +0200

    libudev: util - drop util_delete_path()
    
    Use rmdir_parents() from src/shared instead.

diff --git a/src/libudev/libudev-private.h b/src/libudev/libudev-private.h
index 1c060d9..2f74bc0 100644
--- a/src/libudev/libudev-private.h
+++ b/src/libudev/libudev-private.h
@@ -167,7 +167,6 @@ unsigned int util_string_hash32(const char *key);
 uint64_t util_string_bloom64(const char *str);
 
 /* libudev-util-private.c */
-int util_delete_path(struct udev *udev, const char *path);
 int util_resolve_subsys_kernel(struct udev *udev, const char *string, char *result, size_t maxsize, int read_value);
 
 #endif
diff --git a/src/libudev/libudev-util.c b/src/libudev/libudev-util.c
index 3bc9c67..a7125fa 100644
--- a/src/libudev/libudev-util.c
+++ b/src/libudev/libudev-util.c
@@ -45,38 +45,6 @@
  * Utilities useful when dealing with devices and device node names.
  */
 
-int util_delete_path(struct udev *udev, const char *path)
-{
-        char p[UTIL_PATH_SIZE];
-        char *pos;
-        int err = 0;
-
-        if (path[0] == '/')
-                while(path[1] == '/')
-                        path++;
-        strscpy(p, sizeof(p), path);
-        pos = strrchr(p, '/');
-        if (pos == p || pos == NULL)
-                return 0;
-
-        for (;;) {
-                *pos = '\0';
-                pos = strrchr(p, '/');
-
-                /* don't remove the last one */
-                if ((pos == p) || (pos == NULL))
-                        break;
-
-                err = rmdir(p);
-                if (err < 0) {
-                        if (errno == ENOENT)
-                                err = 0;
-                        break;
-                }
-        }
-        return err;
-}
-
 /* handle "[<SUBSYSTEM>/<KERNEL>]<attribute>" format */
 int util_resolve_subsys_kernel(struct udev *udev, const char *string,
                                char *result, size_t maxsize, int read_value)
diff --git a/src/test/test-udev.c b/src/test/test-udev.c
index f085262..f368c3f 100644
--- a/src/test/test-udev.c
+++ b/src/test/test-udev.c
@@ -149,7 +149,7 @@ int main(int argc, char *argv[]) {
                         mknod(udev_device_get_devnode(dev), mode, udev_device_get_devnum(dev));
                 } else {
                         unlink(udev_device_get_devnode(dev));
-                        util_delete_path(udev, udev_device_get_devnode(dev));
+                        rmdir_parents(udev_device_get_devnode(dev), "/");
                 }
         }
 
diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c
index d42af9a..c164603 100644
--- a/src/udev/udev-node.c
+++ b/src/udev/udev-node.c
@@ -179,7 +179,6 @@ static const char *link_find_prioritized(struct udev_device *dev, bool add, cons
 
 /* manage "stack of names" with possibly specified device priorities */
 static void link_update(struct udev_device *dev, const char *slink, bool add) {
-        struct udev *udev = udev_device_get_udev(dev);
         char name_enc[UTIL_PATH_SIZE];
         char filename[UTIL_PATH_SIZE * 2];
         char dirname[UTIL_PATH_SIZE];
@@ -197,7 +196,7 @@ static void link_update(struct udev_device *dev, const char *slink, bool add) {
         if (target == NULL) {
                 log_debug("no reference left, remove '%s'", slink);
                 if (unlink(slink) == 0)
-                        util_delete_path(udev, slink);
+                        rmdir_parents(slink, "/");
         } else {
                 log_debug("creating link '%s' to '%s'", slink, target);
                 node_symlink(dev, target, slink);

commit 23bf8dd7d5ce1e2a52f28d5d242109ddb668b3fb
Author: Tom Gundersen <teg at jklm.no>
Date:   Tue Sep 9 23:12:14 2014 +0200

    libudev: drop util_lookup_{user,group}
    
    Use shared versions instead. Difference is with overwriting of repeated user/group
    name, and lack of logging.

diff --git a/src/libudev/libudev-private.h b/src/libudev/libudev-private.h
index 35ea7ba..1c060d9 100644
--- a/src/libudev/libudev-private.h
+++ b/src/libudev/libudev-private.h
@@ -168,8 +168,6 @@ uint64_t util_string_bloom64(const char *str);
 
 /* libudev-util-private.c */
 int util_delete_path(struct udev *udev, const char *path);
-uid_t util_lookup_user(struct udev *udev, const char *user);
-gid_t util_lookup_group(struct udev *udev, const char *group);
 int util_resolve_subsys_kernel(struct udev *udev, const char *string, char *result, size_t maxsize, int read_value);
 
 #endif
diff --git a/src/libudev/libudev-util.c b/src/libudev/libudev-util.c
index 9e19e31..3bc9c67 100644
--- a/src/libudev/libudev-util.c
+++ b/src/libudev/libudev-util.c
@@ -77,70 +77,6 @@ int util_delete_path(struct udev *udev, const char *path)
         return err;
 }
 
-uid_t util_lookup_user(struct udev *udev, const char *user)
-{
-        char *endptr;
-        struct passwd pwbuf;
-        struct passwd *pw;
-        uid_t uid;
-        size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
-        char *buf = alloca(buflen);
-
-        if (streq(user, "root"))
-                return 0;
-        uid = strtoul(user, &endptr, 10);
-        if (endptr[0] == '\0')
-                return uid;
-
-        errno = getpwnam_r(user, &pwbuf, buf, buflen, &pw);
-        if (pw != NULL)
-                return pw->pw_uid;
-        if (errno == 0 || errno == ENOENT || errno == ESRCH)
-                udev_err(udev, "specified user '%s' unknown\n", user);
-        else
-                udev_err(udev, "error resolving user '%s': %m\n", user);
-        return 0;
-}
-
-gid_t util_lookup_group(struct udev *udev, const char *group)
-{
-        char *endptr;
-        struct group grbuf;
-        struct group *gr;
-        gid_t gid = 0;
-        size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
-        char *buf = NULL;
-
-        if (streq(group, "root"))
-                return 0;
-        gid = strtoul(group, &endptr, 10);
-        if (endptr[0] == '\0')
-                return gid;
-        gid = 0;
-        for (;;) {
-                char *newbuf;
-
-                newbuf = realloc(buf, buflen);
-                if (!newbuf)
-                        break;
-                buf = newbuf;
-                errno = getgrnam_r(group, &grbuf, buf, buflen, &gr);
-                if (gr != NULL) {
-                        gid = gr->gr_gid;
-                } else if (errno == ERANGE) {
-                        buflen *= 2;
-                        continue;
-                } else if (errno == 0 || errno == ENOENT || errno == ESRCH) {
-                        udev_err(udev, "specified group '%s' unknown\n", group);
-                } else {
-                        udev_err(udev, "error resolving group '%s': %m\n", group);
-                }
-                break;
-        }
-        free(buf);
-        return gid;
-}
-
 /* handle "[<SUBSYSTEM>/<KERNEL>]<attribute>" format */
 int util_resolve_subsys_kernel(struct udev *udev, const char *string,
                                char *result, size_t maxsize, int read_value)
diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
index db95442..ce4d173 100644
--- a/src/udev/udev-rules.c
+++ b/src/udev/udev-rules.c
@@ -459,8 +459,9 @@ static int add_token(struct udev_rules *rules, struct token *token) {
 
 static uid_t add_uid(struct udev_rules *rules, const char *owner) {
         unsigned int i;
-        uid_t uid;
+        uid_t uid = 0;
         unsigned int off;
+        int r;
 
         /* lookup, if we know it already */
         for (i = 0; i < rules->uids_cur; i++) {
@@ -470,7 +471,13 @@ static uid_t add_uid(struct udev_rules *rules, const char *owner) {
                         return uid;
                 }
         }
-        uid = util_lookup_user(rules->udev, owner);
+        r = get_user_creds(&owner, &uid, NULL, NULL, NULL);
+        if (r < 0) {
+                if (r == -ENOENT || r == -ESRCH)
+                        udev_err(rules->udev, "specified user '%s' unknown\n", owner);
+                else
+                        udev_err(rules->udev, "error resolving user '%s': %s\n", owner, strerror(-r));
+        }
 
         /* grow buffer if needed */
         if (rules->uids_cur+1 >= rules->uids_max) {
@@ -499,8 +506,9 @@ static uid_t add_uid(struct udev_rules *rules, const char *owner) {
 
 static gid_t add_gid(struct udev_rules *rules, const char *group) {
         unsigned int i;
-        gid_t gid;
+        gid_t gid = 0;
         unsigned int off;
+        int r;
 
         /* lookup, if we know it already */
         for (i = 0; i < rules->gids_cur; i++) {
@@ -510,7 +518,13 @@ static gid_t add_gid(struct udev_rules *rules, const char *group) {
                         return gid;
                 }
         }
-        gid = util_lookup_group(rules->udev, group);
+        r = get_group_creds(&group, &gid);
+        if (r < 0) {
+                if (r == -ENOENT || r == -ESRCH)
+                        udev_err(rules->udev, "specified group '%s' unknown\n", group);
+                else
+                        udev_err(rules->udev, "error resolving group '%s': %s\n", group, strerror(-r));
+        }
 
         /* grow buffer if needed */
         if (rules->gids_cur+1 >= rules->gids_max) {
@@ -2241,6 +2255,8 @@ int udev_rules_apply_to_event(struct udev_rules *rules,
                         break;
                 case TK_A_OWNER: {
                         char owner[UTIL_NAME_SIZE];
+                        const char *ow = owner;
+                        int r;
 
                         if (event->owner_final)
                                 break;
@@ -2248,7 +2264,15 @@ int udev_rules_apply_to_event(struct udev_rules *rules,
                                 event->owner_final = true;
                         udev_event_apply_format(event, rules_str(rules, cur->key.value_off), owner, sizeof(owner));
                         event->owner_set = true;
-                        event->uid = util_lookup_user(event->udev, owner);
+                        r = get_user_creds(&ow, &event->uid, NULL, NULL, NULL);
+                        if (r < 0) {
+                                if (r == -ENOENT || r == -ESRCH)
+                                        udev_err(event->udev, "specified user '%s' unknown\n", owner);
+                                else
+                                        udev_err(event->udev, "error resolving user '%s': %s\n", owner, strerror(-r));
+
+                                event->uid = 0;
+                        }
                         log_debug("OWNER %u %s:%u",
                                   event->uid,
                                   rules_str(rules, rule->rule.filename_off),
@@ -2257,6 +2281,8 @@ int udev_rules_apply_to_event(struct udev_rules *rules,
                 }
                 case TK_A_GROUP: {
                         char group[UTIL_NAME_SIZE];
+                        const char *gr = group;
+                        int r;
 
                         if (event->group_final)
                                 break;
@@ -2264,7 +2290,15 @@ int udev_rules_apply_to_event(struct udev_rules *rules,
                                 event->group_final = true;
                         udev_event_apply_format(event, rules_str(rules, cur->key.value_off), group, sizeof(group));
                         event->group_set = true;
-                        event->gid = util_lookup_group(event->udev, group);
+                        r = get_group_creds(&gr, &event->gid);
+                        if (r < 0) {
+                                if (r == -ENOENT || r == -ESRCH)
+                                        udev_err(event->udev, "specified group '%s' unknown\n", group);
+                                else
+                                        udev_err(event->udev, "error resolving group '%s': %s\n", group, strerror(-r));
+
+                                event->gid = 0;
+                        }
                         log_debug("GROUP %u %s:%u",
                                   event->gid,
                                   rules_str(rules, rule->rule.filename_off),

commit 4d6dac13ad376cb537647741c45185395beb3e9c
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Sep 15 12:04:29 2014 +0200

    udev: apply permissions to static nodes before signallying READY
    
    Processes expecting static nodes to have the right permissions may order themselves after systemd-udevd.service,
    make sure that actually guarantees what is expected.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index cfa071e..e8b3602 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -1199,6 +1199,20 @@ int main(int argc, char *argv[]) {
 
         udev_monitor_set_receive_buffer_size(monitor, 128 * 1024 * 1024);
 
+        log_info("starting version " VERSION "\n");
+
+        udev_builtin_init(udev);
+
+        rules = udev_rules_new(udev, arg_resolve_names);
+        if (rules == NULL) {
+                log_error("error reading rules");
+                goto exit;
+        }
+
+        rc = udev_rules_apply_static_dev_perms(rules);
+        if (rc < 0)
+                log_error("failed to apply permissions on static device nodes - %s", strerror(-rc));
+
         if (arg_daemonize) {
                 pid_t pid;
 
@@ -1222,20 +1236,6 @@ int main(int argc, char *argv[]) {
                 sd_notify(1, "READY=1");
         }
 
-        log_info("starting version " VERSION "\n");
-
-        udev_builtin_init(udev);
-
-        rules = udev_rules_new(udev, arg_resolve_names);
-        if (rules == NULL) {
-                log_error("error reading rules");
-                goto exit;
-        }
-
-        rc = udev_rules_apply_static_dev_perms(rules);
-        if (rc < 0)
-                log_error("failed to apply permissions on static device nodes - %s", strerror(-rc));
-
         if (arg_children_max <= 0) {
                 cpu_set_t cpu_set;
 

commit ebc164ef40cfa0fa01ce77a6a966129cc611d4ff
Author: Tom Gundersen <teg at jklm.no>
Date:   Mon Sep 15 11:53:03 2014 +0200

    udev: only print after final log level has been determined
    
    This delays label_init(), and drops the (duplicate) printing of version
    information.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 04014b4..cfa071e 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -1104,9 +1104,6 @@ int main(int argc, char *argv[]) {
         udev_set_log_fn(udev, udev_main_log);
         log_set_max_level(udev_get_log_priority(udev));
 
-        log_debug("version %s", VERSION);
-        label_init("/dev");
-
         r = parse_argv(argc, argv);
         if (r <= 0)
                 goto exit;
@@ -1123,6 +1120,8 @@ int main(int argc, char *argv[]) {
                 goto exit;
         }
 
+        label_init("/dev");
+
         /* set umask before creating any file/directory */
         chdir("/");
         umask(022);

commit 3f56f784b98a4b0ad45409a9a19fd787cd7ae455
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri Sep 12 16:45:19 2014 +0200

    udevd: initialize epoll_event structs on allocation
    
    Also move the rest of event initialization next to the event loop (no functional change).

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index e90d9da..04014b4 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -1085,7 +1085,11 @@ int main(int argc, char *argv[]) {
         int fd_ctrl = -1;
         int fd_netlink = -1;
         int fd_worker = -1;
-        struct epoll_event ep_ctrl, ep_inotify, ep_signal, ep_netlink, ep_worker;
+        struct epoll_event ep_ctrl = { .events = EPOLLIN };
+        struct epoll_event ep_inotify = { .events = EPOLLIN };
+        struct epoll_event ep_signal = { .events = EPOLLIN };
+        struct epoll_event ep_netlink = { .events = EPOLLIN };
+        struct epoll_event ep_worker = { .events = EPOLLIN };
         struct udev_ctrl_connection *ctrl_conn = NULL;
         int rc = 1, r;
 
@@ -1221,6 +1225,32 @@ int main(int argc, char *argv[]) {
 
         log_info("starting version " VERSION "\n");
 
+        udev_builtin_init(udev);
+
+        rules = udev_rules_new(udev, arg_resolve_names);
+        if (rules == NULL) {
+                log_error("error reading rules");
+                goto exit;
+        }
+
+        rc = udev_rules_apply_static_dev_perms(rules);
+        if (rc < 0)
+                log_error("failed to apply permissions on static device nodes - %s", strerror(-rc));
+
+        if (arg_children_max <= 0) {
+                cpu_set_t cpu_set;
+
+                arg_children_max = 8;
+
+                if (sched_getaffinity(0, sizeof (cpu_set), &cpu_set) == 0) {
+                        arg_children_max +=  CPU_COUNT(&cpu_set) * 2;
+                }
+        }
+        log_debug("set children_max to %u", arg_children_max);
+
+        udev_list_node_init(&event_list);
+        udev_list_node_init(&worker_list);
+
         fd_inotify = udev_watch_init(udev);
         if (fd_inotify < 0) {
                 log_error("error initializing inotify");
@@ -1247,32 +1277,10 @@ int main(int argc, char *argv[]) {
         }
         fd_worker = worker_watch[READ_END];
 
-        udev_builtin_init(udev);
-
-        rules = udev_rules_new(udev, arg_resolve_names);
-        if (rules == NULL) {
-                log_error("error reading rules");
-                goto exit;
-        }
-
-        memzero(&ep_ctrl, sizeof(struct epoll_event));
-        ep_ctrl.events = EPOLLIN;
         ep_ctrl.data.fd = fd_ctrl;
-
-        memzero(&ep_inotify, sizeof(struct epoll_event));
-        ep_inotify.events = EPOLLIN;
         ep_inotify.data.fd = fd_inotify;
-
-        memzero(&ep_signal, sizeof(struct epoll_event));
-        ep_signal.events = EPOLLIN;
         ep_signal.data.fd = fd_signal;
-
-        memzero(&ep_netlink, sizeof(struct epoll_event));
-        ep_netlink.events = EPOLLIN;
         ep_netlink.data.fd = fd_netlink;
-
-        memzero(&ep_worker, sizeof(struct epoll_event));
-        ep_worker.events = EPOLLIN;
         ep_worker.data.fd = fd_worker;
 
         fd_ep = epoll_create1(EPOLL_CLOEXEC);
@@ -1289,24 +1297,6 @@ int main(int argc, char *argv[]) {
                 goto exit;
         }
 
-        if (arg_children_max <= 0) {
-                cpu_set_t cpu_set;
-
-                arg_children_max = 8;
-
-                if (sched_getaffinity(0, sizeof (cpu_set), &cpu_set) == 0) {
-                        arg_children_max +=  CPU_COUNT(&cpu_set) * 2;
-                }
-        }
-        log_debug("set children_max to %u", arg_children_max);
-
-        rc = udev_rules_apply_static_dev_perms(rules);
-        if (rc < 0)
-                log_error("failed to apply permissions on static device nodes - %s", strerror(-rc));
-
-        udev_list_node_init(&event_list);
-        udev_list_node_init(&worker_list);
-
         for (;;) {
                 static usec_t last_usec;
                 struct epoll_event ev[8];

commit 5c67cf2774a8b964f4d7cd92a4c447da81ac6087
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri Sep 12 16:22:44 2014 +0200

    udev: don't close std{in,out,err}
    
    Rather than printing debug output to stderr and redirecting this to /dev/null when not wanted,
    use the correct log_*() function in the first place.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 8922ff9..e90d9da 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -1221,18 +1221,6 @@ int main(int argc, char *argv[]) {
 
         log_info("starting version " VERSION "\n");
 
-        if (!arg_debug) {
-                int fd;
-
-                fd = open("/dev/null", O_RDWR);
-                if (fd >= 0) {
-                        dup2(fd, STDIN_FILENO);
-                        dup2(fd, STDOUT_FILENO);
-                        dup2(fd, STDERR_FILENO);
-                        close(fd);
-                }
-        }
-
         fd_inotify = udev_watch_init(udev);
         if (fd_inotify < 0) {
                 log_error("error initializing inotify");

commit 959d654105c1303d0c475868a51834db2f7b6099
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri Sep 12 16:17:00 2014 +0200

    udev: drop duplicate logging
    
    Once upon a time logging during early boot was unreliable, so extra logging messages were
    sent by udev to stderr. That is no longer a concern, so drop all fprintf() calls from udved.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 8cdcbd8..8922ff9 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -1058,7 +1058,6 @@ static int parse_argv(int argc, char *argv[]) {
                         } else if (streq(optarg, "never")) {
                                 arg_resolve_names = -1;
                         } else {
-                                fprintf(stderr, "resolve-names must be early, late or never\n");
                                 log_error("resolve-names must be early, late or never");
                                 return 0;
                         }
@@ -1116,7 +1115,6 @@ int main(int argc, char *argv[]) {
         }
 
         if (getuid() != 0) {
-                fprintf(stderr, "root privileges required\n");
                 log_error("root privileges required");
                 goto exit;
         }
@@ -1142,7 +1140,6 @@ int main(int argc, char *argv[]) {
                         if (fd > STDERR_FILENO)
                                 close(fd);
                 } else {
-                        fprintf(stderr, "cannot open /dev/null\n");
                         log_error("cannot open /dev/null");
                 }
         }
@@ -1170,7 +1167,6 @@ int main(int argc, char *argv[]) {
                 /* open control and netlink socket */
                 udev_ctrl = udev_ctrl_new(udev);
                 if (udev_ctrl == NULL) {
-                        fprintf(stderr, "error initializing udev control socket");
                         log_error("error initializing udev control socket");
                         rc = 1;
                         goto exit;
@@ -1179,7 +1175,6 @@ int main(int argc, char *argv[]) {
 
                 monitor = udev_monitor_new_from_netlink(udev, "kernel");
                 if (monitor == NULL) {
-                        fprintf(stderr, "error initializing netlink socket\n");
                         log_error("error initializing netlink socket");
                         rc = 3;
                         goto exit;
@@ -1188,14 +1183,12 @@ int main(int argc, char *argv[]) {
         }
 
         if (udev_monitor_enable_receiving(monitor) < 0) {
-                fprintf(stderr, "error binding netlink socket\n");
                 log_error("error binding netlink socket");
                 rc = 3;
                 goto exit;
         }
 
         if (udev_ctrl_enable_receiving(udev_ctrl) < 0) {
-                fprintf(stderr, "error binding udev control socket\n");
                 log_error("error binding udev control socket");
                 rc = 1;
                 goto exit;
@@ -1242,7 +1235,6 @@ int main(int argc, char *argv[]) {
 
         fd_inotify = udev_watch_init(udev);
         if (fd_inotify < 0) {
-                fprintf(stderr, "error initializing inotify\n");
                 log_error("error initializing inotify");
                 rc = 4;
                 goto exit;
@@ -1254,7 +1246,6 @@ int main(int argc, char *argv[]) {
         sigprocmask(SIG_SETMASK, &mask, &sigmask_orig);
         fd_signal = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC);
         if (fd_signal < 0) {
-                fprintf(stderr, "error creating signalfd\n");
                 log_error("error creating signalfd");
                 rc = 5;
                 goto exit;
@@ -1262,7 +1253,6 @@ int main(int argc, char *argv[]) {
 
         /* unnamed socket from workers to the main daemon */
         if (socketpair(AF_LOCAL, SOCK_DGRAM|SOCK_CLOEXEC, 0, worker_watch) < 0) {
-                fprintf(stderr, "error creating socketpair\n");
                 log_error("error creating socketpair");
                 rc = 6;
                 goto exit;

commit bba7a48439e63defa20641b140522eac55fc1a1e
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri Sep 12 14:42:59 2014 +0200

    udev: split out parse_argv()
    
    Also rename some global variables to arg_* to make it clearer where they come from.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index f0ecbf8..8cdcbd8 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -54,8 +54,6 @@
 #include "dev-setup.h"
 #include "fileio.h"
 
-static bool debug;
-
 void udev_main_log(struct udev *udev, int priority,
                    const char *file, int line, const char *fn,
                    const char *format, va_list args) {
@@ -72,10 +70,13 @@ static int fd_inotify = -1;
 static bool stop_exec_queue;
 static bool reload;
 static int children;
-static int children_max;
-static int exec_delay;
-static usec_t event_timeout_usec = 180 * USEC_PER_SEC;
-static usec_t event_timeout_warn_usec = 180 * USEC_PER_SEC / 3;
+static bool arg_debug = false;
+static int arg_daemonize = false;
+static int arg_resolve_names = 1;
+static int arg_children_max;
+static int arg_exec_delay;
+static usec_t arg_event_timeout_usec = 180 * USEC_PER_SEC;
+static usec_t arg_event_timeout_warn_usec = 180 * USEC_PER_SEC / 3;
 static sigset_t sigmask_orig;
 static UDEV_LIST(event_list);
 static UDEV_LIST(worker_list);
@@ -274,8 +275,8 @@ static void worker_new(struct event *event) {
                         /* needed for SIGCHLD/SIGTERM in spawn() */
                         udev_event->fd_signal = fd_signal;
 
-                        if (exec_delay > 0)
-                                udev_event->exec_delay = exec_delay;
+                        if (arg_exec_delay > 0)
+                                udev_event->exec_delay = arg_exec_delay;
 
                         /*
                          * Take a shared lock on the device node; this establishes
@@ -309,9 +310,9 @@ static void worker_new(struct event *event) {
                         udev_event->rtnl = rtnl;
 
                         /* apply rules, create node, symlinks */
-                        udev_event_execute_rules(udev_event, event_timeout_usec, event_timeout_warn_usec, rules, &sigmask_orig);
+                        udev_event_execute_rules(udev_event, arg_event_timeout_usec, arg_event_timeout_warn_usec, rules, &sigmask_orig);
 
-                        udev_event_execute_run(udev_event, event_timeout_usec, event_timeout_warn_usec, &sigmask_orig);
+                        udev_event_execute_run(udev_event, arg_event_timeout_usec, arg_event_timeout_warn_usec, &sigmask_orig);
 
                         /* in case rtnl was initialized */
                         rtnl = sd_rtnl_ref(udev_event->rtnl);
@@ -441,8 +442,8 @@ static void event_run(struct event *event) {
                 return;
         }
 
-        if (children >= children_max) {
-                if (children_max > 1)
+        if (children >= arg_children_max) {
+                if (arg_children_max > 1)
                         log_debug("maximum number (%i) of children reached", children);
                 return;
         }
@@ -692,7 +693,7 @@ static struct udev_ctrl_connection *handle_ctrl_msg(struct udev_ctrl *uctrl) {
         i = udev_ctrl_get_set_children_max(ctrl_msg);
         if (i >= 0) {
                 log_debug("udevd message (SET_MAX_CHILDREN) received, children_max=%i", i);
-                children_max = i;
+                arg_children_max = i;
         }
 
         if (udev_ctrl_get_ping(ctrl_msg) > 0)
@@ -985,12 +986,12 @@ static void kernel_cmdline_options(struct udev *udev) {
                         log_set_max_level(prio);
                         udev_set_log_priority(udev, prio);
                 } else if (startswith(opt, "udev.children-max=")) {
-                        children_max = strtoul(opt + 18, NULL, 0);
+                        arg_children_max = strtoul(opt + 18, NULL, 0);
                 } else if (startswith(opt, "udev.exec-delay=")) {
-                        exec_delay = strtoul(opt + 16, NULL, 0);
+                        arg_exec_delay = strtoul(opt + 16, NULL, 0);
                 } else if (startswith(opt, "udev.event-timeout=")) {
-                        event_timeout_usec = strtoul(opt + 16, NULL, 0) * USEC_PER_SEC;
-                        event_timeout_warn_usec = (event_timeout_usec / 3) ? : 1;
+                        arg_event_timeout_usec = strtoul(opt + 16, NULL, 0) * USEC_PER_SEC;
+                        arg_event_timeout_warn_usec = (arg_event_timeout_usec / 3) ? : 1;
                 }
 
                 free(s);
@@ -1011,95 +1012,109 @@ static void help(void) {
                , program_invocation_short_name);
 }
 
-int main(int argc, char *argv[]) {
-        struct udev *udev;
-        sigset_t mask;
-        int daemonize = false;
-        int resolve_names = 1;
+static int parse_argv(int argc, char *argv[]) {
         static const struct option options[] = {
-                { "daemon", no_argument, NULL, 'd' },
-                { "debug", no_argument, NULL, 'D' },
-                { "children-max", required_argument, NULL, 'c' },
-                { "exec-delay", required_argument, NULL, 'e' },
-                { "event-timeout", required_argument, NULL, 't' },
-                { "resolve-names", required_argument, NULL, 'N' },
-                { "help", no_argument, NULL, 'h' },
-                { "version", no_argument, NULL, 'V' },
+                { "daemon",             no_argument,            NULL, 'd' },
+                { "debug",              no_argument,            NULL, 'D' },
+                { "children-max",       required_argument,      NULL, 'c' },
+                { "exec-delay",         required_argument,      NULL, 'e' },
+                { "event-timeout",      required_argument,      NULL, 't' },
+                { "resolve-names",      required_argument,      NULL, 'N' },
+                { "help",               no_argument,            NULL, 'h' },
+                { "version",            no_argument,            NULL, 'V' },
                 {}
         };
-        int fd_ctrl = -1;
-        int fd_netlink = -1;
-        int fd_worker = -1;
-        struct epoll_event ep_ctrl, ep_inotify, ep_signal, ep_netlink, ep_worker;
-        struct udev_ctrl_connection *ctrl_conn = NULL;
-        int rc = 1;
-
-        udev = udev_new();
-        if (udev == NULL)
-                goto exit;
-
-        log_set_target(LOG_TARGET_AUTO);
-        log_parse_environment();
-        log_open();
 
-        udev_set_log_fn(udev, udev_main_log);
-        log_set_max_level(udev_get_log_priority(udev));
+        int c;
 
-        log_debug("version %s", VERSION);
-        label_init("/dev");
+        assert(argc >= 0);
+        assert(argv);
 
-        for (;;) {
-                int option;
+        while ((c = getopt_long(argc, argv, "c:de:DtN:hV", options, NULL)) >= 0) {
 
-                option = getopt_long(argc, argv, "c:de:DtN:hV", options, NULL);
-                if (option == -1)
-                        break;
+                switch (c) {
 
-                switch (option) {
                 case 'd':
-                        daemonize = true;
+                        arg_daemonize = true;
                         break;
                 case 'c':
-                        children_max = strtoul(optarg, NULL, 0);
+                        arg_children_max = strtoul(optarg, NULL, 0);
                         break;
                 case 'e':
-                        exec_delay = strtoul(optarg, NULL, 0);
+                        arg_exec_delay = strtoul(optarg, NULL, 0);
                         break;
                 case 't':
-                        event_timeout_usec = strtoul(optarg, NULL, 0) * USEC_PER_SEC;
-                        event_timeout_warn_usec = (event_timeout_usec / 3) ? : 1;
+                        arg_event_timeout_usec = strtoul(optarg, NULL, 0) * USEC_PER_SEC;
+                        arg_event_timeout_warn_usec = (arg_event_timeout_usec / 3) ? : 1;
                         break;
                 case 'D':
-                        debug = true;
-                        log_set_max_level(LOG_DEBUG);
-                        udev_set_log_priority(udev, LOG_DEBUG);
+                        arg_debug = true;
                         break;
                 case 'N':
                         if (streq(optarg, "early")) {
-                                resolve_names = 1;
+                                arg_resolve_names = 1;
                         } else if (streq(optarg, "late")) {
-                                resolve_names = 0;
+                                arg_resolve_names = 0;
                         } else if (streq(optarg, "never")) {
-                                resolve_names = -1;
+                                arg_resolve_names = -1;
                         } else {
                                 fprintf(stderr, "resolve-names must be early, late or never\n");
                                 log_error("resolve-names must be early, late or never");
-                                goto exit;
+                                return 0;
                         }
                         break;
                 case 'h':
                         help();
-                        goto exit;
+                        return 0;
                 case 'V':
                         printf("%s\n", VERSION);
-                        goto exit;
+                        return 0;
+                case '?':
+                        return -EINVAL;
                 default:
-                        goto exit;
+                        assert_not_reached("Unhandled option");
+
                 }
         }
 
+        return 1;
+}
+
+int main(int argc, char *argv[]) {
+        struct udev *udev;
+        sigset_t mask;
+        int fd_ctrl = -1;
+        int fd_netlink = -1;
+        int fd_worker = -1;
+        struct epoll_event ep_ctrl, ep_inotify, ep_signal, ep_netlink, ep_worker;
+        struct udev_ctrl_connection *ctrl_conn = NULL;
+        int rc = 1, r;
+
+        udev = udev_new();
+        if (udev == NULL)
+                goto exit;
+
+        log_set_target(LOG_TARGET_AUTO);
+        log_parse_environment();
+        log_open();
+
+        udev_set_log_fn(udev, udev_main_log);
+        log_set_max_level(udev_get_log_priority(udev));
+
+        log_debug("version %s", VERSION);
+        label_init("/dev");
+
+        r = parse_argv(argc, argv);
+        if (r <= 0)
+                goto exit;
+
         kernel_cmdline_options(udev);
 
+        if (arg_debug) {
+                log_set_max_level(LOG_DEBUG);
+                udev_set_log_priority(udev, LOG_DEBUG);
+        }
+
         if (getuid() != 0) {
                 fprintf(stderr, "root privileges required\n");
                 log_error("root privileges required");
@@ -1115,7 +1130,7 @@ int main(int argc, char *argv[]) {
         dev_setup(NULL);
 
         /* before opening new files, make sure std{in,out,err} fds are in a sane state */
-        if (daemonize) {
+        if (arg_daemonize) {
                 int fd;
 
                 fd = open("/dev/null", O_RDWR);
@@ -1188,7 +1203,7 @@ int main(int argc, char *argv[]) {
 
         udev_monitor_set_receive_buffer_size(monitor, 128 * 1024 * 1024);
 
-        if (daemonize) {
+        if (arg_daemonize) {
                 pid_t pid;
 
                 pid = fork();
@@ -1213,7 +1228,7 @@ int main(int argc, char *argv[]) {
 
         log_info("starting version " VERSION "\n");
 
-        if (!debug) {
+        if (!arg_debug) {
                 int fd;
 
                 fd = open("/dev/null", O_RDWR);
@@ -1256,7 +1271,7 @@ int main(int argc, char *argv[]) {
 
         udev_builtin_init(udev);
 
-        rules = udev_rules_new(udev, resolve_names);
+        rules = udev_rules_new(udev, arg_resolve_names);
         if (rules == NULL) {
                 log_error("error reading rules");
                 goto exit;
@@ -1296,16 +1311,16 @@ int main(int argc, char *argv[]) {
                 goto exit;
         }
 
-        if (children_max <= 0) {
+        if (arg_children_max <= 0) {
                 cpu_set_t cpu_set;
 
-                children_max = 8;
+                arg_children_max = 8;
 
                 if (sched_getaffinity(0, sizeof (cpu_set), &cpu_set) == 0) {
-                        children_max +=  CPU_COUNT(&cpu_set) * 2;
+                        arg_children_max +=  CPU_COUNT(&cpu_set) * 2;
                 }
         }
-        log_debug("set children_max to %u", children_max);
+        log_debug("set children_max to %u", arg_children_max);
 
         rc = udev_rules_apply_static_dev_perms(rules);
         if (rc < 0)
@@ -1401,8 +1416,8 @@ int main(int argc, char *argv[]) {
 
                                 ts = now(CLOCK_MONOTONIC);
 
-                                if ((ts - worker->event_start_usec) > event_timeout_warn_usec) {
-                                        if ((ts - worker->event_start_usec) > event_timeout_usec) {
+                                if ((ts - worker->event_start_usec) > arg_event_timeout_warn_usec) {
+                                        if ((ts - worker->event_start_usec) > arg_event_timeout_usec) {
                                                 log_error("worker [%u] %s timeout; kill it", worker->pid, worker->event->devpath);
                                                 kill(worker->pid, SIGKILL);
                                                 worker->state = WORKER_KILLED;
@@ -1473,7 +1488,7 @@ int main(int argc, char *argv[]) {
                 if (!udev_list_node_is_empty(&event_list) && !udev_exit && !stop_exec_queue) {
                         udev_builtin_init(udev);
                         if (rules == NULL)
-                                rules = udev_rules_new(udev, resolve_names);
+                                rules = udev_rules_new(udev, arg_resolve_names);
                         if (rules != NULL)
                                 event_queue_start(udev);
                 }

commit ed216e1ff0fdd7950b662b9ce7cbb5ca40b575a1
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri Sep 12 14:18:06 2014 +0200

    udev: split out help and modernise a bit

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index e54bfec..f0ecbf8 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -997,6 +997,20 @@ static void kernel_cmdline_options(struct udev *udev) {
         }
 }
 
+static void help(void) {
+        printf("%s [OPTIONS...]\n\n"
+               "Manages devices.\n\n"
+               "  --daemon\n"
+               "  --debug\n"
+               "  --children-max=<maximum number of workers>\n"
+               "  --exec-delay=<seconds to wait before executing RUN=>\n"
+               "  --event-timeout=<seconds to wait before terminating an event>\n"
+               "  --resolve-names=early|late|never\n"
+               "  --version\n"
+               "  --help\n"
+               , program_invocation_short_name);
+}
+
 int main(int argc, char *argv[]) {
         struct udev *udev;
         sigset_t mask;
@@ -1074,16 +1088,7 @@ int main(int argc, char *argv[]) {
                         }
                         break;
                 case 'h':
-                        printf("Usage: udevd OPTIONS\n"
-                               "  --daemon\n"
-                               "  --debug\n"
-                               "  --children-max=<maximum number of workers>\n"
-                               "  --exec-delay=<seconds to wait before executing RUN=>\n"
-                               "  --event-timeout=<seconds to wait before terminating an event>\n"
-                               "  --resolve-names=early|late|never\n"
-                               "  --version\n"
-                               "  --help\n"
-                               "\n");
+                        help();
                         goto exit;
                 case 'V':
                         printf("%s\n", VERSION);



More information about the systemd-commits mailing list