udev: modernize udev-rules.c

yuwata · yuwata · commit 25de7aa7b90c · 2019-06-02T14:15:26.000+09:00
This does the following:
- rename enum udev_builtin_cmd -&gt; UdevBuiltinCmd
- rename struct udev_builtin -&gt; UdevBuiltin
- move type definitions to udev-rules.h
- move prototypes of functions defined in udev-rules.c to udev-rules.h
- drop to use strbuf
- propagate critical errors in applying rules,
- drop limitation for number of tokens per line.
diff --git a/src/fuzz/fuzz-udev-rules.c b/src/fuzz/fuzz-udev-rules.c
@@ -14,7 +14,7 @@
 #include "rm-rf.h"
 #include "string-util.h"
 #include "tests.h"
-#include "udev.h"
+#include "udev-rules.h"
 
 static struct fakefs {
         const char *target;
diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c
@@ -1,7 +1,6 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 
 #include <errno.h>
-#include <string.h>
 
 #include "alloc-util.h"
 #include "env-file.h"
@@ -10,7 +9,6 @@
 #include "string-table.h"
 #include "string-util.h"
 #include "udev-util.h"
-#include "udev.h"
 
 static const char* const resolve_name_timing_table[_RESOLVE_NAME_TIMING_MAX] = {
         [RESOLVE_NAME_NEVER] = "never",
diff --git a/src/test/test-udev.c b/src/test/test-udev.c
@@ -22,7 +22,7 @@
 #include "signal-util.h"
 #include "string-util.h"
 #include "tests.h"
-#include "udev.h"
+#include "udev-event.h"
 
 static int fake_filesystems(void) {
         static const struct fakefs {
diff --git a/src/udev/meson.build b/src/udev/meson.build
@@ -18,13 +18,14 @@ udevadm_sources = files('''
 systemd_udevd_sources = files('udevd.c')
 
 libudev_core_sources = '''
-        udev.h
         udev-ctrl.c
         udev-ctrl.h
         udev-event.c
+        udev-event.h
         udev-node.c
         udev-node.h
         udev-rules.c
+        udev-rules.h
         udev-watch.c
         udev-watch.h
         udev-builtin.c
@@ -117,7 +118,6 @@ libudev_basic = static_library(
 
 libudev_static = static_library(
         'udev_static',
-        'udev.h',
         include_directories : includes,
         link_with : udev_link_with,
         link_whole : libudev_basic)
diff --git a/src/udev/udev-builtin-blkid.c b/src/udev/udev-builtin-blkid.c
@@ -310,7 +310,7 @@ static int builtin_blkid(sd_device *dev, int argc, char *argv[], bool test) {
         return 0;
 }
 
-const struct udev_builtin udev_builtin_blkid = {
+const UdevBuiltin udev_builtin_blkid = {
         .name = "blkid",
         .cmd = builtin_blkid,
         .help = "Filesystem and partition probing",
diff --git a/src/udev/udev-builtin-btrfs.c b/src/udev/udev-builtin-btrfs.c
@@ -33,7 +33,7 @@ static int builtin_btrfs(sd_device *dev, int argc, char *argv[], bool test) {
         return 0;
 }
 
-const struct udev_builtin udev_builtin_btrfs = {
+const UdevBuiltin udev_builtin_btrfs = {
         .name = "btrfs",
         .cmd = builtin_btrfs,
         .help = "btrfs volume management",
diff --git a/src/udev/udev-builtin-hwdb.c b/src/udev/udev-builtin-hwdb.c
@@ -208,7 +208,7 @@ static bool builtin_hwdb_validate(void) {
         return hwdb_validate(hwdb);
 }
 
-const struct udev_builtin udev_builtin_hwdb = {
+const UdevBuiltin udev_builtin_hwdb = {
         .name = "hwdb",
         .cmd = builtin_hwdb,
         .init = builtin_hwdb_init,
diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
@@ -357,7 +357,7 @@ static int builtin_input_id(sd_device *dev, int argc, char *argv[], bool test) {
         return 0;
 }
 
-const struct udev_builtin udev_builtin_input_id = {
+const UdevBuiltin udev_builtin_input_id = {
         .name = "input_id",
         .cmd = builtin_input_id,
         .help = "Input device properties",
diff --git a/src/udev/udev-builtin-keyboard.c b/src/udev/udev-builtin-keyboard.c
@@ -251,7 +251,7 @@ static int builtin_keyboard(sd_device *dev, int argc, char *argv[], bool test) {
         return 0;
 }
 
-const struct udev_builtin udev_builtin_keyboard = {
+const UdevBuiltin udev_builtin_keyboard = {
         .name = "keyboard",
         .cmd = builtin_keyboard,
         .help = "Keyboard scan code to key mapping",
diff --git a/src/udev/udev-builtin-kmod.c b/src/udev/udev-builtin-kmod.c
@@ -66,7 +66,7 @@ static bool builtin_kmod_validate(void) {
         return (kmod_validate_resources(ctx) != KMOD_RESOURCES_OK);
 }
 
-const struct udev_builtin udev_builtin_kmod = {
+const UdevBuiltin udev_builtin_kmod = {
         .name = "kmod",
         .cmd = builtin_kmod,
         .init = builtin_kmod_init,
diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c
@@ -927,7 +927,7 @@ static int builtin_net_id(sd_device *dev, int argc, char *argv[], bool test) {
         return 0;
 }
 
-const struct udev_builtin udev_builtin_net_id = {
+const UdevBuiltin udev_builtin_net_id = {
         .name = "net_id",
         .cmd = builtin_net_id,
         .help = "Network device properties",
diff --git a/src/udev/udev-builtin-net_setup_link.c b/src/udev/udev-builtin-net_setup_link.c
@@ -74,7 +74,7 @@ static bool builtin_net_setup_link_validate(void) {
         return link_config_should_reload(ctx);
 }
 
-const struct udev_builtin udev_builtin_net_setup_link = {
+const UdevBuiltin udev_builtin_net_setup_link = {
         .name = "net_setup_link",
         .cmd = builtin_net_setup_link,
         .init = builtin_net_setup_link_init,
diff --git a/src/udev/udev-builtin-path_id.c b/src/udev/udev-builtin-path_id.c
@@ -675,7 +675,7 @@ static int builtin_path_id(sd_device *dev, int argc, char *argv[], bool test) {
         return 0;
 }
 
-const struct udev_builtin udev_builtin_path_id = {
+const UdevBuiltin udev_builtin_path_id = {
         .name = "path_id",
         .cmd = builtin_path_id,
         .help = "Compose persistent device path",
diff --git a/src/udev/udev-builtin-uaccess.c b/src/udev/udev-builtin-uaccess.c
@@ -73,7 +73,7 @@ static int builtin_uaccess(sd_device *dev, int argc, char *argv[], bool test) {
         return r;
 }
 
-const struct udev_builtin udev_builtin_uaccess = {
+const UdevBuiltin udev_builtin_uaccess = {
         .name = "uaccess",
         .cmd = builtin_uaccess,
         .help = "Manage device node user ACL",
diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c
@@ -455,7 +455,7 @@ static int builtin_usb_id(sd_device *dev, int argc, char *argv[], bool test) {
         return 0;
 }
 
-const struct udev_builtin udev_builtin_usb_id = {
+const UdevBuiltin udev_builtin_usb_id = {
         .name = "usb_id",
         .cmd = builtin_usb_id,
         .help = "USB device properties",
diff --git a/src/udev/udev-builtin.c b/src/udev/udev-builtin.c
@@ -12,7 +12,7 @@
 
 static bool initialized;
 
-static const struct udev_builtin *const builtins[_UDEV_BUILTIN_MAX] = {
+static const UdevBuiltin *const builtins[_UDEV_BUILTIN_MAX] = {
 #if HAVE_BLKID
         [UDEV_BUILTIN_BLKID] = &udev_builtin_blkid,
 #endif
@@ -75,7 +75,7 @@ void udev_builtin_list(void) {
                         fprintf(stderr, "  %-14s  %s\n", builtins[i]->name, builtins[i]->help);
 }
 
-const char *udev_builtin_name(enum udev_builtin_cmd cmd) {
+const char *udev_builtin_name(UdevBuiltinCommand cmd) {
         assert(cmd >= 0 && cmd < _UDEV_BUILTIN_MAX);
 
         if (!builtins[cmd])
@@ -84,7 +84,7 @@ const char *udev_builtin_name(enum udev_builtin_cmd cmd) {
         return builtins[cmd]->name;
 }
 
-bool udev_builtin_run_once(enum udev_builtin_cmd cmd) {
+bool udev_builtin_run_once(UdevBuiltinCommand cmd) {
         assert(cmd >= 0 && cmd < _UDEV_BUILTIN_MAX);
 
         if (!builtins[cmd])
@@ -93,8 +93,8 @@ bool udev_builtin_run_once(enum udev_builtin_cmd cmd) {
         return builtins[cmd]->run_once;
 }
 
-enum udev_builtin_cmd udev_builtin_lookup(const char *command) {
-        enum udev_builtin_cmd i;
+UdevBuiltinCommand udev_builtin_lookup(const char *command) {
+        UdevBuiltinCommand i;
         size_t n;
 
         assert(command);
@@ -108,7 +108,7 @@ enum udev_builtin_cmd udev_builtin_lookup(const char *command) {
         return _UDEV_BUILTIN_INVALID;
 }
 
-int udev_builtin_run(sd_device *dev, enum udev_builtin_cmd cmd, const char *command, bool test) {
+int udev_builtin_run(sd_device *dev, UdevBuiltinCommand cmd, const char *command, bool test) {
         _cleanup_strv_free_ char **argv = NULL;
 
         assert(dev);
diff --git a/src/udev/udev-builtin.h b/src/udev/udev-builtin.h
@@ -5,7 +5,7 @@
 
 #include "sd-device.h"
 
-enum udev_builtin_cmd {
+typedef enum {
 #if HAVE_BLKID
         UDEV_BUILTIN_BLKID,
 #endif
@@ -25,42 +25,45 @@ enum udev_builtin_cmd {
 #endif
         _UDEV_BUILTIN_MAX,
         _UDEV_BUILTIN_INVALID = -1,
-};
+} UdevBuiltinCommand;
 
-struct udev_builtin {
+typedef struct UdevBuiltin {
         const char *name;
         int (*cmd)(sd_device *dev, int argc, char *argv[], bool test);
         const char *help;
         int (*init)(void);
         void (*exit)(void);
         bool (*validate)(void);
         bool run_once;
-};
+} UdevBuiltin;
+
+#define PTR_TO_UDEV_BUILTIN_CMD(p) ((UdevBuiltinCommand) ((intptr_t) (p)-1))
+#define UDEV_BUILTIN_CMD_TO_PTR(u) ((void *)             ((intptr_t) (u)+1))
 
 #if HAVE_BLKID
-extern const struct udev_builtin udev_builtin_blkid;
+extern const UdevBuiltin udev_builtin_blkid;
 #endif
-extern const struct udev_builtin udev_builtin_btrfs;
-extern const struct udev_builtin udev_builtin_hwdb;
-extern const struct udev_builtin udev_builtin_input_id;
-extern const struct udev_builtin udev_builtin_keyboard;
+extern const UdevBuiltin udev_builtin_btrfs;
+extern const UdevBuiltin udev_builtin_hwdb;
+extern const UdevBuiltin udev_builtin_input_id;
+extern const UdevBuiltin udev_builtin_keyboard;
 #if HAVE_KMOD
-extern const struct udev_builtin udev_builtin_kmod;
+extern const UdevBuiltin udev_builtin_kmod;
 #endif
-extern const struct udev_builtin udev_builtin_net_id;
-extern const struct udev_builtin udev_builtin_net_setup_link;
-extern const struct udev_builtin udev_builtin_path_id;
-extern const struct udev_builtin udev_builtin_usb_id;
+extern const UdevBuiltin udev_builtin_net_id;
+extern const UdevBuiltin udev_builtin_net_setup_link;
+extern const UdevBuiltin udev_builtin_path_id;
+extern const UdevBuiltin udev_builtin_usb_id;
 #if HAVE_ACL
-extern const struct udev_builtin udev_builtin_uaccess;
+extern const UdevBuiltin udev_builtin_uaccess;
 #endif
 
 void udev_builtin_init(void);
 void udev_builtin_exit(void);
-enum udev_builtin_cmd udev_builtin_lookup(const char *command);
-const char *udev_builtin_name(enum udev_builtin_cmd cmd);
-bool udev_builtin_run_once(enum udev_builtin_cmd cmd);
-int udev_builtin_run(sd_device *dev, enum udev_builtin_cmd cmd, const char *command, bool test);
+UdevBuiltinCommand udev_builtin_lookup(const char *command);
+const char *udev_builtin_name(UdevBuiltinCommand cmd);
+bool udev_builtin_run_once(UdevBuiltinCommand cmd);
+int udev_builtin_run(sd_device *dev, UdevBuiltinCommand cmd, const char *command, bool test);
 void udev_builtin_list(void);
 bool udev_builtin_validate(void);
 int udev_builtin_add_property(sd_device *dev, bool test, const char *key, const char *val);
diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
@@ -16,6 +16,7 @@
 #include "device-private.h"
 #include "device-util.h"
 #include "fd-util.h"
+#include "fs-util.h"
 #include "format-util.h"
 #include "libudev-util.h"
 #include "netlink-util.h"
@@ -28,10 +29,11 @@
 #include "strv.h"
 #include "strxcpyx.h"
 #include "udev-builtin.h"
+#include "udev-event.h"
 #include "udev-node.h"
 #include "udev-util.h"
 #include "udev-watch.h"
-#include "udev.h"
+#include "user-util.h"
 
 typedef struct Spawn {
         const char *cmd;
@@ -61,6 +63,9 @@ UdevEvent *udev_event_new(sd_device *dev, usec_t exec_delay_usec, sd_netlink *rt
                 .birth_usec = now(CLOCK_MONOTONIC),
                 .exec_delay_usec = exec_delay_usec,
                 .rtnl = sd_netlink_ref(rtnl),
+                .uid = UID_INVALID,
+                .gid = GID_INVALID,
+                .mode = MODE_INVALID,
         };
 
         return event;
@@ -753,19 +758,23 @@ static int update_devnode(UdevEvent *event) {
         if (event->dev_db_clone)
                 (void) udev_node_update_old_links(dev, event->dev_db_clone);
 
-        if (!event->owner_set) {
+        if (!uid_is_valid(event->uid)) {
                 r = device_get_devnode_uid(dev, &event->uid);
-                if (r < 0 && r != -ENOENT)
+                if (r == -ENOENT)
+                        event->uid = 0;
+                else if (r < 0)
                         return log_device_error_errno(dev, r, "Failed to get devnode UID: %m");
         }
 
-        if (!event->group_set) {
+        if (!gid_is_valid(event->gid)) {
                 r = device_get_devnode_gid(dev, &event->gid);
-                if (r < 0 && r != -ENOENT)
+                if (r == -ENOENT)
+                        event->gid = 0;
+                else if (r < 0)
                         return log_device_error_errno(dev, r, "Failed to get devnode GID: %m");
         }
 
-        if (!event->mode_set) {
+        if (event->mode == MODE_INVALID) {
                 r = device_get_devnode_mode(dev, &event->mode);
                 if (r < 0 && r != -ENOENT)
                         return log_device_error_errno(dev, r, "Failed to get devnode mode: %m");
@@ -779,7 +788,10 @@ static int update_devnode(UdevEvent *event) {
                 }
         }
 
-        apply = device_for_action(dev, DEVICE_ACTION_ADD) || event->owner_set || event->group_set || event->mode_set;
+        apply = device_for_action(dev, DEVICE_ACTION_ADD) ||
+                uid_is_valid(event->uid) ||
+                gid_is_valid(event->gid) ||
+                event->mode != MODE_INVALID;
         return udev_node_add(dev, apply, event->mode, event->uid, event->gid, event->seclabel_list);
 }
 
@@ -900,22 +912,32 @@ void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec) {
         const char *cmd;
         void *val;
         Iterator i;
+        int r;
 
         ORDERED_HASHMAP_FOREACH_KEY(val, cmd, event->run_list, i) {
-                enum udev_builtin_cmd builtin_cmd = PTR_TO_INT(val);
+                UdevBuiltinCommand builtin_cmd = PTR_TO_UDEV_BUILTIN_CMD(val);
                 char command[UTIL_PATH_SIZE];
 
-                udev_event_apply_format(event, cmd, command, sizeof(command), false);
+                (void) udev_event_apply_format(event, cmd, command, sizeof(command), false);
 
-                if (builtin_cmd >= 0 && builtin_cmd < _UDEV_BUILTIN_MAX)
-                        udev_builtin_run(event->dev, builtin_cmd, command, false);
-                else {
+                if (builtin_cmd != _UDEV_BUILTIN_INVALID) {
+                        log_device_debug(event->dev, "Running built-in command \"%s\"", command);
+                        r = udev_builtin_run(event->dev, builtin_cmd, command, false);
+                        if (r < 0)
+                                log_device_debug_errno(event->dev, r, "Failed to run built-in command \"%s\", ignoring: %m", command);
+                } else {
                         if (event->exec_delay_usec > 0) {
-                                log_debug("delay execution of '%s'", command);
+                                char buf[FORMAT_TIMESPAN_MAX];
+
+                                log_device_debug(event->dev, "Delaying execution of \"%s\" for %s.",
+                                                 command, format_timespan(buf, sizeof(buf), event->exec_delay_usec, USEC_PER_SEC));
                                 (void) usleep(event->exec_delay_usec);
                         }
 
-                        (void) udev_event_spawn(event, timeout_usec, false, command, NULL, 0);
+                        log_device_debug(event->dev, "Running command \"%s\"", command);
+                        r = udev_event_spawn(event, timeout_usec, false, command, NULL, 0);
+                        if (r > 0) /* returned value is positive when program fails */
+                                log_device_debug(event->dev, "Command \"%s\" returned %d (error), ignoring.", command, r);
                 }
         }
 }
diff --git a/src/udev/udev-event.h b/src/udev/udev-event.h
diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
diff --git a/src/udev/udev-rules.h b/src/udev/udev-rules.h
diff --git a/src/udev/udevadm-test-builtin.c b/src/udev/udevadm-test-builtin.c
diff --git a/src/udev/udevadm-test.c b/src/udev/udevadm-test.c
diff --git a/src/udev/udevd.c b/src/udev/udevd.c

Original file line number	Diff line number	Diff line change
`@@ -310,7 +310,7 @@ static int builtin_blkid(sd_device dev, int argc, char argv[], bool test) {`
`310`	`310`	`return 0;`
`311`	`311`	`}`
`312`	`312`
`313`		`-const struct udev_builtin udev_builtin_blkid = {`
	`313`	`+const UdevBuiltin udev_builtin_blkid = {`
`314`	`314`	`.name = "blkid",`
`315`	`315`	`.cmd = builtin_blkid,`
`316`	`316`	`.help = "Filesystem and partition probing",`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ static int builtin_btrfs(sd_device dev, int argc, char argv[], bool test) {`
`33`	`33`	`return 0;`
`34`	`34`	`}`
`35`	`35`
`36`		`-const struct udev_builtin udev_builtin_btrfs = {`
	`36`	`+const UdevBuiltin udev_builtin_btrfs = {`
`37`	`37`	`.name = "btrfs",`
`38`	`38`	`.cmd = builtin_btrfs,`
`39`	`39`	`.help = "btrfs volume management",`
Original file line number	Diff line number	Diff line change
`@@ -208,7 +208,7 @@ static bool builtin_hwdb_validate(void) {`
`208`	`208`	`return hwdb_validate(hwdb);`
`209`	`209`	`}`
`210`	`210`
`211`		`-const struct udev_builtin udev_builtin_hwdb = {`
	`211`	`+const UdevBuiltin udev_builtin_hwdb = {`
`212`	`212`	`.name = "hwdb",`
`213`	`213`	`.cmd = builtin_hwdb,`
`214`	`214`	`.init = builtin_hwdb_init,`
Original file line number	Diff line number	Diff line change
`@@ -357,7 +357,7 @@ static int builtin_input_id(sd_device dev, int argc, char argv[], bool test) {`
`357`	`357`	`return 0;`
`358`	`358`	`}`
`359`	`359`
`360`		`-const struct udev_builtin udev_builtin_input_id = {`
	`360`	`+const UdevBuiltin udev_builtin_input_id = {`
`361`	`361`	`.name = "input_id",`
`362`	`362`	`.cmd = builtin_input_id,`
`363`	`363`	`.help = "Input device properties",`
Original file line number	Diff line number	Diff line change
`@@ -251,7 +251,7 @@ static int builtin_keyboard(sd_device dev, int argc, char argv[], bool test) {`
`251`	`251`	`return 0;`
`252`	`252`	`}`
`253`	`253`
`254`		`-const struct udev_builtin udev_builtin_keyboard = {`
	`254`	`+const UdevBuiltin udev_builtin_keyboard = {`
`255`	`255`	`.name = "keyboard",`
`256`	`256`	`.cmd = builtin_keyboard,`
`257`	`257`	`.help = "Keyboard scan code to key mapping",`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,7 @@ static bool builtin_kmod_validate(void) {`
`66`	`66`	`return (kmod_validate_resources(ctx) != KMOD_RESOURCES_OK);`
`67`	`67`	`}`
`68`	`68`
`69`		`-const struct udev_builtin udev_builtin_kmod = {`
	`69`	`+const UdevBuiltin udev_builtin_kmod = {`
`70`	`70`	`.name = "kmod",`
`71`	`71`	`.cmd = builtin_kmod,`
`72`	`72`	`.init = builtin_kmod_init,`