From 1937f5105d375ba3fbf7fa65366e6435d6b36b5a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Aug 2024 04:01:47 +0900 Subject: [PATCH 1/6] sd-device: make sd_device_get_devtype() return 0 on success again This partially reverts the commit 730b76bd2cd5f0866baa738ae283e3b62544a28f. Before the commit, the function returned 0 on success, but the commit made the function always return 1, as if device->devtype is NULL, the function returns -ENOENT in the above. Fortunately, udev_device_get_devtype() does not propagate any non-negative value from sd_device_get_devtype(). Hence, hopefully we can safely revert the change. --- src/libsystemd/sd-device/sd-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index d8d151835c..dafda626f0 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1211,7 +1211,7 @@ _public_ int sd_device_get_devtype(sd_device *device, const char **devtype) { if (devtype) *devtype = device->devtype; - return !!device->devtype; + return 0; } _public_ int sd_device_get_parent_with_subsystem_devtype(sd_device *device, const char *subsystem, const char *devtype, sd_device **ret) { From 55e35a4d59bea8653a428f544490b1d80b848d64 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Aug 2024 06:33:42 +0900 Subject: [PATCH 2/6] sd-device: refuse earlier when too long ifname is passed to sd_device_new_from_ifname() Otherwise, alloca() called in strjoina() may trigger assertion. This partially reverts 3652891c3904992e21739e9bfb004073841db63c. The commit mistakenly dropped the check. --- src/libsystemd/sd-device/sd-device.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index dafda626f0..3d683fb280 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -345,9 +345,11 @@ _public_ int sd_device_new_from_ifname(sd_device **ret, const char *ifname) { assert_return(ret, -EINVAL); assert_return(ifname, -EINVAL); - r = device_new_from_main_ifname(ret, ifname); - if (r >= 0) - return r; + if (ifname_valid(ifname)) { + r = device_new_from_main_ifname(ret, ifname); + if (r >= 0) + return r; + } r = rtnl_resolve_ifname_full(NULL, RESOLVE_IFNAME_ALTERNATIVE | RESOLVE_IFNAME_NUMERIC, ifname, &main_name, NULL); if (r < 0) From 44bc6f3cabb2d26b432a91c09e1747c7a783dac9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Aug 2024 04:09:49 +0900 Subject: [PATCH 3/6] sd-device: introduce sd_device_get_driver_subsystem() To create the sd_device object of a driver, the function sd_device_new_from_subsystem_sysname() requires "drivers" for subsystem and e.g. "pci:iwlwifi" for sysname. Similarly, sd_device_new_from_device_id() also requires driver subsystem. However, we have never provided a way to get the driver subsystem ("pci" for the previous example) from an existing sd_device object. Let's introduce a way to get driver subsystem. --- man/rules/meson.build | 1 + man/sd_device_get_syspath.xml | 15 +++++++++++++++ src/libsystemd/libsystemd.sym | 1 + src/libsystemd/sd-device/sd-device.c | 14 ++++++++++++++ src/libsystemd/sd-device/test-sd-device.c | 8 +++++--- src/systemd/sd-device.h | 1 + 6 files changed, 37 insertions(+), 3 deletions(-) diff --git a/man/rules/meson.build b/man/rules/meson.build index a42143ec7c..f551c25e6b 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -530,6 +530,7 @@ manpages = [ 'sd_device_get_devtype', 'sd_device_get_diskseq', 'sd_device_get_driver', + 'sd_device_get_driver_subsystem', 'sd_device_get_ifindex', 'sd_device_get_subsystem', 'sd_device_get_sysname', diff --git a/man/sd_device_get_syspath.xml b/man/sd_device_get_syspath.xml index 718da91e87..f21d7a297b 100644 --- a/man/sd_device_get_syspath.xml +++ b/man/sd_device_get_syspath.xml @@ -22,6 +22,7 @@ sd_device_get_sysname sd_device_get_sysnum sd_device_get_subsystem + sd_device_get_driver_subsystem sd_device_get_devtype sd_device_get_devname sd_device_get_devnum @@ -66,6 +67,12 @@ const char **ret + + int sd_device_get_driver_subsystem + sd_device *device + const char **ret + + int sd_device_get_devtype sd_device *device @@ -126,6 +133,13 @@ record. This is a short string fitting into a filename, and thus does not contain a slash and cannot be empty. Example: tty, block or net. + sd_device_get_driver_subsystem() returns the connected bus type of the devices + loaded by the specified driver device record. For example, when iwlwifi driver device + is specified, which is used by the wireless network interfaces connected to PCI bus, this function returns + pci. This function only succeeds when sd_device_get_subsystem() + returns drivers. Example: pci, i2c, or + hid. + sd_device_get_devtype() returns the device type of the specified device record, if the subsystem manages multiple types of devices. Example: for devices of the block subsystem this can be disk or partition @@ -206,6 +220,7 @@ sd_device_get_ifindex(), sd_device_get_driver(), and sd_device_get_diskseq() were added in version 251. + sd_device_get_driver_subsystem() was added in version 257. diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 50ef8096db..e79c1a65d2 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -1046,6 +1046,7 @@ global: sd_varlink_take_fd; sd_varlink_unref; sd_varlink_wait; + sd_device_get_driver_subsystem; sd_device_monitor_is_running; sd_device_monitor_get_fd; sd_device_monitor_receive; diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 3d683fb280..6dedfc4379 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1198,6 +1198,20 @@ _public_ int sd_device_get_subsystem(sd_device *device, const char **ret) { return 0; } +_public_ int sd_device_get_driver_subsystem(sd_device *device, const char **ret) { + assert_return(device, -EINVAL); + + if (!device_in_subsystem(device, "drivers")) + return -ENOENT; + + assert(device->driver_subsystem); + + if (ret) + *ret = device->driver_subsystem; + + return 0; +} + _public_ int sd_device_get_devtype(sd_device *device, const char **devtype) { int r; diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index 9fde1a0814..ae745ab321 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -79,9 +79,11 @@ static void test_sd_device_one(sd_device *d) { * sd_device_new_from_device_id() may not work as expected. */ const char *name, *id; - if (streq(subsystem, "drivers")) - name = strjoina(d->driver_subsystem, ":", sysname); - else + if (streq(subsystem, "drivers")) { + const char *driver_subsystem; + ASSERT_OK(sd_device_get_driver_subsystem(d, &driver_subsystem)); + name = strjoina(driver_subsystem, ":", sysname); + } else name = sysname; assert_se(sd_device_new_from_subsystem_sysname(&dev, subsystem, name) >= 0); assert_se(sd_device_get_syspath(dev, &val) >= 0); diff --git a/src/systemd/sd-device.h b/src/systemd/sd-device.h index c84cc55202..86f90a1fd1 100644 --- a/src/systemd/sd-device.h +++ b/src/systemd/sd-device.h @@ -74,6 +74,7 @@ int sd_device_get_parent_with_subsystem_devtype(sd_device *child, const char *su int sd_device_get_syspath(sd_device *device, const char **ret); int sd_device_get_subsystem(sd_device *device, const char **ret); +int sd_device_get_driver_subsystem(sd_device *device, const char **ret); int sd_device_get_devtype(sd_device *device, const char **ret); int sd_device_get_devnum(sd_device *device, dev_t *devnum); int sd_device_get_ifindex(sd_device *device, int *ifindex); From 1ff0164be5978b824d2213bc546dac66619e1a48 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Aug 2024 04:36:16 +0900 Subject: [PATCH 4/6] sd-device: make device_get_device_id() public We have already exposed sd_device_new_from_device_id(), but we have never provide the way to get device ID from an existing sd_device object. --- man/rules/meson.build | 3 ++- man/sd_device_get_syspath.xml | 26 ++++++++++++++++++++++- src/libsystemd/libsystemd.sym | 1 + src/libsystemd/sd-device/device-private.c | 4 ++-- src/libsystemd/sd-device/device-private.h | 1 - src/libsystemd/sd-device/sd-device.c | 8 +++---- src/libsystemd/sd-device/test-sd-device.c | 2 +- src/systemd/sd-device.h | 1 + src/udev/udev-manager.c | 2 +- src/udev/udev-node.c | 8 +++---- src/udev/udev-watch.c | 4 ++-- 11 files changed, 43 insertions(+), 17 deletions(-) diff --git a/man/rules/meson.build b/man/rules/meson.build index f551c25e6b..ad617aa4d5 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -524,7 +524,8 @@ manpages = [ ['sd_bus_wait', '3', [], ''], ['sd_device_get_syspath', '3', - ['sd_device_get_devname', + ['sd_device_get_device_id', + 'sd_device_get_devname', 'sd_device_get_devnum', 'sd_device_get_devpath', 'sd_device_get_devtype', diff --git a/man/sd_device_get_syspath.xml b/man/sd_device_get_syspath.xml index f21d7a297b..63ef61e1ff 100644 --- a/man/sd_device_get_syspath.xml +++ b/man/sd_device_get_syspath.xml @@ -29,6 +29,7 @@ sd_device_get_ifindex sd_device_get_driver sd_device_get_diskseq + sd_device_get_device_id Returns various fields of device objects @@ -109,6 +110,12 @@ uint64_t *ret + + int sd_device_get_device_id + sd_device *device + uint64_t *ret + + @@ -171,6 +178,22 @@ the device name changing, and is relevant for block devices encapsulating devices with changing media (e.g. floppy or CD-ROM), or loopback block devices. Only defined for block devices, i.e. those of subsystem block. + + sd_device_get_device_id() returns the short string that identifies the device + record. When the device ID obtained by the function for a specified device record is passed to + sd_device_new_from_device_id(), a new instance of the same device record will be + gained. When a block or character device is specified, which has corresponding device node, this returns + b or c, respectively, followed by the device node major and minor + numbers separated with a colon. Example: b259:1 or c10:121. Whan a + network interface device is specified, this returns n followed by the interface index, + which can be obtained by sd_device_get_ifindex(). Example: n1. + When a device in the driver subsystem is specified, this returns + +drivers: followed by its driver subsystem and sysfs name separated with a colon. + Example: +drivers:pci:iwlwifi for a driver device record whose driver subsystem is + pci and sysfs name is iwlwifi, + When an other type of device is specified, this function returns + followed by its + subsystem and sysfs name separated with a colon. Example: +acpi:ACPI0003:00, + +input:input16, or +pci:0000:00:1f.6. @@ -220,7 +243,8 @@ sd_device_get_ifindex(), sd_device_get_driver(), and sd_device_get_diskseq() were added in version 251. - sd_device_get_driver_subsystem() was added in version 257. + sd_device_get_driver_subsystem() and + sd_device_get_device_id() were added in version 257. diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index e79c1a65d2..84053ee7c9 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -1046,6 +1046,7 @@ global: sd_varlink_take_fd; sd_varlink_unref; sd_varlink_wait; + sd_device_get_device_id; sd_device_get_driver_subsystem; sd_device_monitor_is_running; sd_device_monitor_get_fd; diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index cd85ec9c62..1c148b8573 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -711,7 +711,7 @@ static int device_tag(sd_device *device, const char *tag, bool add) { assert(device); assert(tag); - r = device_get_device_id(device, &id); + r = sd_device_get_device_id(device, &id); if (r < 0) return r; @@ -797,7 +797,7 @@ static int device_get_db_path(sd_device *device, char **ret) { assert(device); assert(ret); - r = device_get_device_id(device, &id); + r = sd_device_get_device_id(device, &id); if (r < 0) return r; diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index e0b1efe506..eab54203f0 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -26,7 +26,6 @@ static inline int device_get_sysattr_unsigned(sd_device *device, const char *sys } int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret_value); int device_get_sysattr_bool(sd_device *device, const char *sysattr); -int device_get_device_id(sd_device *device, const char **ret); int device_get_devlink_priority(sd_device *device, int *ret); int device_get_devnode_mode(sd_device *device, mode_t *ret); int device_get_devnode_uid(sd_device *device, uid_t *ret); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 6dedfc4379..383445edc8 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1637,9 +1637,8 @@ static int handle_db_line(sd_device *device, char key, const char *value) { } } -int device_get_device_id(sd_device *device, const char **ret) { - assert(device); - assert(ret); +_public_ int sd_device_get_device_id(sd_device *device, const char **ret) { + assert_return(device, -EINVAL); if (!device->device_id) { _cleanup_free_ char *id = NULL; @@ -1689,7 +1688,8 @@ int device_get_device_id(sd_device *device, const char **ret) { device->device_id = TAKE_PTR(id); } - *ret = device->device_id; + if (ret) + *ret = device->device_id; return 0; } diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index ae745ab321..a048860dbe 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -91,7 +91,7 @@ static void test_sd_device_one(sd_device *d) { dev = sd_device_unref(dev); /* The device ID depends on subsystem. */ - assert_se(device_get_device_id(d, &id) >= 0); + assert_se(sd_device_get_device_id(d, &id) >= 0); r = sd_device_new_from_device_id(&dev, id); if (r == -ENODEV && ifindex > 0) log_device_warning_errno(d, r, diff --git a/src/systemd/sd-device.h b/src/systemd/sd-device.h index 86f90a1fd1..ff66414701 100644 --- a/src/systemd/sd-device.h +++ b/src/systemd/sd-device.h @@ -86,6 +86,7 @@ int sd_device_get_sysnum(sd_device *device, const char **ret); int sd_device_get_action(sd_device *device, sd_device_action_t *ret); int sd_device_get_seqnum(sd_device *device, uint64_t *ret); int sd_device_get_diskseq(sd_device *device, uint64_t *ret); +int sd_device_get_device_id(sd_device *device, const char **ret); int sd_device_get_is_initialized(sd_device *device); int sd_device_get_usec_initialized(sd_device *device, uint64_t *ret); diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index cc8f7fdf8f..a031444c2e 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -729,7 +729,7 @@ static int event_queue_insert(Manager *manager, sd_device *dev) { if (r < 0 && r != -ENOENT) return r; - r = device_get_device_id(dev, &id); + r = sd_device_get_device_id(dev, &id); if (r < 0 && r != -ENOENT) return r; diff --git a/src/udev/udev-node.c b/src/udev/udev-node.c index 253ffa7b7c..a4e73025a0 100644 --- a/src/udev/udev-node.c +++ b/src/udev/udev-node.c @@ -220,7 +220,7 @@ static int stack_directory_find_prioritized_devnode(sd_device *dev, int dirfd, b if (!dir) return -errno; - r = device_get_device_id(dev, &id); + r = sd_device_get_device_id(dev, &id); if (r < 0) return r; @@ -246,7 +246,7 @@ static int stack_directory_update(sd_device *dev, int fd, bool add) { assert(dev); assert(fd >= 0); - r = device_get_device_id(dev, &id); + r = sd_device_get_device_id(dev, &id); if (r < 0) return r; @@ -405,7 +405,7 @@ static int node_get_current(const char *slink, int dirfd, char **ret_id, int *re if (r < 0) return r; - r = device_get_device_id(dev, &id); + r = sd_device_get_device_id(dev, &id); if (r < 0) return r; @@ -446,7 +446,7 @@ static int link_update(sd_device *dev, const char *slink, bool add) { if (current_id) { const char *id; - r = device_get_device_id(dev, &id); + r = sd_device_get_device_id(dev, &id); if (r < 0) return log_device_debug_errno(dev, r, "Failed to get device id: %m"); diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index 1e7b9c04ca..c28c43b2af 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -111,7 +111,7 @@ static int udev_watch_clear(sd_device *dev, int dirfd, int *ret_wd) { assert(dev); assert(dirfd >= 0); - r = device_get_device_id(dev, &id); + r = sd_device_get_device_id(dev, &id); if (r < 0) return log_device_debug_errno(dev, r, "Failed to get device ID: %m"); @@ -188,7 +188,7 @@ int udev_watch_begin(int inotify_fd, sd_device *dev) { if (r < 0) return log_device_debug_errno(dev, r, "Failed to get device node: %m"); - r = device_get_device_id(dev, &id); + r = sd_device_get_device_id(dev, &id); if (r < 0) return log_device_debug_errno(dev, r, "Failed to get device ID: %m"); From cd7c71154cd62d3f50c07ce387edd9c20aebd7bc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Aug 2024 06:24:24 +0900 Subject: [PATCH 5/6] sd-device: make sd_device_new_from_subsystem_sysname() stricter As workarounded by fc0cbed2db860d163d59d04c32fa6ec30bd0606f, the pair of subsystem and sysname is not unique. For examples, - /sys/bus/gpio and /sys/class/gpio, both have gpiochip%N. However, these point to different devpaths. - /sys/bus/mdio_bus and /sys/class/mdio_bus, - /sys/bus/mei and /sys/class/mei, - /sys/bus/typec and /sys/class/typec, and so on. Let's refuse to provide sd_device object in such cases. --- src/libsystemd/sd-device/sd-device.c | 106 ++++++++++++++++------ src/libsystemd/sd-device/test-sd-device.c | 22 +++-- 2 files changed, 89 insertions(+), 39 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 383445edc8..01fa90b1ff 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -386,25 +386,80 @@ _public_ int sd_device_new_from_ifindex(sd_device **ret, int ifindex) { return 0; } -static int device_strjoin_new( +static int device_new_from_path_join( + sd_device **device, + const char *subsystem, + const char *driver_subsystem, + const char *sysname, const char *a, const char *b, const char *c, - const char *d, - sd_device **ret) { + const char *d) { - const char *p; + _cleanup_(sd_device_unrefp) sd_device *new_device = NULL; + _cleanup_free_ char *p = NULL; int r; - p = strjoina(a, b, c, d); - if (access(p, F_OK) < 0) - return IN_SET(errno, ENOENT, ENAMETOOLONG) ? 0 : -errno; /* If this sysfs is too long then it doesn't exist either */ + assert(device); + assert(subsystem); + assert(sysname); - r = sd_device_new_from_syspath(ret, p); + p = path_join(a, b, c, d); + if (!p) + return -ENOMEM; + + r = sd_device_new_from_syspath(&new_device, p); + if (r == -ENODEV) + return 0; if (r < 0) return r; - return 1; + /* Check if the found device really has the expected subsystem and sysname, for safety. */ + if (!device_in_subsystem(new_device, subsystem)) + return 0; + + const char *new_driver_subsystem = NULL; + (void) sd_device_get_driver_subsystem(new_device, &new_driver_subsystem); + + if (!streq_ptr(driver_subsystem, new_driver_subsystem)) + return 0; + + const char *new_sysname; + r = sd_device_get_sysname(new_device, &new_sysname); + if (r < 0) + return r; + + if (!streq(sysname, new_sysname)) + return 0; + + /* If this is the first device we found, then take it. */ + if (!*device) { + *device = TAKE_PTR(new_device); + return 1; + } + + /* Unfortunately, (subsystem, sysname) pair is not unique. For examples, + * - /sys/bus/gpio and /sys/class/gpio, both have gpiochip%N. However, these point to different devpaths. + * - /sys/bus/mdio_bus and /sys/class/mdio_bus, + * - /sys/bus/mei and /sys/class/mei, + * - /sys/bus/typec and /sys/class/typec, and so on. + * Hence, if we already know a device, then we need to check if it is equivalent to the newly found one. */ + + const char *devpath, *new_devpath; + r = sd_device_get_devpath(*device, &devpath); + if (r < 0) + return r; + + r = sd_device_get_devpath(new_device, &new_devpath); + if (r < 0) + return r; + + if (!streq(devpath, new_devpath)) + return log_debug_errno(SYNTHETIC_ERRNO(ETOOMANYREFS), + "sd-device: found multiple devices for subsystem=%s and sysname=%s, refusing: %s, %s", + subsystem, sysname, devpath, new_devpath); + + return 1; /* Fortunately, they are consistent. */ } _public_ int sd_device_new_from_subsystem_sysname( @@ -412,6 +467,7 @@ _public_ int sd_device_new_from_subsystem_sysname( const char *subsystem, const char *sysname) { + _cleanup_(sd_device_unrefp) sd_device *device = NULL; char *name; int r; @@ -430,19 +486,15 @@ _public_ int sd_device_new_from_subsystem_sysname( if (streq(subsystem, "subsystem")) { FOREACH_STRING(s, "/sys/bus/", "/sys/class/") { - r = device_strjoin_new(s, name, NULL, NULL, ret); + r = device_new_from_path_join(&device, subsystem, NULL, sysname, s, name, NULL, NULL); if (r < 0) return r; - if (r > 0) - return 0; } } else if (streq(subsystem, "module")) { - r = device_strjoin_new("/sys/module/", name, NULL, NULL, ret); + r = device_new_from_path_join(&device, subsystem, NULL, sysname, "/sys/module/", name, NULL, NULL); if (r < 0) return r; - if (r > 0) - return 0; } else if (streq(subsystem, "drivers")) { const char *sep; @@ -454,35 +506,31 @@ _public_ int sd_device_new_from_subsystem_sysname( sep++; if (streq(sep, "drivers")) /* If the sysname is "drivers", then it's the drivers directory itself that is meant. */ - r = device_strjoin_new("/sys/bus/", subsys, "/drivers", NULL, ret); + r = device_new_from_path_join(&device, subsystem, subsys, "drivers", "/sys/bus/", subsys, "/drivers", NULL); else - r = device_strjoin_new("/sys/bus/", subsys, "/drivers/", sep, ret); + r = device_new_from_path_join(&device, subsystem, subsys, sep, "/sys/bus/", subsys, "/drivers/", sep); if (r < 0) return r; - if (r > 0) - return 0; } } - r = device_strjoin_new("/sys/bus/", subsystem, "/devices/", name, ret); + r = device_new_from_path_join(&device, subsystem, NULL, sysname, "/sys/bus/", subsystem, "/devices/", name); if (r < 0) return r; - if (r > 0) - return 0; - r = device_strjoin_new("/sys/class/", subsystem, "/", name, ret); + r = device_new_from_path_join(&device, subsystem, NULL, sysname, "/sys/class/", subsystem, name, NULL); if (r < 0) return r; - if (r > 0) - return 0; - r = device_strjoin_new("/sys/firmware/", subsystem, "/", name, ret); + r = device_new_from_path_join(&device, subsystem, NULL, sysname, "/sys/firmware/", subsystem, name, NULL); if (r < 0) return r; - if (r > 0) - return 0; - return -ENODEV; + if (!device) + return -ENODEV; + + *ret = TAKE_PTR(device); + return 0; } _public_ int sd_device_new_from_stat_rdev(sd_device **ret, const struct stat *st) { diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index a048860dbe..620615b6bb 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -74,9 +74,7 @@ static void test_sd_device_one(sd_device *d) { r = sd_device_get_subsystem(d, &subsystem); if (r < 0) assert_se(r == -ENOENT); - else if (!streq(subsystem, "gpio")) { /* Unfortunately, there exist /sys/class/gpio and /sys/bus/gpio. - * Hence, sd_device_new_from_subsystem_sysname() and - * sd_device_new_from_device_id() may not work as expected. */ + else { const char *name, *id; if (streq(subsystem, "drivers")) { @@ -85,10 +83,14 @@ static void test_sd_device_one(sd_device *d) { name = strjoina(driver_subsystem, ":", sysname); } else name = sysname; - assert_se(sd_device_new_from_subsystem_sysname(&dev, subsystem, name) >= 0); - assert_se(sd_device_get_syspath(dev, &val) >= 0); - assert_se(streq(syspath, val)); - dev = sd_device_unref(dev); + + r = sd_device_new_from_subsystem_sysname(&dev, subsystem, name); + if (r >= 0) { + assert_se(sd_device_get_syspath(dev, &val) >= 0); + assert_se(streq(syspath, val)); + dev = sd_device_unref(dev); + } else + ASSERT_ERROR(r, ETOOMANYREFS); /* The device ID depends on subsystem. */ assert_se(sd_device_get_device_id(d, &id) >= 0); @@ -97,12 +99,12 @@ static void test_sd_device_one(sd_device *d) { log_device_warning_errno(d, r, "Failed to create sd-device object from device ID \"%s\". " "Maybe running on a non-host network namespace.", id); - else { - assert_se(r >= 0); + else if (r >= 0) { assert_se(sd_device_get_syspath(dev, &val) >= 0); assert_se(streq(syspath, val)); dev = sd_device_unref(dev); - } + } else + ASSERT_ERROR(r, ETOOMANYREFS); /* These require udev database, and reading database requires device ID. */ r = sd_device_get_is_initialized(d); From 3fe0fd20c9f50948929ef623fd6d47b37e30fd11 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 27 Aug 2024 04:14:12 +0900 Subject: [PATCH 6/6] udevadm/info: also show driver subsystem and device ID This adds two more fields in 'udevadm info': - J for device ID, e.g. b128:1, c10:1, n1, and so on. - B for driver subsystem, e.g. pci, i2c, and so on. These, especially the device ID field may be useful to find udev database file under /run/udev/data for a device. --- man/udevadm.xml | 8 ++++++++ src/udev/udevadm-info.c | 23 +++++++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/man/udevadm.xml b/man/udevadm.xml index b515f3c79c..556f845dce 100644 --- a/man/udevadm.xml +++ b/man/udevadm.xml @@ -336,10 +336,18 @@ R: Device number in /sys/ (i.e. the numeric suffix of the last component of P:) + + J: + Device ID + U: Kernel subsystem + + B: + Driver subsystem + T: Kernel device type within subsystem diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c index 824b6fc726..0a1da3ed31 100644 --- a/src/udev/udevadm-info.c +++ b/src/udev/udevadm-info.c @@ -323,9 +323,15 @@ static int print_record(sd_device *device, const char *prefix) { if (sd_device_get_sysnum(device, &str) >= 0) printf("%sR: %s%s%s\n", prefix, ansi_highlight_white(), str, ansi_normal()); + if (sd_device_get_device_id(device, &str) >= 0) + printf("%sJ: %s%s%s\n", prefix, ansi_highlight_white(), str, ansi_normal()); + if (sd_device_get_subsystem(device, &subsys) >= 0) printf("%sU: %s%s%s\n", prefix, ansi_highlight_green(), subsys, ansi_normal()); + if (sd_device_get_driver_subsystem(device, &str) >= 0) + printf("%sB: %s%s%s\n", prefix, ansi_highlight_green(), str, ansi_normal()); + if (sd_device_get_devtype(device, &str) >= 0) printf("%sT: %s%s%s\n", prefix, ansi_highlight_green(), str, ansi_normal()); @@ -376,8 +382,9 @@ static int record_to_json(sd_device *device, sd_json_variant **ret) { assert(device); assert(ret); - /* We don't show any shorthand fields here as done in print_record() except for SYSNAME and SYSNUM as - * all the other ones have a matching property which will already be included. */ + /* We don't show any shorthand fields here as done in print_record() except for SYSNAME, SYSNUM, + * DRIVER_SUBSYSTEM, and DEVICE_ID, as all the other ones have a matching property which will already + * be included. */ if (sd_device_get_sysname(device, &str) >= 0) { r = sd_json_variant_set_field_string(&v, "SYSNAME", str); @@ -391,6 +398,18 @@ static int record_to_json(sd_device *device, sd_json_variant **ret) { return r; } + if (sd_device_get_driver_subsystem(device, &str) >= 0) { + r = sd_json_variant_set_field_string(&v, "DRIVER_SUBSYSTEM", str); + if (r < 0) + return r; + } + + if (sd_device_get_device_id(device, &str) >= 0) { + r = sd_json_variant_set_field_string(&v, "DEVICE_ID", str); + if (r < 0) + return r; + } + FOREACH_DEVICE_PROPERTY(device, key, val) { r = sd_json_variant_set_field_string(&v, key, val); if (r < 0)