From 42c689f63de53089e573cbb945d84477a91b8248 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 05:19:25 +0900 Subject: [PATCH 01/20] sd-device: logs about refused property Follow-up for a62cd5a153ffe18c27aff02685ed75c5bc4509a2. --- src/libsystemd/sd-device/sd-device.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index ae44d0c86f..9d9ac7d430 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -15,6 +15,7 @@ #include "dirent-util.h" #include "env-util.h" #include "errno-util.h" +#include "escape.h" #include "extract-word.h" #include "fd-util.h" #include "fileio.h" @@ -108,8 +109,15 @@ int device_add_property_aux(sd_device *device, const char *key, const char *valu assert(device); assert(key); - if (!property_is_valid(key, value)) + if (!property_is_valid(key, value)) { + if (DEBUG_LOGGING) { + _cleanup_free_ char *escaped_key = cescape(key), + *escaped_value = cescape(strempty(value)); + log_device_debug(device, "sd-device: Refusing invalid property: %s=%s", + strnull(escaped_key), strnull(escaped_value)); + } return -EINVAL; + } if (db) properties = &device->properties_db; From 8e8d3a22b2a6f766f9251a7c8963ca3d1c033788 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 18 May 2026 01:17:42 +0900 Subject: [PATCH 02/20] sd-device: use string_is_safe() at one more place --- src/libsystemd/sd-device/sd-device.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 9d9ac7d430..307137b431 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -31,7 +31,6 @@ #include "string-util.h" #include "strv.h" #include "time-util.h" -#include "utf8.h" int device_new_aux(sd_device **ret) { sd_device *device; @@ -97,10 +96,10 @@ static bool property_is_valid(const char *key, const char *value) { return true; /* refuse invalid UTF8 and control characters */ - if (!utf8_is_valid(value) || string_has_cc(value, /* ok= */ NULL)) - return false; - - return true; + return string_is_safe(value, + STRING_ALLOW_BACKSLASHES | + STRING_ALLOW_QUOTES | + STRING_ALLOW_GLOBS); } int device_add_property_aux(sd_device *device, const char *key, const char *value, bool db) { From b74f016e598c2428b55db1b04788e82061f79153 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 18 May 2026 00:18:23 +0900 Subject: [PATCH 03/20] string-util: introduce STRING_ALLOW_NEWLINES switch for string_is_safe() This also make StringSafeFlags defined in basic-forward.h so other headers can use the enum. --- src/basic/basic-forward.h | 1 + src/basic/string-util.c | 6 +++++- src/basic/string-util.h | 9 +++++---- src/test/test-string-util.c | 25 ++++++++++++++++++++++++- 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/basic/basic-forward.h b/src/basic/basic-forward.h index 064af284a6..5edd494f78 100644 --- a/src/basic/basic-forward.h +++ b/src/basic/basic-forward.h @@ -103,6 +103,7 @@ typedef enum Glyph Glyph; typedef enum ImageClass ImageClass; typedef enum JobMode JobMode; typedef enum RuntimeScope RuntimeScope; +typedef enum StringSafeFlags StringSafeFlags; typedef enum TimestampStyle TimestampStyle; typedef enum UnitActiveState UnitActiveState; typedef enum UnitDependency UnitDependency; diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 4d93083923..b972f7a42c 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1113,7 +1113,11 @@ bool string_is_safe(const char *p, StringSafeFlags flags) { return false; for (const char *t = p; *t; t++) { - if ((*t > 0 && *t < ' ') || *t == 0x7f) /* never allow control characters */ + /* never allow control characters, except for new line */ + if ((*t > 0 && *t < ' ' && *t != '\n') || *t == 0x7f) + return false; + + if (!FLAGS_SET(flags, STRING_ALLOW_NEWLINES) && *t == '\n') return false; if (!FLAGS_SET(flags, STRING_ALLOW_BACKSLASHES) && *t == '\\') diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 17eaf5d9a6..8cddc051d5 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -223,10 +223,11 @@ static inline int strdup_to(char **ret, const char *src) { typedef enum StringSafeFlags { STRING_ASCII = 1 << 0, /* Verify string is 7-Bit ASCII (rather than just UTF-8) */ STRING_ALLOW_EMPTY = 1 << 1, /* Allow empty strings */ - STRING_ALLOW_BACKSLASHES = 1 << 2, /* Allow backslashes (\) */ - STRING_ALLOW_QUOTES = 1 << 3, /* Allow quotes (" or ') */ - STRING_ALLOW_GLOBS = 1 << 4, /* Allow globs (?, * or [) */ - STRING_FILENAME = 1 << 5, /* Verify the string is valid as regular filename */ + STRING_ALLOW_NEWLINES = 1 << 2, /* Allow newlines (\n) */ + STRING_ALLOW_BACKSLASHES = 1 << 3, /* Allow backslashes (\) */ + STRING_ALLOW_QUOTES = 1 << 4, /* Allow quotes (" or ') */ + STRING_ALLOW_GLOBS = 1 << 5, /* Allow globs (?, * or [) */ + STRING_FILENAME = 1 << 6, /* Verify the string is valid as regular filename */ } StringSafeFlags; bool string_is_safe(const char *p, StringSafeFlags flags) _pure_; diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 6e07d72763..4a8656f411 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -1547,6 +1547,18 @@ TEST(string_is_safe) { ASSERT_FALSE(string_is_safe("a\"b", STRING_ASCII)); ASSERT_FALSE(string_is_safe("a*b", STRING_ASCII)); + /* STRING_ALLOW_NEWLINES: newlines allowed, quotes/globs still rejected. */ + ASSERT_TRUE(string_is_safe("hello", STRING_ALLOW_NEWLINES)); + ASSERT_TRUE(string_is_safe("hello world", STRING_ALLOW_NEWLINES)); + ASSERT_TRUE(string_is_safe("\n", STRING_ALLOW_NEWLINES)); + ASSERT_TRUE(string_is_safe("a\nb", STRING_ALLOW_NEWLINES)); + ASSERT_TRUE(string_is_safe("foo\n", STRING_ALLOW_NEWLINES)); + ASSERT_TRUE(string_is_safe("\nfoo", STRING_ALLOW_NEWLINES)); + ASSERT_TRUE(string_is_safe("foo\nbar", STRING_ALLOW_NEWLINES)); + ASSERT_FALSE(string_is_safe("foo\\nbar", STRING_ALLOW_NEWLINES)); /* literal backslash, not newline, rejected */ + ASSERT_FALSE(string_is_safe("\"", STRING_ALLOW_NEWLINES)); /* quotes still rejected */ + ASSERT_FALSE(string_is_safe("*", STRING_ALLOW_NEWLINES)); /* globs still rejected */ + /* STRING_ALLOW_BACKSLASHES: backslashes allowed, quotes/globs still rejected. */ ASSERT_TRUE(string_is_safe("hello", STRING_ALLOW_BACKSLASHES)); ASSERT_TRUE(string_is_safe("hello world", STRING_ALLOW_BACKSLASHES)); @@ -1555,6 +1567,7 @@ TEST(string_is_safe) { ASSERT_TRUE(string_is_safe("foo\\", STRING_ALLOW_BACKSLASHES)); ASSERT_TRUE(string_is_safe("\\foo", STRING_ALLOW_BACKSLASHES)); ASSERT_TRUE(string_is_safe("foo\\nbar", STRING_ALLOW_BACKSLASHES)); /* literal backslash, not newline */ + ASSERT_FALSE(string_is_safe("foo\nbar", STRING_ALLOW_BACKSLASHES)); /* newline still rejected */ ASSERT_FALSE(string_is_safe("\"", STRING_ALLOW_BACKSLASHES)); /* quotes still rejected */ ASSERT_FALSE(string_is_safe("*", STRING_ALLOW_BACKSLASHES)); /* globs still rejected */ @@ -1565,6 +1578,7 @@ TEST(string_is_safe) { ASSERT_TRUE(string_is_safe("'", STRING_ALLOW_QUOTES)); ASSERT_TRUE(string_is_safe("hello\"world", STRING_ALLOW_QUOTES)); ASSERT_TRUE(string_is_safe("it's", STRING_ALLOW_QUOTES)); + ASSERT_FALSE(string_is_safe("a\nb", STRING_ALLOW_QUOTES)); /* newline still rejected */ ASSERT_FALSE(string_is_safe("a\\b", STRING_ALLOW_QUOTES)); /* backslashes still rejected */ ASSERT_FALSE(string_is_safe("*", STRING_ALLOW_QUOTES)); /* globs still rejected */ @@ -1577,6 +1591,7 @@ TEST(string_is_safe) { ASSERT_TRUE(string_is_safe("foo*bar", STRING_ALLOW_GLOBS)); ASSERT_TRUE(string_is_safe("foo?bar", STRING_ALLOW_GLOBS)); ASSERT_TRUE(string_is_safe("foo[bar", STRING_ALLOW_GLOBS)); + ASSERT_FALSE(string_is_safe("foo\nbar", STRING_ALLOW_GLOBS)); /* newline still rejected */ ASSERT_FALSE(string_is_safe("\"", STRING_ALLOW_GLOBS)); /* quotes still rejected */ ASSERT_FALSE(string_is_safe("a\\b", STRING_ALLOW_GLOBS)); /* backslashes still rejected */ @@ -1618,10 +1633,18 @@ TEST(string_is_safe) { ASSERT_TRUE(string_is_safe("foo\"bar", STRING_ALLOW_BACKSLASHES | STRING_ALLOW_QUOTES)); /* All allow flags combined: only baseline (control chars, invalid UTF-8) and STRING_FILENAME apply. */ - StringSafeFlags all = STRING_ALLOW_EMPTY | STRING_ASCII | STRING_ALLOW_BACKSLASHES | STRING_ALLOW_QUOTES | STRING_ALLOW_GLOBS | STRING_FILENAME; + StringSafeFlags all = + STRING_ASCII | + STRING_ALLOW_EMPTY | + STRING_ALLOW_NEWLINES | + STRING_ALLOW_BACKSLASHES | + STRING_ALLOW_QUOTES | + STRING_ALLOW_GLOBS | + STRING_FILENAME; ASSERT_TRUE(string_is_safe("hello.txt", all)); ASSERT_TRUE(string_is_safe("foo-bar_baz.conf", all)); ASSERT_TRUE(string_is_safe("a", all)); + ASSERT_TRUE(string_is_safe("foo\nbar", all)); /* newline allowed */ ASSERT_TRUE(string_is_safe("foo\\bar", all)); /* backslash allowed */ ASSERT_TRUE(string_is_safe("foo\"bar", all)); /* quote allowed */ ASSERT_TRUE(string_is_safe("foo'bar", all)); /* quote allowed */ From 0341058ee6a237f4990be0825218dfdd4587a0d0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 05:39:42 +0900 Subject: [PATCH 04/20] sd-device: introduce device_get_sysattr_safe_string() This introduce a helper function that verifies read sysattr value. Some sysattr value may come from firmware, and kernel may expose unsafe string. The introduced function should be used when we use the result as a string. --- src/libsystemd/sd-device/device-private.h | 1 + src/libsystemd/sd-device/sd-device.c | 28 +++++++++++++++++++++++ src/libsystemd/sd-device/test-sd-device.c | 7 ++++++ 3 files changed, 36 insertions(+) diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index f2f349f2ac..27aa6bf2fc 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -16,6 +16,7 @@ int device_get_property_bool(sd_device *device, const char *key); int device_get_property_int(sd_device *device, const char *key, int *ret); int device_get_property_uint(sd_device *device, const char *key, unsigned *ret); int device_get_ifname(sd_device *device, const char **ret); +int device_get_sysattr_safe_string(sd_device *device, const char *sysattr, const char **ret); int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret_value); int device_get_sysattr_unsigned_full(sd_device *device, const char *sysattr, unsigned base, unsigned *ret_value); static inline int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned *ret_value) { diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 307137b431..09a48a74c5 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -2619,6 +2619,34 @@ _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, return sd_device_get_sysattr_value_with_size(device, sysattr, ret_value, NULL); } +int device_get_sysattr_safe_string(sd_device *device, const char *sysattr, const char **ret) { + const char *value; + int r; + + r = sd_device_get_sysattr_value(device, sysattr, &value); + if (r < 0) + return r; + + if (!string_is_safe(value, + STRING_ALLOW_EMPTY | + STRING_ALLOW_NEWLINES | + STRING_ALLOW_BACKSLASHES | + STRING_ALLOW_QUOTES | + STRING_ALLOW_GLOBS)) { + if (DEBUG_LOGGING) { + _cleanup_free_ char *escaped = cescape(value); + log_device_debug(device, "sd-device: '%s' sysattr contains invalid characters, refusing: %s", + sysattr, strnull(escaped)); + } + return -ENXIO; + } + + if (ret) + *ret = value; + + return 0; +} + int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret_value) { const char *value; int r; diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index bf62e9082b..9201919898 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -305,6 +305,13 @@ static void test_sd_device_one(sd_device *d) { ASSERT_OK(r = device_get_sysattr_unsigned(d, "nsid", &x)); ASSERT_EQ(x > 0, r > 0); } + + const char *uevent; + if (sd_device_get_sysattr_value(d, "uevent", &uevent) >= 0) { + const char *uevent_safe; + ASSERT_OK(device_get_sysattr_safe_string(d, "uevent", &uevent_safe)); + ASSERT_STREQ(uevent, uevent_safe); + } } static void exclude_problematic_devices(sd_device_enumerator *e) { From 13ce62caba10e0dfa715dfa5c3ab551dedb02095 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 05:44:51 +0900 Subject: [PATCH 05/20] sd-device: use device_get_sysattr_safe_string() The read value are exposed by sd_device_get_subsystem() and friends. Hence, it is better to filter invalid characters. Of course, these should be always safe unless the kernel is buggy. But, just for safety. Note, even if uevent file contains invalid characters, then device_read_uevent_file() should succeed without parsing the contents. The caller should fail later with a proper error code if a necessary field is unset. E.g. sd_device_get_ifindex() should still return -ENOENT even when uevent file contains an invalid characters. --- src/libsystemd/sd-device/sd-device.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 09a48a74c5..4d1693e74c 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -837,7 +837,7 @@ int device_read_uevent_file(sd_device *device) { device->uevent_loaded = true; const char *uevent; - r = sd_device_get_sysattr_value(device, "uevent", &uevent); + r = device_get_sysattr_safe_string(device, "uevent", &uevent); if (ERRNO_IS_NEG_PRIVILEGE(r) || ERRNO_IS_NEG_DEVICE_ABSENT(r)) /* The uevent files may be write-only, the device may be already removed, or the device * may not have the uevent file. */ @@ -1258,7 +1258,7 @@ _public_ int sd_device_get_subsystem(sd_device *device, const char **ret) { if (!device->subsystem_set) { const char *subsystem; - r = sd_device_get_sysattr_value(device, "subsystem", &subsystem); + r = device_get_sysattr_safe_string(device, "subsystem", &subsystem); if (r < 0 && r != -ENOENT) return log_device_debug_errno(device, r, "sd-device: Failed to read subsystem for %s: %m", @@ -1398,7 +1398,7 @@ _public_ int sd_device_get_driver(sd_device *device, const char **ret) { if (!device->driver_set) { const char *driver = NULL; - r = sd_device_get_sysattr_value(device, "driver", &driver); + r = device_get_sysattr_safe_string(device, "driver", &driver); if (r < 0 && r != -ENOENT) return log_device_debug_errno(device, r, "sd-device: Failed to read driver: %m"); From d0e48e2601aa990f4761985796a9098333d6703c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 07:09:44 +0900 Subject: [PATCH 06/20] udev: use device_get_sysattr_safe_string() in device_get_sysattr_value_filtered() Then, renamed to device_get_sysattr_safe_string_filtered(). --- src/shared/netif-naming-scheme.c | 4 ++-- src/shared/netif-naming-scheme.h | 2 +- src/udev/udev-builtin-net_id.c | 20 ++++++++++---------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/shared/netif-naming-scheme.c b/src/shared/netif-naming-scheme.c index 607094eecf..ddf3bdde9e 100644 --- a/src/shared/netif-naming-scheme.c +++ b/src/shared/netif-naming-scheme.c @@ -178,7 +178,7 @@ int device_get_sysattr_bool_filtered(sd_device *device, const char *sysattr) { return device_get_sysattr_bool(device, sysattr); } -int device_get_sysattr_value_filtered(sd_device *device, const char *sysattr, const char **ret_value) { +int device_get_sysattr_safe_string_filtered(sd_device *device, const char *sysattr, const char **ret_value) { int r; r = naming_sysattr_allowed(device, sysattr); @@ -187,5 +187,5 @@ int device_get_sysattr_value_filtered(sd_device *device, const char *sysattr, co if (r == 0) return -ENOENT; - return sd_device_get_sysattr_value(device, sysattr, ret_value); + return device_get_sysattr_safe_string(device, sysattr, ret_value); } diff --git a/src/shared/netif-naming-scheme.h b/src/shared/netif-naming-scheme.h index c13d7649c9..52c462a6bb 100644 --- a/src/shared/netif-naming-scheme.h +++ b/src/shared/netif-naming-scheme.h @@ -104,4 +104,4 @@ DECLARE_STRING_TABLE_LOOKUP(alternative_names_policy, NamePolicy); int device_get_sysattr_int_filtered(sd_device *device, const char *sysattr, int *ret_value); int device_get_sysattr_unsigned_filtered(sd_device *device, const char *sysattr, unsigned *ret_value); int device_get_sysattr_bool_filtered(sd_device *device, const char *sysattr); -int device_get_sysattr_value_filtered(sd_device *device, const char *sysattr, const char **ret_value); +int device_get_sysattr_safe_string_filtered(sd_device *device, const char *sysattr, const char **ret_value); diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index 4e962d54de..018edcc6d9 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -183,7 +183,7 @@ static int get_port_specifier(sd_device *dev, char **ret) { assert(ret); /* First, try to use the kernel provided front panel port name for multiple port PCI device. */ - r = device_get_sysattr_value_filtered(dev, "phys_port_name", &phys_port_name); + r = device_get_sysattr_safe_string_filtered(dev, "phys_port_name", &phys_port_name); if (r >= 0 && !isempty(phys_port_name)) { if (naming_scheme_has(NAMING_SR_IOV_R)) { int vf_id = -1; @@ -304,7 +304,7 @@ static int names_pci_onboard_label(UdevEvent *event, sd_device *pci_dev, const c assert(prefix); /* retrieve on-board label from firmware */ - r = device_get_sysattr_value_filtered(pci_dev, "label", &label); + r = device_get_sysattr_safe_string_filtered(pci_dev, "label", &label); if (r < 0) return log_device_debug_errno(pci_dev, r, "Failed to get PCI onboard label: %m"); @@ -360,7 +360,7 @@ static bool is_pci_bridge(sd_device *dev) { assert(dev); - if (device_get_sysattr_value_filtered(dev, "modalias", &v) < 0) + if (device_get_sysattr_safe_string_filtered(dev, "modalias", &v) < 0) return false; if (!startswith(v, "pci:")) @@ -402,7 +402,7 @@ static int parse_hotplug_slot_from_function_id(sd_device *dev, int slots_dirfd, return 0; } - if (device_get_sysattr_value_filtered(dev, "function_id", &attr) < 0) { + if (device_get_sysattr_safe_string_filtered(dev, "function_id", &attr) < 0) { *ret = 0; return 0; } @@ -465,7 +465,7 @@ static int pci_get_hotplug_slot_from_address( if (!path) return log_oom_debug(); - if (device_get_sysattr_value_filtered(pci, path, &address) < 0) + if (device_get_sysattr_safe_string_filtered(pci, path, &address) < 0) continue; /* match slot address with device by stripping the function */ @@ -544,7 +544,7 @@ static int get_device_firmware_node_sun(sd_device *dev, uint32_t *ret) { assert(dev); assert(ret); - r = device_get_sysattr_value_filtered(dev, "firmware_node/sun", &attr); + r = device_get_sysattr_safe_string_filtered(dev, "firmware_node/sun", &attr); if (r < 0) return log_device_debug_errno(dev, r, "Failed to read firmware_node/sun, ignoring: %m"); @@ -876,7 +876,7 @@ static int names_devicetree_alias_prefix(UdevEvent *event, const char *prefix, c if (!alias_index) continue; - if (device_get_sysattr_value_filtered(aliases_dev, alias, &alias_path) < 0) + if (device_get_sysattr_safe_string_filtered(aliases_dev, alias, &alias_path) < 0) continue; if (!path_equal(ofnode_path, alias_path)) @@ -895,7 +895,7 @@ static int names_devicetree_alias_prefix(UdevEvent *event, const char *prefix, c } /* ...but make sure we don't have an alias conflict */ - if (i == 0 && device_get_sysattr_value_filtered(aliases_dev, conflict, NULL) >= 0) + if (i == 0 && device_get_sysattr_safe_string_filtered(aliases_dev, conflict, NULL) >= 0) return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EEXIST), "DeviceTree alias conflict: %s and %s both exist.", alias_prefix, alias_prefix_0); @@ -1208,7 +1208,7 @@ static int names_mac(UdevEvent *event, const char *prefix) { return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "addr_assign_type=%u, MAC address is not permanent.", assign_type); - r = device_get_sysattr_value_filtered(dev, "address", &s); + r = device_get_sysattr_safe_string_filtered(dev, "address", &s); if (r < 0) return log_device_debug_errno(dev, r, "Failed to read 'address' attribute: %m"); @@ -1253,7 +1253,7 @@ static int names_netdevsim(UdevEvent *event, const char *prefix) { if (r < 0) return log_device_debug_errno(netdevsimdev, r, "Failed to get device sysnum: %m"); - r = device_get_sysattr_value_filtered(dev, "phys_port_name", &phys_port_name); + r = device_get_sysattr_safe_string_filtered(dev, "phys_port_name", &phys_port_name); if (r < 0) return log_device_debug_errno(dev, r, "Failed to get 'phys_port_name' attribute: %m"); if (isempty(phys_port_name)) From c6ea72e39a8d829b1bd65f15f6dd7d1c2b6d04c3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 07:14:44 +0900 Subject: [PATCH 07/20] Revert "udev-builtin-net-id: print cescaped bad attributes" This reverts commit 7c4047957ef58744ecfad6d277f7c45d430f6d70. This is not necessary, as bad characters are already filtered. --- src/udev/udev-builtin-net_id.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index 018edcc6d9..39579f897a 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -22,7 +22,6 @@ #include "device-private.h" #include "device-util.h" #include "dirent-util.h" -#include "escape.h" #include "ether-addr-util.h" #include "fd-util.h" #include "fileio.h" @@ -37,12 +36,6 @@ #define ONBOARD_14BIT_INDEX_MAX ((1U << 14) - 1) #define ONBOARD_16BIT_INDEX_MAX ((1U << 16) - 1) -static int log_invalid_device_attr(sd_device *dev, const char *attr, const char *value) { - _cleanup_free_ char *escaped = cescape(value); - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), - "Invalid %s value '%s'.", attr, strnull(escaped)); -} - static int device_get_parent_skip_virtio(sd_device *dev, sd_device **ret) { int r; @@ -202,7 +195,7 @@ static int get_port_specifier(sd_device *dev, char **ret) { } if (!utf8_is_valid(phys_port_name) || string_has_cc(phys_port_name, /* ok= */ NULL)) - return log_invalid_device_attr(dev, "phys_port_name", phys_port_name); + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invalid phys_port_name"); /* Otherwise, use phys_port_name as is. */ buf = strjoin("n", phys_port_name); @@ -309,7 +302,7 @@ static int names_pci_onboard_label(UdevEvent *event, sd_device *pci_dev, const c return log_device_debug_errno(pci_dev, r, "Failed to get PCI onboard label: %m"); if (!utf8_is_valid(label) || string_has_cc(label, /* ok= */ NULL)) - return log_invalid_device_attr(dev, "label", label); + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invalid label"); char str[ALTIFNAMSIZ]; if (snprintf_ok(str, sizeof str, "%s%s", @@ -724,7 +717,8 @@ static int names_vio(UdevEvent *event, const char *prefix) { "VIO bus ID and slot ID have invalid length: %s", s); if (!in_charset(s, HEXDIGITS)) - return log_invalid_device_attr(dev, "VIO bus ID and slot ID", s); + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), + "VIO bus ID and slot ID contain invalid characters: %s", s); /* Parse only slot ID (the last 4 hexdigits). */ r = safe_atou_full(s + 4, 16, &slotid); @@ -780,7 +774,8 @@ static int names_platform(UdevEvent *event, const char *prefix) { return -EOPNOTSUPP; if (!in_charset(vendor, validchars)) - return log_invalid_device_attr(dev, "platform vendor", vendor); + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(ENOENT), + "Platform vendor contains invalid characters: %s", vendor); ascii_strlower(vendor); @@ -1260,7 +1255,7 @@ static int names_netdevsim(UdevEvent *event, const char *prefix) { return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EOPNOTSUPP), "The 'phys_port_name' attribute is empty."); if (!utf8_is_valid(phys_port_name) || string_has_cc(phys_port_name, /* ok= */ NULL)) - return log_invalid_device_attr(dev, "phys_port_name", phys_port_name); + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invalid phys_port_name"); char str[ALTIFNAMSIZ]; if (snprintf_ok(str, sizeof str, "%si%un%s", prefix, addr, phys_port_name)) From c5a04f59d912fb2c3451994279dea3ae3064f6c7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 07:16:46 +0900 Subject: [PATCH 08/20] udev: drop redundant checks This partially reverts 16325b35fa6ecb25f66534a562583ce3b96d52f3, as bad characters are already filtered. --- src/udev/udev-builtin-net_id.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index 39579f897a..bbf719bba5 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -31,7 +31,6 @@ #include "stdio-util.h" #include "string-util.h" #include "udev-builtin.h" -#include "utf8.h" #define ONBOARD_14BIT_INDEX_MAX ((1U << 14) - 1) #define ONBOARD_16BIT_INDEX_MAX ((1U << 16) - 1) @@ -194,9 +193,6 @@ static int get_port_specifier(sd_device *dev, char **ret) { } } - if (!utf8_is_valid(phys_port_name) || string_has_cc(phys_port_name, /* ok= */ NULL)) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invalid phys_port_name"); - /* Otherwise, use phys_port_name as is. */ buf = strjoin("n", phys_port_name); if (!buf) @@ -301,9 +297,6 @@ static int names_pci_onboard_label(UdevEvent *event, sd_device *pci_dev, const c if (r < 0) return log_device_debug_errno(pci_dev, r, "Failed to get PCI onboard label: %m"); - if (!utf8_is_valid(label) || string_has_cc(label, /* ok= */ NULL)) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invalid label"); - char str[ALTIFNAMSIZ]; if (snprintf_ok(str, sizeof str, "%s%s", naming_scheme_has(NAMING_LABEL_NOPREFIX) ? "" : prefix, @@ -1254,8 +1247,6 @@ static int names_netdevsim(UdevEvent *event, const char *prefix) { if (isempty(phys_port_name)) return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EOPNOTSUPP), "The 'phys_port_name' attribute is empty."); - if (!utf8_is_valid(phys_port_name) || string_has_cc(phys_port_name, /* ok= */ NULL)) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invalid phys_port_name"); char str[ALTIFNAMSIZ]; if (snprintf_ok(str, sizeof str, "%si%un%s", prefix, addr, phys_port_name)) From 35edd031b9bc7e11eed5cd84c341560633b5afa3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 14 Mar 2026 05:40:14 +0900 Subject: [PATCH 09/20] udev-builtin-net_id: use EINVAL when we get unexpected sysattr value The results are completely ignored or only checked if it is negative or not. Hence, this does not change anything. --- src/udev/udev-builtin-net_id.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index bbf719bba5..1f04075723 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -249,7 +249,7 @@ static int pci_get_onboard_index(sd_device *dev, unsigned *ret) { return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Naming scheme does not allow onboard index==0."); if (!is_valid_onboard_index(idx)) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(ENOENT), + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Not a valid onboard index: %u", idx); *ret = idx; @@ -767,7 +767,7 @@ static int names_platform(UdevEvent *event, const char *prefix) { return -EOPNOTSUPP; if (!in_charset(vendor, validchars)) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(ENOENT), + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Platform vendor contains invalid characters: %s", vendor); ascii_strlower(vendor); From fdce4d46872370585b04f96487feb70679a80f48 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 18 May 2026 00:52:29 +0900 Subject: [PATCH 10/20] udev-builtin-net_id: quote strings in log messages --- src/udev/udev-builtin-net_id.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index 1f04075723..2491484cf6 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -395,7 +395,7 @@ static int parse_hotplug_slot_from_function_id(sd_device *dev, int slots_dirfd, r = safe_atou64(attr, &function_id); if (r < 0) - return log_device_debug_errno(dev, r, "Failed to parse function_id, ignoring: %s", attr); + return log_device_debug_errno(dev, r, "Failed to parse function_id, ignoring: '%s'", attr); if (function_id <= 0 || function_id > UINT32_MAX) return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), @@ -407,7 +407,7 @@ static int parse_hotplug_slot_from_function_id(sd_device *dev, int slots_dirfd, "PCI slot path is too long, ignoring."); if (faccessat(slots_dirfd, filename, F_OK, 0) < 0) - return log_device_debug_errno(dev, errno, "Cannot access %s under pci slots, ignoring: %m", filename); + return log_device_debug_errno(dev, errno, "Cannot access '%s' under pci slots, ignoring: %m", filename); *ret = (uint32_t) function_id; return 1; /* Found. We should ignore domain part. */ @@ -707,11 +707,11 @@ static int names_vio(UdevEvent *event, const char *prefix) { if (r != 8) return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), - "VIO bus ID and slot ID have invalid length: %s", s); + "VIO bus ID and slot ID have invalid length: '%s'", s); if (!in_charset(s, HEXDIGITS)) return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), - "VIO bus ID and slot ID contain invalid characters: %s", s); + "VIO bus ID and slot ID contain invalid characters: '%s'", s); /* Parse only slot ID (the last 4 hexdigits). */ r = safe_atou_full(s + 4, 16, &slotid); @@ -768,7 +768,7 @@ static int names_platform(UdevEvent *event, const char *prefix) { if (!in_charset(vendor, validchars)) return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), - "Platform vendor contains invalid characters: %s", vendor); + "Platform vendor contains invalid characters: '%s'", vendor); ascii_strlower(vendor); @@ -878,14 +878,14 @@ static int names_devicetree_alias_prefix(UdevEvent *event, const char *prefix, c r = safe_atou(alias_index, &i); if (r < 0) return log_device_debug_errno(dev, r, - "Could not get index of alias %s: %m", alias); + "Could not get index of alias '%s': %m", alias); conflict = alias_prefix; } /* ...but make sure we don't have an alias conflict */ if (i == 0 && device_get_sysattr_safe_string_filtered(aliases_dev, conflict, NULL) >= 0) return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EEXIST), - "DeviceTree alias conflict: %s and %s both exist.", + "DeviceTree alias conflict: '%s' and '%s' both exist.", alias_prefix, alias_prefix_0); char str[ALTIFNAMSIZ]; @@ -1121,7 +1121,7 @@ static int names_ccw(UdevEvent *event, const char *prefix) { */ bus_id_len = strlen(bus_id); if (!IN_SET(bus_id_len, 8, 9)) - return log_device_debug_errno(cdev, SYNTHETIC_ERRNO(EINVAL), "Invalid bus_id: %s", bus_id); + return log_device_debug_errno(cdev, SYNTHETIC_ERRNO(EINVAL), "Invalid bus_id: '%s'", bus_id); /* Strip leading zeros from the bus id for aesthetic purposes. This * keeps the ccw names stable, yet much shorter in general case of @@ -1281,7 +1281,7 @@ static int names_xen(UdevEvent *event, const char *prefix) { p = startswith(vif, "vif-"); if (!p) - return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invalid vif name: %s.", vif); + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "Invalid vif name: '%s'.", vif); r = safe_atou_full(p, SAFE_ATO_REFUSE_PLUS_MINUS | SAFE_ATO_REFUSE_LEADING_ZERO | SAFE_ATO_REFUSE_LEADING_WHITESPACE | 10, &id); From 06d687ffacecac22cbe5a99723e99bf0401757a1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 07:00:06 +0900 Subject: [PATCH 11/20] tree-wide: use device_get_sysattr_safe_string() The obtained strings are passed to another function, e,g, handled as a path and opened, printed to the terminal, written to a file, saved to udev database as udev property, exposed through DBus, passed to logger, and so on. Hence, these should not contain any malicious characters. --- src/backlight/backlight.c | 4 ++-- src/hostname/hostnamed.c | 11 +++++++---- src/login/sysfs-show.c | 5 +++-- src/mount/mount-tool.c | 3 ++- src/network/networkd-sriov.c | 3 ++- src/rfkill/rfkill.c | 3 ++- src/shared/battery-util.c | 6 +++--- src/shared/loop-util.c | 3 ++- src/udev/udev-builtin-hwdb.c | 3 ++- src/udev/udev-builtin-input_id.c | 3 ++- src/udev/udev-builtin-keyboard.c | 3 ++- src/udev/udev-builtin-path_id.c | 26 +++++++++++++------------- src/udev/udev-builtin-usb_id.c | 5 +++-- src/udev/udevadm-info.c | 4 ++-- 14 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index 7e296c9ffc..29ce29fab9 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -281,7 +281,7 @@ static int validate_device(sd_device *device) { const char *other_sysname = NULL, *other_type = NULL; (void) sd_device_get_sysname(other, &other_sysname); - (void) sd_device_get_sysattr_value(other, "type", &other_type); + (void) device_get_sysattr_safe_string(other, "type", &other_type); log_device_debug(device, "Found another %s backlight device %s on the same PCI, skipping.", strna(other_type), strna(other_sysname)); @@ -295,7 +295,7 @@ static int validate_device(sd_device *device) { const char *other_sysname = NULL, *other_type = NULL; (void) sd_device_get_sysname(other, &other_sysname); - (void) sd_device_get_sysattr_value(other, "type", &other_type); + (void) device_get_sysattr_safe_string(other, "type", &other_type); log_device_debug(device, "Found another %s backlight device %s, which has higher precedence, skipping.", strna(other_type), strna(other_sysname)); diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 5ec7b2fea9..2292c07661 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -20,6 +20,7 @@ #include "constants.h" #include "daemon-util.h" #include "device-private.h" +#include "device-util.h" #include "env-file.h" #include "env-util.h" #include "extract-word.h" @@ -442,12 +443,14 @@ static int get_sysattr(sd_device *device, const char *key, char **ret) { if (!device) return -ENODEV; - r = sd_device_get_sysattr_value(device, key, &s); + r = device_get_sysattr_safe_string(device, key, &s); if (r < 0) - return r; + return log_device_debug_errno(device, r, "Failed to read '%s' attribute: %m", key); if (!string_is_safe_for_dbus(s)) - return -ENXIO; + return log_device_debug_errno(device, SYNTHETIC_ERRNO(ENXIO), + "'%s' attribute is not safe for exposing through DBus: %s", + key, s); return strdup_to(ret, empty_to_null(s)); } @@ -703,7 +706,7 @@ static const char* fallback_chassis_by_device_tree(Context *c) { if (!c->device_tree) return NULL; - r = sd_device_get_sysattr_value(c->device_tree, "chassis-type", &type); + r = device_get_sysattr_safe_string(c->device_tree, "chassis-type", &type); if (r < 0) { log_debug_errno(r, "Failed to read device-tree chassis type, ignoring: %m"); return NULL; diff --git a/src/login/sysfs-show.c b/src/login/sysfs-show.c index 202041832c..bdbd8a64e4 100644 --- a/src/login/sysfs-show.c +++ b/src/login/sysfs-show.c @@ -4,6 +4,7 @@ #include "alloc-util.h" #include "device-enumerator-private.h" +#include "device-private.h" #include "device-util.h" #include "glyph-util.h" #include "path-util.h" @@ -61,8 +62,8 @@ static int show_sysfs_one( is_master = sd_device_has_current_tag(dev_list[*i_dev], "master-of-seat") > 0; - if (sd_device_get_sysattr_value(dev_list[*i_dev], "name", &name) < 0) - (void) sd_device_get_sysattr_value(dev_list[*i_dev], "id", &name); + if (device_get_sysattr_safe_string(dev_list[*i_dev], "name", &name) < 0) + (void) device_get_sysattr_safe_string(dev_list[*i_dev], "id", &name); /* Look if there's more coming after this */ for (lookahead = *i_dev + 1; lookahead < n_dev; lookahead++) { diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 636930d277..2e391ee4d2 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -12,6 +12,7 @@ #include "bus-util.h" #include "bus-wait-for-jobs.h" #include "chase.h" +#include "device-private.h" #include "device-util.h" #include "errno-util.h" #include "escape.h" @@ -840,7 +841,7 @@ static int find_loop_device(const char *backing_file, sd_device **ret) { FOREACH_DEVICE(e, dev) { const char *s; - r = sd_device_get_sysattr_value(dev, "loop/backing_file", &s); + r = device_get_sysattr_safe_string(dev, "loop/backing_file", &s); if (r < 0) { log_device_debug_errno(dev, r, "Failed to read \"loop/backing_file\" sysattr, ignoring: %m"); continue; diff --git a/src/network/networkd-sriov.c b/src/network/networkd-sriov.c index f5591f5672..4664425979 100644 --- a/src/network/networkd-sriov.c +++ b/src/network/networkd-sriov.c @@ -3,6 +3,7 @@ #include "sd-netlink.h" +#include "device-private.h" #include "device-util.h" #include "errno-util.h" #include "hashmap.h" @@ -282,7 +283,7 @@ int link_set_sr_iov_ifindices(Link *link) { /* This may return -EINVAL or -ENODEV, instead of -ENOENT, if the device has been removed or is being * removed. Let's ignore the error codes here. */ - r = sd_device_get_sysattr_value(link->dev, "dev_port", &dev_port); + r = device_get_sysattr_safe_string(link->dev, "dev_port", &dev_port); if (ERRNO_IS_NEG_DEVICE_ABSENT(r) || r == -EINVAL) return 0; if (r < 0) diff --git a/src/rfkill/rfkill.c b/src/rfkill/rfkill.c index f5b67e5f01..36ae1d8d23 100644 --- a/src/rfkill/rfkill.c +++ b/src/rfkill/rfkill.c @@ -10,6 +10,7 @@ #include "sd-device.h" #include "alloc-util.h" +#include "device-private.h" #include "device-util.h" #include "errno-util.h" #include "escape.h" @@ -82,7 +83,7 @@ static int find_device( return log_full_errno(ERRNO_IS_DEVICE_ABSENT(r) ? LOG_DEBUG : LOG_ERR, r, "Failed to open device '%s': %m", sysname); - r = sd_device_get_sysattr_value(device, "name", &name); + r = device_get_sysattr_safe_string(device, "name", &name); if (r < 0) return log_device_debug_errno(device, r, "Device has no name, ignoring: %m"); diff --git a/src/shared/battery-util.c b/src/shared/battery-util.c index 8ca9a6d4b2..7c3336f465 100644 --- a/src/shared/battery-util.c +++ b/src/shared/battery-util.c @@ -43,7 +43,7 @@ static int device_is_power_sink(sd_device *device) { FOREACH_DEVICE(e, d) { const char *val; - r = sd_device_get_sysattr_value(d, "power_role", &val); + r = device_get_sysattr_safe_string(d, "power_role", &val); if (r < 0) { if (r != -ENOENT) log_device_debug_errno(d, r, "Failed to read 'power_role' sysfs attribute, ignoring: %m"); @@ -93,7 +93,7 @@ static bool battery_is_discharging(sd_device *d) { } /* Possible values: "Unknown", "Charging", "Discharging", "Not charging", "Full" */ - r = sd_device_get_sysattr_value(d, "status", &val); + r = device_get_sysattr_safe_string(d, "status", &val); if (r < 0) { log_device_debug_errno(d, r, "Failed to read 'status' sysfs attribute, assuming the battery is discharging: %m"); return true; @@ -130,7 +130,7 @@ int on_ac_power(void) { * https://docs.kernel.org/admin-guide/abi-testing.html#abi-file-testing-sysfs-class-power */ const char *val; - r = sd_device_get_sysattr_value(d, "type", &val); + r = device_get_sysattr_safe_string(d, "type", &val); if (r < 0) { log_device_debug_errno(d, r, "Failed to read 'type' sysfs attribute, ignoring device: %m"); continue; diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index 3437afcda4..1debc4267f 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -15,6 +15,7 @@ #include "alloc-util.h" #include "blockdev-util.h" #include "data-fd-util.h" +#include "device-private.h" #include "device-util.h" #include "devnum-util.h" #include "dissect-image.h" @@ -1027,7 +1028,7 @@ int loop_device_open( #endif nr = info.lo_number; - if (sd_device_get_sysattr_value(dev, "loop/backing_file", &s) >= 0) { + if (device_get_sysattr_safe_string(dev, "loop/backing_file", &s) >= 0) { backing_file = strdup(s); if (!backing_file) return -ENOMEM; diff --git a/src/udev/udev-builtin-hwdb.c b/src/udev/udev-builtin-hwdb.c index dececd9c03..5cda0f9efd 100644 --- a/src/udev/udev-builtin-hwdb.c +++ b/src/udev/udev-builtin-hwdb.c @@ -6,6 +6,7 @@ #include "sd-hwdb.h" #include "alloc-util.h" +#include "device-private.h" #include "device-util.h" #include "hwdb-util.h" #include "options.h" @@ -59,7 +60,7 @@ static const char* modalias_usb(sd_device *dev, char *s, size_t size) { return NULL; if (safe_atoux16(p, &pn) < 0) return NULL; - (void) sd_device_get_sysattr_value(dev, "product", &n); + (void) device_get_sysattr_safe_string(dev, "product", &n); (void) snprintf(s, size, "usb:v%04Xp%04X:%s", vn, pn, strempty(n)); return s; diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c index ecb2afe048..da4a9b33a4 100644 --- a/src/udev/udev-builtin-input_id.c +++ b/src/udev/udev-builtin-input_id.c @@ -9,6 +9,7 @@ #include #include +#include "device-private.h" #include "device-util.h" #include "fd-util.h" #include "parse-util.h" @@ -85,7 +86,7 @@ static void get_cap_mask( unsigned long val; int r; - if (sd_device_get_sysattr_value(pdev, attr, &v) < 0) + if (device_get_sysattr_safe_string(pdev, attr, &v) < 0) v = ""; xsprintf(text, "%s", v); diff --git a/src/udev/udev-builtin-keyboard.c b/src/udev/udev-builtin-keyboard.c index 3ced8ad91c..926ee50b3e 100644 --- a/src/udev/udev-builtin-keyboard.c +++ b/src/udev/udev-builtin-keyboard.c @@ -5,6 +5,7 @@ #include #include +#include "device-private.h" #include "device-util.h" #include "errno-util.h" #include "fd-util.h" @@ -32,7 +33,7 @@ static int install_force_release(sd_device *dev, const unsigned *release, unsign if (r < 0) return log_device_error_errno(dev, r, "Failed to get serio parent: %m"); - r = sd_device_get_sysattr_value(atkbd, "force_release", &cur); + r = device_get_sysattr_safe_string(atkbd, "force_release", &cur); if (r < 0) return log_device_error_errno(atkbd, r, "Failed to get force-release attribute: %m"); diff --git a/src/udev/udev-builtin-path_id.c b/src/udev/udev-builtin-path_id.c index 6d6108ee01..af4e2b2d40 100644 --- a/src/udev/udev-builtin-path_id.c +++ b/src/udev/udev-builtin-path_id.c @@ -108,7 +108,7 @@ static sd_device* handle_scsi_fibre_channel(sd_device *parent, char **path) { return NULL; if (sd_device_new_from_subsystem_sysname(&fcdev, "fc_transport", sysname) < 0) return NULL; - if (sd_device_get_sysattr_value(fcdev, "port_name", &port) < 0) + if (device_get_sysattr_safe_string(fcdev, "port_name", &port) < 0) return NULL; format_lun_number(parent, &lun); @@ -133,7 +133,7 @@ static sd_device* handle_scsi_sas_wide_port(sd_device *parent, char **path) { return NULL; if (sd_device_new_from_subsystem_sysname(&sasdev, "sas_device", sysname) < 0) return NULL; - if (sd_device_get_sysattr_value(sasdev, "sas_address", &sas_address) < 0) + if (device_get_sysattr_safe_string(sasdev, "sas_address", &sas_address) < 0) return NULL; format_lun_number(parent, &lun); @@ -175,7 +175,7 @@ static sd_device* handle_scsi_sas(sd_device *parent, char **path) { return handle_scsi_sas_wide_port(parent, path); /* Get connected phy */ - if (sd_device_get_sysattr_value(target_sasdev, "phy_identifier", &phy_id) < 0) + if (device_get_sysattr_safe_string(target_sasdev, "phy_identifier", &phy_id) < 0) return NULL; /* The port's parent is either hba or expander */ @@ -187,7 +187,7 @@ static sd_device* handle_scsi_sas(sd_device *parent, char **path) { /* Get expander device */ if (sd_device_new_from_subsystem_sysname(&expander_sasdev, "sas_device", sysname) >= 0) { /* Get expander's address */ - if (sd_device_get_sysattr_value(expander_sasdev, "sas_address", &sas_address) < 0) + if (device_get_sysattr_safe_string(expander_sasdev, "sas_address", &sas_address) < 0) return NULL; } @@ -224,7 +224,7 @@ static sd_device* handle_scsi_iscsi(sd_device *parent, char **path) { if (sd_device_new_from_subsystem_sysname(&sessiondev, "iscsi_session", sysname) < 0) return NULL; - if (sd_device_get_sysattr_value(sessiondev, "targetname", &target) < 0) + if (device_get_sysattr_safe_string(sessiondev, "targetname", &target) < 0) return NULL; if (sd_device_get_sysnum(transportdev, &sysnum) < 0) @@ -233,9 +233,9 @@ static sd_device* handle_scsi_iscsi(sd_device *parent, char **path) { if (sd_device_new_from_subsystem_sysname(&conndev, "iscsi_connection", connname) < 0) return NULL; - if (sd_device_get_sysattr_value(conndev, "persistent_address", &addr) < 0) + if (device_get_sysattr_safe_string(conndev, "persistent_address", &addr) < 0) return NULL; - if (sd_device_get_sysattr_value(conndev, "persistent_port", &port) < 0) + if (device_get_sysattr_safe_string(conndev, "persistent_port", &port) < 0) return NULL; format_lun_number(parent, &lun); @@ -268,7 +268,7 @@ static sd_device* handle_scsi_ata(sd_device *parent, char **path, char **compat_ if (sd_device_new_from_subsystem_sysname(&atadev, "ata_port", sysname) < 0) return NULL; - if (sd_device_get_sysattr_value(atadev, "port_no", &port_no) < 0) + if (device_get_sysattr_safe_string(atadev, "port_no", &port_no) < 0) return NULL; if (bus != 0) @@ -375,7 +375,7 @@ static sd_device* handle_scsi_hyperv(sd_device *parent, char **path, size_t guid if (sd_device_get_parent(hostdev, &vmbusdev) < 0) return NULL; - if (sd_device_get_sysattr_value(vmbusdev, "device_id", &guid_str) < 0) + if (device_get_sysattr_safe_string(vmbusdev, "device_id", &guid_str) < 0) return NULL; if (strlen(guid_str) < guid_str_len || guid_str[0] != '{' || guid_str[guid_str_len-1] != '}') @@ -403,7 +403,7 @@ static sd_device* handle_scsi(sd_device *parent, char **path, char **compat_path return parent; /* firewire */ - if (sd_device_get_sysattr_value(parent, "ieee1394_id", &id) >= 0) { + if (device_get_sysattr_safe_string(parent, "ieee1394_id", &id) >= 0) { path_prepend(path, "ieee1394-0x%s", id); *supported_parent = true; return skip_subsystem(parent, "scsi"); @@ -570,8 +570,8 @@ static sd_device* handle_ap(sd_device *parent, char **path) { assert(parent); assert(path); - if (sd_device_get_sysattr_value(parent, "type", &type) >= 0 && - sd_device_get_sysattr_value(parent, "ap_functions", &func) >= 0) + if (device_get_sysattr_safe_string(parent, "type", &type) >= 0 && + device_get_sysattr_safe_string(parent, "ap_functions", &func) >= 0) path_prepend(path, "ap-%s-%s", type, func); else { const char *sysname; @@ -787,7 +787,7 @@ static int builtin_path_id(UdevEvent *event, int argc, char *argv[]) { } else if (device_in_subsystem(parent, "nvme", "nvme-subsystem") > 0) { const char *nsid; - if (sd_device_get_sysattr_value(dev, "nsid", &nsid) >= 0) { + if (device_get_sysattr_safe_string(dev, "nsid", &nsid) >= 0) { path_prepend(&path, "nvme-%s", nsid); if (compat_path) path_prepend(&compat_path, "nvme-%s", nsid); diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c index cfbea9d981..68bc55ba9e 100644 --- a/src/udev/udev-builtin-usb_id.c +++ b/src/udev/udev-builtin-usb_id.c @@ -11,6 +11,7 @@ #include #include "device-nodes.h" +#include "device-private.h" #include "device-util.h" #include "fd-util.h" #include "parse-util.h" @@ -259,8 +260,8 @@ static int builtin_usb_id(UdevEvent *event, int argc, char *argv[]) { r = sd_device_get_syspath(dev_interface, &interface_syspath); if (r < 0) return log_device_debug_errno(dev_interface, r, "Failed to get syspath: %m"); - (void) sd_device_get_sysattr_value(dev_interface, "bInterfaceNumber", &ifnum); - (void) sd_device_get_sysattr_value(dev_interface, "driver", &driver); + (void) device_get_sysattr_safe_string(dev_interface, "bInterfaceNumber", &ifnum); + (void) device_get_sysattr_safe_string(dev_interface, "driver", &driver); r = sd_device_get_sysattr_value(dev_interface, "bInterfaceClass", &if_class); if (r < 0) diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c index 73ed70f4bf..f2944b02f3 100644 --- a/src/udev/udevadm-info.c +++ b/src/udev/udevadm-info.c @@ -162,7 +162,7 @@ static int print_all_attributes(sd_device *device, bool is_parent) { if (skip_attribute(name)) continue; - r = sd_device_get_sysattr_value(device, name, &value); + r = device_get_sysattr_safe_string(device, name, &value); if (r >= 0) { /* skip any values that look like a path */ if (value[0] == '/') @@ -264,7 +264,7 @@ static int print_all_attributes_in_json(sd_device *device, bool is_parent) { if (skip_attribute(name)) continue; - r = sd_device_get_sysattr_value(device, name, &value); + r = device_get_sysattr_safe_string(device, name, &value); if (r >= 0) { /* skip any values that look like a path */ if (value[0] == '/') From 532276524f9831e669fe80b37f08954281ee85da Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 09:00:10 +0900 Subject: [PATCH 12/20] sd-device: rename argument names for storing result When a function takes only one argument for storing result, the argument is not necessary to be named as ret_xyz, but ret should be enough. --- src/libsystemd/sd-device/device-private.h | 12 +++--- src/libsystemd/sd-device/sd-device.c | 46 +++++++++++------------ src/systemd/sd-device.h | 8 ++-- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 27aa6bf2fc..9d2a8f4a8a 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -17,13 +17,13 @@ int device_get_property_int(sd_device *device, const char *key, int *ret); int device_get_property_uint(sd_device *device, const char *key, unsigned *ret); int device_get_ifname(sd_device *device, const char **ret); int device_get_sysattr_safe_string(sd_device *device, const char *sysattr, const char **ret); -int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret_value); -int device_get_sysattr_unsigned_full(sd_device *device, const char *sysattr, unsigned base, unsigned *ret_value); -static inline int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned *ret_value) { - return device_get_sysattr_unsigned_full(device, sysattr, 0, ret_value); +int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret); +int device_get_sysattr_unsigned_full(sd_device *device, const char *sysattr, unsigned base, unsigned *ret); +static inline int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned *ret) { + return device_get_sysattr_unsigned_full(device, sysattr, 0, ret); } -int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret_value); -int device_get_sysattr_u64(sd_device *device, const char *sysattr, uint64_t *ret_value); +int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret); +int device_get_sysattr_u64(sd_device *device, const char *sysattr, uint64_t *ret); int device_get_sysattr_bool(sd_device *device, const char *sysattr); int device_get_devlink_priority(sd_device *device, int *ret); int device_get_devnode_mode(sd_device *device, mode_t *ret); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 4d1693e74c..0bba1efbe2 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -888,7 +888,7 @@ int device_read_uevent_file(sd_device *device) { return 0; } -_public_ int sd_device_get_ifindex(sd_device *device, int *ifindex) { +_public_ int sd_device_get_ifindex(sd_device *device, int *ret) { int r; assert_return(device, -EINVAL); @@ -900,8 +900,8 @@ _public_ int sd_device_get_ifindex(sd_device *device, int *ifindex) { if (device->ifindex <= 0) return -ENOENT; - if (ifindex) - *ifindex = device->ifindex; + if (ret) + *ret = device->ifindex; return 0; } @@ -1348,7 +1348,7 @@ _public_ int sd_device_get_parent_with_subsystem_devtype(sd_device *device, cons } } -_public_ int sd_device_get_devnum(sd_device *device, dev_t *devnum) { +_public_ int sd_device_get_devnum(sd_device *device, dev_t *ret) { int r; assert_return(device, -EINVAL); @@ -1360,8 +1360,8 @@ _public_ int sd_device_get_devnum(sd_device *device, dev_t *devnum) { if (major(device->devnum) <= 0) return -ENOENT; - if (devnum) - *devnum = device->devnum; + if (ret) + *ret = device->devnum; return 0; } @@ -2268,7 +2268,7 @@ _public_ int sd_device_has_current_tag(sd_device *device, const char *tag) { return set_contains(device->current_tags, tag); } -_public_ int sd_device_get_property_value(sd_device *device, const char *key, const char **ret_value) { +_public_ int sd_device_get_property_value(sd_device *device, const char *key, const char **ret) { const char *value; int r; @@ -2283,8 +2283,8 @@ _public_ int sd_device_get_property_value(sd_device *device, const char *key, co if (!value) return -ENOENT; - if (ret_value) - *ret_value = value; + if (ret) + *ret = value; return 0; } @@ -2615,8 +2615,8 @@ cache_result: return device_get_cached_sysattr_value(device, sysattr, ret_value, ret_size); } -_public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, const char **ret_value) { - return sd_device_get_sysattr_value_with_size(device, sysattr, ret_value, NULL); +_public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, const char **ret) { + return sd_device_get_sysattr_value_with_size(device, sysattr, ret, NULL); } int device_get_sysattr_safe_string(sd_device *device, const char *sysattr, const char **ret) { @@ -2647,7 +2647,7 @@ int device_get_sysattr_safe_string(sd_device *device, const char *sysattr, const return 0; } -int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret_value) { +int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret) { const char *value; int r; @@ -2660,13 +2660,13 @@ int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret_valu if (r < 0) return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); - if (ret_value) - *ret_value = v; + if (ret) + *ret = v; /* We return "true" if the value is positive. */ return v > 0; } -int device_get_sysattr_unsigned_full(sd_device *device, const char *sysattr, unsigned base, unsigned *ret_value) { +int device_get_sysattr_unsigned_full(sd_device *device, const char *sysattr, unsigned base, unsigned *ret) { const char *value; int r; @@ -2679,13 +2679,13 @@ int device_get_sysattr_unsigned_full(sd_device *device, const char *sysattr, uns if (r < 0) return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); - if (ret_value) - *ret_value = v; + if (ret) + *ret = v; /* We return "true" if the value is positive. */ return v > 0; } -int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret_value) { +int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret) { const char *value; int r; @@ -2698,13 +2698,13 @@ int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret if (r < 0) return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); - if (ret_value) - *ret_value = v; + if (ret) + *ret = v; /* We return "true" if the value is positive. */ return v > 0; } -int device_get_sysattr_u64(sd_device *device, const char *sysattr, uint64_t *ret_value) { +int device_get_sysattr_u64(sd_device *device, const char *sysattr, uint64_t *ret) { const char *value; int r; @@ -2717,8 +2717,8 @@ int device_get_sysattr_u64(sd_device *device, const char *sysattr, uint64_t *ret if (r < 0) return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); - if (ret_value) - *ret_value = v; + if (ret) + *ret = v; /* We return "true" if the value is positive. */ return v > 0; } diff --git a/src/systemd/sd-device.h b/src/systemd/sd-device.h index 8eb784b1cf..87aab9f8c5 100644 --- a/src/systemd/sd-device.h +++ b/src/systemd/sd-device.h @@ -71,8 +71,8 @@ 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); +int sd_device_get_devnum(sd_device *device, dev_t *ret); +int sd_device_get_ifindex(sd_device *device, int *ret); int sd_device_get_driver(sd_device *device, const char **ret); int sd_device_get_devpath(sd_device *device, const char **ret); int sd_device_get_devname(sd_device *device, const char **ret); @@ -102,10 +102,10 @@ sd_device* sd_device_get_child_next(sd_device *device, const char **ret_suffix); int sd_device_has_tag(sd_device *device, const char *tag); int sd_device_has_current_tag(sd_device *device, const char *tag); -int sd_device_get_property_value(sd_device *device, const char *key, const char **value); +int sd_device_get_property_value(sd_device *device, const char *key, const char **ret); int sd_device_get_trigger_uuid(sd_device *device, sd_id128_t *ret); int sd_device_get_sysattr_value_with_size(sd_device *device, const char *sysattr, const char **ret_value, size_t *ret_size); -int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, const char **ret_value); +int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, const char **ret); int sd_device_set_sysattr_value(sd_device *device, const char *sysattr, const char *value); int sd_device_set_sysattr_valuef(sd_device *device, const char *sysattr, const char *format, ...) _sd_printf_(3, 4); From dbfe8b162b366d48cae5a13dab11e496843d7035 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 09:19:34 +0900 Subject: [PATCH 13/20] test: add test cases for device_get_sysattr_int() and friends --- src/libsystemd/sd-device/test-sd-device.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index 9201919898..13fb9fd25d 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -312,6 +312,24 @@ static void test_sd_device_one(sd_device *d) { ASSERT_OK(device_get_sysattr_safe_string(d, "uevent", &uevent_safe)); ASSERT_STREQ(uevent, uevent_safe); } + + if (sd_device_get_ifindex(d, &ifindex) >= 0) { + int i; + ASSERT_OK_POSITIVE(device_get_sysattr_int(d, "ifindex", &i)); + ASSERT_EQ(i, ifindex); + + unsigned u; + ASSERT_OK_POSITIVE(device_get_sysattr_unsigned(d, "ifindex", &u)); + ASSERT_EQ(u, (unsigned) ifindex); + + uint64_t u64; + ASSERT_OK_POSITIVE(device_get_sysattr_u64(d, "ifindex", &u64)); + ASSERT_EQ(u64, (uint64_t) ifindex); + + uint32_t u32; + ASSERT_OK_POSITIVE(device_get_sysattr_u32(d, "ifindex", &u32)); + ASSERT_EQ(u32, (uint32_t) ifindex); + } } static void exclude_problematic_devices(sd_device_enumerator *e) { From 3c230737f532713ec8f67456585c714c10a7efec Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 09:26:29 +0900 Subject: [PATCH 14/20] sd-device: use macro to define device_get_sysattr_int() and friends --- src/libsystemd/sd-device/device-private.h | 5 +- src/libsystemd/sd-device/sd-device.c | 115 ++++++++-------------- 2 files changed, 46 insertions(+), 74 deletions(-) diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 9d2a8f4a8a..1364dd8663 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -22,7 +22,10 @@ int device_get_sysattr_unsigned_full(sd_device *device, const char *sysattr, uns static inline int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned *ret) { return device_get_sysattr_unsigned_full(device, sysattr, 0, ret); } -int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret); +int device_get_sysattr_u32_full(sd_device *device, const char *sysattr, unsigned base, uint32_t *ret); +static inline int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret) { + return device_get_sysattr_u32_full(device, sysattr, 0, ret); +} int device_get_sysattr_u64(sd_device *device, const char *sysattr, uint64_t *ret); int device_get_sysattr_bool(sd_device *device, const char *sysattr); int device_get_devlink_priority(sd_device *device, int *ret); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 0bba1efbe2..72d28998a7 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -2647,81 +2647,50 @@ int device_get_sysattr_safe_string(sd_device *device, const char *sysattr, const return 0; } -int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret) { - const char *value; - int r; +#define DEFINE_DEVICE_GET_SYSATTR_PARSE(name, type, parser) \ + int device_get_sysattr_##name(sd_device *device, const char *sysattr, type *ret) { \ + const char *value; \ + int r; \ + \ + r = sd_device_get_sysattr_value(device, sysattr, &value); \ + if (r < 0) \ + return r; \ + \ + type v; \ + r = parser(value, &v); \ + if (r < 0) \ + return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); \ + \ + if (ret) \ + *ret = v; \ + /* We return "true" if the value is positive. */ \ + return v > 0; \ + } - r = sd_device_get_sysattr_value(device, sysattr, &value); - if (r < 0) - return r; +#define DEFINE_DEVICE_GET_SYSATTR_PARSE_BASE(name, type, parser) \ + int device_get_sysattr_##name##_full(sd_device *device, const char *sysattr, unsigned base, type *ret) { \ + const char *value; \ + int r; \ + \ + r = sd_device_get_sysattr_value(device, sysattr, &value); \ + if (r < 0) \ + return r; \ + \ + type v; \ + r = parser(value, base, &v); \ + if (r < 0) \ + return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); \ + \ + if (ret) \ + *ret = v; \ + /* We return "true" if the value is positive. */ \ + return v > 0; \ + } - int v; - r = safe_atoi(value, &v); - if (r < 0) - return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); - - if (ret) - *ret = v; - /* We return "true" if the value is positive. */ - return v > 0; -} - -int device_get_sysattr_unsigned_full(sd_device *device, const char *sysattr, unsigned base, unsigned *ret) { - const char *value; - int r; - - r = sd_device_get_sysattr_value(device, sysattr, &value); - if (r < 0) - return r; - - unsigned v; - r = safe_atou_full(value, base, &v); - if (r < 0) - return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); - - if (ret) - *ret = v; - /* We return "true" if the value is positive. */ - return v > 0; -} - -int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret) { - const char *value; - int r; - - r = sd_device_get_sysattr_value(device, sysattr, &value); - if (r < 0) - return r; - - uint32_t v; - r = safe_atou32(value, &v); - if (r < 0) - return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); - - if (ret) - *ret = v; - /* We return "true" if the value is positive. */ - return v > 0; -} - -int device_get_sysattr_u64(sd_device *device, const char *sysattr, uint64_t *ret) { - const char *value; - int r; - - r = sd_device_get_sysattr_value(device, sysattr, &value); - if (r < 0) - return r; - - uint64_t v; - r = safe_atou64(value, &v); - if (r < 0) - return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr); - - if (ret) - *ret = v; - /* We return "true" if the value is positive. */ - return v > 0; -} +DEFINE_DEVICE_GET_SYSATTR_PARSE(int, int, safe_atoi); +DEFINE_DEVICE_GET_SYSATTR_PARSE_BASE(unsigned, unsigned, safe_atou_full); +DEFINE_DEVICE_GET_SYSATTR_PARSE_BASE(u32, uint32_t, safe_atou32_full); +DEFINE_DEVICE_GET_SYSATTR_PARSE(u64, uint64_t, safe_atou64); int device_get_sysattr_bool(sd_device *device, const char *sysattr) { const char *value; From 1e39d3eb08a2f79c7bb729a3c0645f0215546b5b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 09:48:22 +0900 Subject: [PATCH 15/20] sd-device: also add device_get_sysattr_u8() and _u16() --- src/libsystemd/sd-device/device-private.h | 12 ++++++++++-- src/libsystemd/sd-device/sd-device.c | 2 ++ src/libsystemd/sd-device/test-sd-device.c | 12 ++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 1364dd8663..33a2c7ebb6 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -20,11 +20,19 @@ int device_get_sysattr_safe_string(sd_device *device, const char *sysattr, const int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret); int device_get_sysattr_unsigned_full(sd_device *device, const char *sysattr, unsigned base, unsigned *ret); static inline int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned *ret) { - return device_get_sysattr_unsigned_full(device, sysattr, 0, ret); + return device_get_sysattr_unsigned_full(device, sysattr, /* base= */ 0, ret); +} +int device_get_sysattr_u8_full(sd_device *device, const char *sysattr, unsigned base, uint8_t *ret); +static inline int device_get_sysattr_u8(sd_device *device, const char *sysattr, uint8_t *ret) { + return device_get_sysattr_u8_full(device, sysattr, /* base= */ 0, ret); +} +int device_get_sysattr_u16_full(sd_device *device, const char *sysattr, unsigned base, uint16_t *ret); +static inline int device_get_sysattr_u16(sd_device *device, const char *sysattr, uint16_t *ret) { + return device_get_sysattr_u16_full(device, sysattr, /* base= */ 0, ret); } int device_get_sysattr_u32_full(sd_device *device, const char *sysattr, unsigned base, uint32_t *ret); static inline int device_get_sysattr_u32(sd_device *device, const char *sysattr, uint32_t *ret) { - return device_get_sysattr_u32_full(device, sysattr, 0, ret); + return device_get_sysattr_u32_full(device, sysattr, /* base= */ 0, ret); } int device_get_sysattr_u64(sd_device *device, const char *sysattr, uint64_t *ret); int device_get_sysattr_bool(sd_device *device, const char *sysattr); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 72d28998a7..f157125c50 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -2689,6 +2689,8 @@ int device_get_sysattr_safe_string(sd_device *device, const char *sysattr, const DEFINE_DEVICE_GET_SYSATTR_PARSE(int, int, safe_atoi); DEFINE_DEVICE_GET_SYSATTR_PARSE_BASE(unsigned, unsigned, safe_atou_full); +DEFINE_DEVICE_GET_SYSATTR_PARSE_BASE(u8, uint8_t, safe_atou8_full); +DEFINE_DEVICE_GET_SYSATTR_PARSE_BASE(u16, uint16_t, safe_atou16_full); DEFINE_DEVICE_GET_SYSATTR_PARSE_BASE(u32, uint32_t, safe_atou32_full); DEFINE_DEVICE_GET_SYSATTR_PARSE(u64, uint64_t, safe_atou64); diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index 13fb9fd25d..efeae7a715 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -329,6 +329,18 @@ static void test_sd_device_one(sd_device *d) { uint32_t u32; ASSERT_OK_POSITIVE(device_get_sysattr_u32(d, "ifindex", &u32)); ASSERT_EQ(u32, (uint32_t) ifindex); + + if (ifindex <= UINT16_MAX) { + uint16_t u16; + ASSERT_OK_POSITIVE(device_get_sysattr_u16(d, "ifindex", &u16)); + ASSERT_EQ(u16, (uint16_t) ifindex); + } + + if (ifindex <= UINT8_MAX) { + uint8_t u8; + ASSERT_OK_POSITIVE(device_get_sysattr_u8(d, "ifindex", &u8)); + ASSERT_EQ(u8, (uint8_t) ifindex); + } } } From 247e4caa3cf5339be9cb372f673725517268cfcd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 09:53:13 +0900 Subject: [PATCH 16/20] sd-device: also add device_get_sysattr_streq() --- src/libsystemd/sd-device/device-private.h | 1 + src/libsystemd/sd-device/sd-device.c | 13 +++++++++++++ src/libsystemd/sd-device/test-sd-device.c | 5 +++++ 3 files changed, 19 insertions(+) diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 33a2c7ebb6..0773ec9098 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -16,6 +16,7 @@ int device_get_property_bool(sd_device *device, const char *key); int device_get_property_int(sd_device *device, const char *key, int *ret); int device_get_property_uint(sd_device *device, const char *key, unsigned *ret); int device_get_ifname(sd_device *device, const char **ret); +int device_get_sysattr_streq(sd_device *device, const char *sysattr, const char *expected); int device_get_sysattr_safe_string(sd_device *device, const char *sysattr, const char **ret); int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret); int device_get_sysattr_unsigned_full(sd_device *device, const char *sysattr, unsigned base, unsigned *ret); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index f157125c50..07c3dd60d2 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -2619,6 +2619,19 @@ _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, return sd_device_get_sysattr_value_with_size(device, sysattr, ret, NULL); } +int device_get_sysattr_streq(sd_device *device, const char *sysattr, const char *expected) { + const char *value; + int r; + + assert(expected); + + r = sd_device_get_sysattr_value(device, sysattr, &value); + if (r < 0) + return r; + + return streq(value, expected); +} + int device_get_sysattr_safe_string(sd_device *device, const char *sysattr, const char **ret) { const char *value; int r; diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index efeae7a715..c53f0b0e88 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -341,6 +341,11 @@ static void test_sd_device_one(sd_device *d) { ASSERT_OK_POSITIVE(device_get_sysattr_u8(d, "ifindex", &u8)); ASSERT_EQ(u8, (uint8_t) ifindex); } + + const char *s; + ASSERT_OK(sd_device_get_sysattr_value(d, "ifindex", &s)); + ASSERT_OK_POSITIVE(device_get_sysattr_streq(d, "ifindex", s)); + ASSERT_OK_ZERO(device_get_sysattr_streq(d, "ifindex", "hoge")); } } From 23c93e2cd648db1c55901e7ee4c969ddcec5fa7f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 07:06:00 +0900 Subject: [PATCH 17/20] udev-builtin-usb_id: use device_get_sysattr_unsigned() and friends This also fix the parser of bInterfaceClass from safe_atoi() to safe_atou_full() as it is unsigned hex. --- src/udev/udev-builtin-usb_id.c | 110 ++++++++++++++++----------------- 1 file changed, 53 insertions(+), 57 deletions(-) diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c index 68bc55ba9e..66a0f0ee10 100644 --- a/src/udev/udev-builtin-usb_id.c +++ b/src/udev/udev-builtin-usb_id.c @@ -14,7 +14,6 @@ #include "device-private.h" #include "device-util.h" #include "fd-util.h" -#include "parse-util.h" #include "string-util.h" #include "strxcpyx.h" #include "udev-builtin.h" @@ -71,59 +70,58 @@ static void set_usb_iftype(char *to, int if_class_num, size_t len) { to[len-1] = '\0'; } -static int set_usb_mass_storage_ifsubtype(char *to, const char *from, size_t len) { - int type_num = 0; - const char *type = "generic"; +static void set_usb_mass_storage_ifsubtype(char *to, uint8_t type_num, size_t len) { + const char *type; assert(to); - if (safe_atoi(from, &type_num) >= 0) - switch (type_num) { - case 1: /* RBC devices */ - type = "rbc"; - break; - case 2: - type = "atapi"; - break; - case 3: - type = "tape"; - break; - case 4: /* UFI */ - type = "floppy"; - break; - case 6: /* Transparent SPC-2 devices */ - type = "scsi"; - break; - } + switch (type_num) { + case 1: /* RBC devices */ + type = "rbc"; + break; + case 2: + type = "atapi"; + break; + case 3: + type = "tape"; + break; + case 4: /* UFI */ + type = "floppy"; + break; + case 6: /* Transparent SPC-2 devices */ + type = "scsi"; + break; + default: + type = "generic"; + } strscpy(to, len, type); - return type_num; } -static void set_scsi_type(char *to, const char *from, size_t len) { - unsigned type_num; - const char *type = "generic"; +static void set_scsi_type(char *to, unsigned type_num, size_t len) { + const char *type; assert(to); - if (safe_atou(from, &type_num) >= 0) - switch (type_num) { - case 0: - case 0xe: - type = "disk"; - break; - case 1: - type = "tape"; - break; - case 4: - case 7: - case 0xf: - type = "optical"; - break; - case 5: - type = "cd"; - break; - } + switch (type_num) { + case 0: + case 0xe: + type = "disk"; + break; + case 1: + type = "tape"; + break; + case 4: + case 7: + case 0xf: + type = "optical"; + break; + case 5: + type = "cd"; + break; + default: + type = "generic"; + } strscpy(to, len, type); } @@ -229,12 +227,12 @@ static int dev_if_packed_info(sd_device *dev, char *ifs_str, size_t len) { static int builtin_usb_id(UdevEvent *event, int argc, char *argv[]) { sd_device *dev_interface, *dev_usb, *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); const char *syspath, *sysname, *interface_syspath, *vendor_id, *product_id, - *ifnum = NULL, *driver = NULL, *if_class, *if_subclass; + *ifnum = NULL, *driver = NULL; char *s, model_str[64] = "", model_str_enc[256], serial_str[UDEV_NAME_SIZE] = "", packed_if_str[UDEV_NAME_SIZE] = "", revision_str[64] = "", type_str[64] = "", instance_str[64] = "", serial[256], vendor_str[64] = "", vendor_str_enc[256]; - unsigned if_class_num; - int r, protocol = 0; + unsigned if_class_num, protocol = 0; + int r; size_t l; r = sd_device_get_syspath(dev, &syspath); @@ -263,21 +261,18 @@ static int builtin_usb_id(UdevEvent *event, int argc, char *argv[]) { (void) device_get_sysattr_safe_string(dev_interface, "bInterfaceNumber", &ifnum); (void) device_get_sysattr_safe_string(dev_interface, "driver", &driver); - r = sd_device_get_sysattr_value(dev_interface, "bInterfaceClass", &if_class); + r = device_get_sysattr_unsigned_full(dev_interface, "bInterfaceClass", 16, &if_class_num); if (r < 0) - return log_device_debug_errno(dev_interface, r, "Failed to get bInterfaceClass attribute: %m"); + return log_device_debug_errno(dev_interface, r, "Failed to read bInterfaceClass attribute: %m"); - r = safe_atou_full(if_class, 16, &if_class_num); - if (r < 0) - return log_device_debug_errno(dev_interface, r, "Failed to parse if_class: %m"); if (if_class_num == 8) { /* mass storage */ - if (sd_device_get_sysattr_value(dev_interface, "bInterfaceSubClass", &if_subclass) >= 0) - protocol = set_usb_mass_storage_ifsubtype(type_str, if_subclass, sizeof(type_str)-1); + if (device_get_sysattr_unsigned_full(dev_interface, "bInterfaceSubClass", 16, &protocol) >= 0) + set_usb_mass_storage_ifsubtype(type_str, protocol, sizeof(type_str)-1); } else set_usb_iftype(type_str, if_class_num, sizeof(type_str)-1); - log_device_debug(dev_interface, "if_class:%u protocol:%i", if_class_num, protocol); + log_device_debug(dev_interface, "if_class:%u protocol:%u", if_class_num, protocol); /* usb device directory */ r = sd_device_get_parent_with_subsystem_devtype(dev_interface, "usb", "usb_device", &dev_usb); @@ -290,7 +285,7 @@ static int builtin_usb_id(UdevEvent *event, int argc, char *argv[]) { /* mass storage : SCSI or ATAPI */ if (IN_SET(protocol, 6, 2)) { sd_device *dev_scsi; - const char *scsi_sysname, *scsi_model, *scsi_vendor, *scsi_type, *scsi_rev; + const char *scsi_sysname, *scsi_model, *scsi_vendor, *scsi_rev; int host, bus, target, lun; /* get scsi device */ @@ -325,7 +320,8 @@ static int builtin_usb_id(UdevEvent *event, int argc, char *argv[]) { udev_replace_whitespace(scsi_model, model_str, sizeof(model_str)-1); udev_replace_chars(model_str, NULL); - r = sd_device_get_sysattr_value(dev_scsi, "type", &scsi_type); + unsigned scsi_type; + r = device_get_sysattr_unsigned(dev_scsi, "type", &scsi_type); if (r < 0) { log_device_debug_errno(dev_scsi, r, "Failed to get SCSI type attribute: %m"); goto fallback; From 6ad682d70f5a741f54454e44c61e74d268e74e14 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 18 May 2026 01:10:17 +0900 Subject: [PATCH 18/20] udev-builtin-usb_id: make set_usb_iftype() and friends return const char* --- src/udev/udev-builtin-usb_id.c | 153 ++++++++++++--------------------- 1 file changed, 56 insertions(+), 97 deletions(-) diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c index 66a0f0ee10..be9675fe88 100644 --- a/src/udev/udev-builtin-usb_id.c +++ b/src/udev/udev-builtin-usb_id.c @@ -19,111 +19,70 @@ #include "udev-builtin.h" #include "udev-util.h" -static void set_usb_iftype(char *to, int if_class_num, size_t len) { - const char *type = "generic"; - - assert(to); - assert(len > 0); - +static const char* set_usb_iftype(unsigned if_class_num) { switch (if_class_num) { - case 1: - type = "audio"; - break; - case 2: /* CDC-Control */ - break; - case 3: - type = "hid"; - break; - case 5: /* Physical */ - break; - case 6: - type = "media"; - break; - case 7: - type = "printer"; - break; - case 8: - type = "storage"; - break; - case 9: - type = "hub"; - break; - case 0x0a: /* CDC-Data */ - break; - case 0x0b: /* Chip/Smart Card */ - break; - case 0x0d: /* Content Security */ - break; + case 0x01: + return "audio"; + case 0x03: + return "hid"; + case 0x06: + return "media"; + case 0x07: + return "printer"; + case 0x08: + return "storage"; + case 0x09: + return "hub"; case 0x0e: - type = "video"; - break; - case 0xdc: /* Diagnostic Device */ - break; - case 0xe0: /* Wireless Controller */ - break; - case 0xfe: /* Application-specific */ - break; - case 0xff: /* Vendor-specific */ - break; + return "video"; + default: + /* Other known types: + * 0x02: CDC-Control + * 0x05: Physical + * 0x0a: CDC-Data + * 0x0b: Chip/Smart Card + * 0x0d: Content Security + * 0xdc: Diagnostic Device + * 0xe0: Wireless Controller + * 0xfe: Application-specific + * 0xff: Vendor-specific */ + return "generic"; } - strncpy(to, type, len); - to[len-1] = '\0'; } -static void set_usb_mass_storage_ifsubtype(char *to, uint8_t type_num, size_t len) { - const char *type; - - assert(to); - +static const char* set_usb_mass_storage_ifsubtype(uint8_t type_num) { switch (type_num) { - case 1: /* RBC devices */ - type = "rbc"; - break; - case 2: - type = "atapi"; - break; - case 3: - type = "tape"; - break; - case 4: /* UFI */ - type = "floppy"; - break; - case 6: /* Transparent SPC-2 devices */ - type = "scsi"; - break; + case 0x01: /* RBC devices */ + return "rbc"; + case 0x02: + return "atapi"; + case 0x03: + return "tape"; + case 0x04: /* UFI */ + return "floppy"; + case 0x06: /* Transparent SPC-2 devices */ + return "scsi"; default: - type = "generic"; + return "generic"; } - - strscpy(to, len, type); } -static void set_scsi_type(char *to, unsigned type_num, size_t len) { - const char *type; - - assert(to); - +static const char* set_scsi_type(unsigned type_num) { switch (type_num) { - case 0: - case 0xe: - type = "disk"; - break; - case 1: - type = "tape"; - break; - case 4: - case 7: - case 0xf: - type = "optical"; - break; - case 5: - type = "cd"; - break; + case 0x00: + case 0x0e: + return "disk"; + case 0x01: + return "tape"; + case 0x04: + case 0x07: + case 0x0f: + return "optical"; + case 0x05: + return "cd"; default: - type = "generic"; + return "generic"; } - - strscpy(to, len, type); } #define USB_DT_DEVICE 0x01 @@ -227,9 +186,9 @@ static int dev_if_packed_info(sd_device *dev, char *ifs_str, size_t len) { static int builtin_usb_id(UdevEvent *event, int argc, char *argv[]) { sd_device *dev_interface, *dev_usb, *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); const char *syspath, *sysname, *interface_syspath, *vendor_id, *product_id, - *ifnum = NULL, *driver = NULL; + *ifnum = NULL, *driver = NULL, *type_str = NULL; char *s, model_str[64] = "", model_str_enc[256], serial_str[UDEV_NAME_SIZE] = "", - packed_if_str[UDEV_NAME_SIZE] = "", revision_str[64] = "", type_str[64] = "", + packed_if_str[UDEV_NAME_SIZE] = "", revision_str[64] = "", instance_str[64] = "", serial[256], vendor_str[64] = "", vendor_str_enc[256]; unsigned if_class_num, protocol = 0; int r; @@ -268,9 +227,9 @@ static int builtin_usb_id(UdevEvent *event, int argc, char *argv[]) { if (if_class_num == 8) { /* mass storage */ if (device_get_sysattr_unsigned_full(dev_interface, "bInterfaceSubClass", 16, &protocol) >= 0) - set_usb_mass_storage_ifsubtype(type_str, protocol, sizeof(type_str)-1); + type_str = set_usb_mass_storage_ifsubtype(protocol); } else - set_usb_iftype(type_str, if_class_num, sizeof(type_str)-1); + type_str = set_usb_iftype(if_class_num); log_device_debug(dev_interface, "if_class:%u protocol:%u", if_class_num, protocol); @@ -326,7 +285,7 @@ static int builtin_usb_id(UdevEvent *event, int argc, char *argv[]) { log_device_debug_errno(dev_scsi, r, "Failed to get SCSI type attribute: %m"); goto fallback; } - set_scsi_type(type_str, scsi_type, sizeof(type_str)-1); + type_str = set_scsi_type(scsi_type); r = sd_device_get_sysattr_value(dev_scsi, "rev", &scsi_rev); if (r < 0) { From 6f72bb7bc9a264e269adb1ff3228467adccd0e94 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 07:07:12 +0900 Subject: [PATCH 19/20] tree-wide: use device_get_sysattr_unsigned() and friends --- src/backlight/backlight.c | 23 +++++++---------------- src/login/logind-core.c | 13 +++++-------- src/mount/mount-tool.c | 11 ++++++----- src/network/networkd-link.c | 12 ++++-------- src/shared/battery-util.c | 9 ++++----- src/shared/netif-sriov.c | 29 ++++------------------------- src/sleep/battery-capacity.c | 12 +++--------- src/udev/udev-builtin-hwdb.c | 15 +++++---------- src/udev/udev-builtin-input_id.c | 15 ++++++--------- src/udev/udev-builtin-path_id.c | 12 +++--------- 10 files changed, 47 insertions(+), 104 deletions(-) diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index 29ce29fab9..a84640d45d 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -66,13 +66,9 @@ static int has_multiple_graphics_cards(void) { return r; FOREACH_DEVICE(e, dev) { - const char *s; - unsigned long c; + uint32_t c; - if (sd_device_get_sysattr_value(dev, "class", &s) < 0) - continue; - - if (safe_atolu(s, &c) < 0) + if (device_get_sysattr_u32(dev, "class", &c) < 0) continue; if (c != PCI_CLASS_GRAPHICS_CARD) @@ -180,7 +176,7 @@ static int same_device(sd_device *a, sd_device *b) { static int validate_device(sd_device *device) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *enumerate = NULL; - const char *v, *sysname; + const char *sysname; sd_device *parent; int r; @@ -205,10 +201,10 @@ static int validate_device(sd_device *device) { if (r > 0) return true; /* We assume LED device is always valid. */ - r = sd_device_get_sysattr_value(device, "type", &v); + r = device_get_sysattr_streq(device, "type", "raw"); if (r < 0) return log_device_debug_errno(device, r, "Failed to read 'type' sysattr: %m"); - if (!streq(v, "raw")) + if (r == 0) return true; r = find_pci_or_platform_parent(device, &parent); @@ -316,7 +312,7 @@ static int read_max_brightness(sd_device *device, unsigned *ret) { r = device_get_sysattr_unsigned(device, "max_brightness", &max_brightness); if (r < 0) - return log_device_warning_errno(device, r, "Failed to read/parse 'max_brightness' attribute: %m"); + return log_device_warning_errno(device, r, "Failed to read 'max_brightness' attribute: %m"); /* If max_brightness is 0, then there is no actual backlight device. This happens on desktops * with Asus mainboards that load the eeepc-wmi module. */ @@ -418,21 +414,16 @@ static int shall_clamp(sd_device *device, unsigned *ret) { } static int read_brightness(sd_device *device, unsigned max_brightness, unsigned *ret_brightness) { - const char *value; unsigned brightness; int r; assert(device); assert(ret_brightness); - r = sd_device_get_sysattr_value(device, "brightness", &value); + r = device_get_sysattr_unsigned(device, "brightness", &brightness); if (r < 0) return log_device_debug_errno(device, r, "Failed to read 'brightness' attribute: %m"); - r = safe_atou(value, &brightness); - if (r < 0) - return log_device_debug_errno(device, r, "Failed to parse 'brightness' attribute: %s", value); - if (brightness > max_brightness) return log_device_debug_errno(device, SYNTHETIC_ERRNO(EINVAL), "brightness=%u is larger than max_brightness=%u", diff --git a/src/login/logind-core.c b/src/login/logind-core.c index 33f895f004..c8846d592b 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -13,6 +13,7 @@ #include "bus-locator.h" #include "cgroup-util.h" #include "conf-parser.h" +#include "device-private.h" #include "device-util.h" #include "efi-loader.h" #include "errno-util.h" @@ -676,21 +677,17 @@ static int manager_count_external_displays(Manager *m) { continue; /* Ignore ports that are not enabled */ - const char *enabled; - r = sd_device_get_sysattr_value(d, "enabled", &enabled); - if (r == -ENOENT) + r = device_get_sysattr_streq(d, "enabled", "enabled"); + if (IN_SET(r, 0, -ENOENT)) continue; if (r < 0) return r; - if (!streq(enabled, "enabled")) - continue; /* We count any connector which is not explicitly "disconnected" as connected. */ - const char *status = NULL; - r = sd_device_get_sysattr_value(d, "status", &status); + r = device_get_sysattr_streq(d, "status", "disconnected"); if (r < 0 && r != -ENOENT) return r; - if (!streq_ptr(status, "disconnected")) + if (r <= 0) n++; } diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 2e391ee4d2..0869fe27e1 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -1228,7 +1228,6 @@ static int acquire_description(sd_device *d) { } static int acquire_removable(sd_device *d) { - const char *v; int r; assert(d); @@ -1238,8 +1237,13 @@ static int acquire_removable(sd_device *d) { return 0; for (;;) { - if (sd_device_get_sysattr_value(d, "removable", &v) >= 0) + r = device_get_sysattr_bool(d, "removable"); + if (r == 0) + return 0; /* not a removable device */ + if (r > 0) break; + if (r != -ENOENT) + return log_device_debug_errno(d, r, "Failed to read 'removable' sysattr: %m"); r = sd_device_get_parent(d, &d); if (r == -ENODEV) @@ -1252,9 +1256,6 @@ static int acquire_removable(sd_device *d) { return r; } - if (parse_boolean(v) <= 0) - return 0; - log_debug("Discovered removable device."); if (arg_action == ACTION_DEFAULT) { diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 3327553836..a69a5e8979 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -23,6 +23,7 @@ #include "alloc-util.h" #include "arphrd-util.h" #include "bitfield.h" +#include "device-private.h" #include "device-util.h" #include "dns-domain.h" #include "errno-util.h" @@ -66,7 +67,6 @@ #include "networkd-wifi.h" #include "networkd-wwan-bus.h" #include "ordered-set.h" -#include "parse-util.h" #include "set.h" #include "socket-util.h" #include "string-table.h" @@ -1334,13 +1334,9 @@ static int link_get_network(Link *link, Network **ret) { continue; if (network->match.ifname && link->dev) { - uint8_t name_assign_type = NET_NAME_UNKNOWN; - const char *attr; - - if (sd_device_get_sysattr_value(link->dev, "name_assign_type", &attr) >= 0) - (void) safe_atou8(attr, &name_assign_type); - - warn = name_assign_type == NET_NAME_ENUM; + uint8_t name_assign_type; + if (device_get_sysattr_u8(link->dev, "name_assign_type", &name_assign_type) >= 0) + warn = name_assign_type == NET_NAME_ENUM; } log_link_full(link, warn ? LOG_WARNING : LOG_DEBUG, diff --git a/src/shared/battery-util.c b/src/shared/battery-util.c index 7c3336f465..7e18c2ee87 100644 --- a/src/shared/battery-util.c +++ b/src/shared/battery-util.c @@ -75,11 +75,10 @@ static bool battery_is_discharging(sd_device *d) { assert(d); - r = sd_device_get_sysattr_value(d, "scope", &val); - if (r < 0) { - if (r != -ENOENT) - log_device_debug_errno(d, r, "Failed to read 'scope' sysfs attribute, ignoring: %m"); - } else if (streq(val, "Device")) { + r = device_get_sysattr_streq(d, "scope", "Device"); + if (r < 0 && r != -ENOENT) + log_device_debug_errno(d, r, "Failed to read 'scope' sysfs attribute, ignoring: %m"); + if (r > 0) { log_device_debug(d, "The power supply is a device battery, ignoring device."); return false; } diff --git a/src/shared/netif-sriov.c b/src/shared/netif-sriov.c index f621a9dde7..f5df7fb392 100644 --- a/src/shared/netif-sriov.c +++ b/src/shared/netif-sriov.c @@ -5,6 +5,7 @@ #include "alloc-util.h" #include "conf-parser.h" +#include "device-private.h" #include "device-util.h" #include "ether-addr-util.h" #include "netif-sriov.h" @@ -267,28 +268,11 @@ int sr_iov_set_netlink_message(SRIOV *sr_iov, SRIOVAttribute attr, sd_netlink_me } int sr_iov_get_num_vfs(sd_device *device, uint32_t *ret) { - const char *str; - uint32_t n; - int r; - - assert(device); - assert(ret); - - r = sd_device_get_sysattr_value(device, "device/sriov_numvfs", &str); - if (r < 0) - return r; - - r = safe_atou32(str, &n); - if (r < 0) - return r; - - *ret = n; - return 0; + return device_get_sysattr_u32(device, "device/sriov_numvfs", ret); } int sr_iov_set_num_vfs(sd_device *device, uint32_t num_vfs, OrderedHashmap *sr_iov_by_section) { char val[DECIMAL_STR_MAX(uint32_t)]; - const char *str; int r; assert(device); @@ -327,14 +311,9 @@ int sr_iov_set_num_vfs(sd_device *device, uint32_t num_vfs, OrderedHashmap *sr_i * maximum allowed number of VFs from the sriov_totalvfs sysattr. Note that the sysattr * currently exists only for PCI drivers. Hence, ignore -ENOENT. * TODO: netdevsim provides the information in debugfs. */ - r = sd_device_get_sysattr_value(device, "device/sriov_totalvfs", &str); + uint32_t max_num_vfs; + r = device_get_sysattr_u32(device, "device/sriov_totalvfs", &max_num_vfs); if (r >= 0) { - uint32_t max_num_vfs; - - r = safe_atou32(str, &max_num_vfs); - if (r < 0) - return log_device_debug_errno(device, r, "Failed to parse device/sriov_totalvfs sysfs attribute '%s': %m", str); - if (num_vfs > max_num_vfs) return log_device_debug_errno(device, SYNTHETIC_ERRNO(ERANGE), "Specified number of virtual functions is out of range. " diff --git a/src/sleep/battery-capacity.c b/src/sleep/battery-capacity.c index f6a1ed25ac..168fff91c0 100644 --- a/src/sleep/battery-capacity.c +++ b/src/sleep/battery-capacity.c @@ -5,6 +5,7 @@ #include "alloc-util.h" #include "battery-capacity.h" #include "battery-util.h" +#include "device-private.h" #include "device-util.h" #include "extract-word.h" #include "fd-util.h" @@ -368,20 +369,13 @@ int battery_trip_point_alarm_exists(void) { return log_debug_errno(r, "Failed to initialize battery enumerator: %m"); FOREACH_DEVICE(e, dev) { - const char *alarm_attr; - int has_alarm; - has_battery = true; - r = sd_device_get_sysattr_value(dev, "alarm", &alarm_attr); + int has_alarm; + r = device_get_sysattr_int(dev, "alarm", &has_alarm); if (r < 0) return log_device_debug_errno(dev, r, "Failed to read battery alarm attribute: %m"); - r = safe_atoi(alarm_attr, &has_alarm); - if (r < 0) - return log_device_debug_errno(dev, r, - "Failed to parse battery alarm attribute '%s': %m", - alarm_attr); if (has_alarm <= 0) return false; } diff --git a/src/udev/udev-builtin-hwdb.c b/src/udev/udev-builtin-hwdb.c index 5cda0f9efd..351c861037 100644 --- a/src/udev/udev-builtin-hwdb.c +++ b/src/udev/udev-builtin-hwdb.c @@ -10,7 +10,6 @@ #include "device-util.h" #include "hwdb-util.h" #include "options.h" -#include "parse-util.h" #include "string-util.h" #include "udev-builtin.h" @@ -49,20 +48,16 @@ int udev_builtin_hwdb_lookup( } static const char* modalias_usb(sd_device *dev, char *s, size_t size) { - const char *v, *p, *n = NULL; - uint16_t vn, pn; + const char *n = NULL; + uint16_t v, p; - if (sd_device_get_sysattr_value(dev, "idVendor", &v) < 0) + if (device_get_sysattr_u16_full(dev, "idVendor", 16, &v) < 0) return NULL; - if (sd_device_get_sysattr_value(dev, "idProduct", &p) < 0) - return NULL; - if (safe_atoux16(v, &vn) < 0) - return NULL; - if (safe_atoux16(p, &pn) < 0) + if (device_get_sysattr_u16_full(dev, "idProduct", 16, &p) < 0) return NULL; (void) device_get_sysattr_safe_string(dev, "product", &n); - (void) snprintf(s, size, "usb:v%04Xp%04X:%s", vn, pn, strempty(n)); + (void) snprintf(s, size, "usb:v%04Xp%04X:%s", v, p, strempty(n)); return s; } diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c index da4a9b33a4..8a31760889 100644 --- a/src/udev/udev-builtin-input_id.c +++ b/src/udev/udev-builtin-input_id.c @@ -131,17 +131,14 @@ static void get_cap_mask( } static struct input_id get_input_id(sd_device *dev) { - const char *v; struct input_id id = {}; - if (sd_device_get_sysattr_value(dev, "id/bustype", &v) >= 0) - (void) safe_atoux16(v, &id.bustype); - if (sd_device_get_sysattr_value(dev, "id/vendor", &v) >= 0) - (void) safe_atoux16(v, &id.vendor); - if (sd_device_get_sysattr_value(dev, "id/product", &v) >= 0) - (void) safe_atoux16(v, &id.product); - if (sd_device_get_sysattr_value(dev, "id/version", &v) >= 0) - (void) safe_atoux16(v, &id.version); + assert(dev); + + (void) device_get_sysattr_u16_full(dev, "id/bustype", 16, &id.bustype); + (void) device_get_sysattr_u16_full(dev, "id/vendor", 16, &id.vendor); + (void) device_get_sysattr_u16_full(dev, "id/product", 16, &id.product); + (void) device_get_sysattr_u16_full(dev, "id/version", 16, &id.version); return id; } diff --git a/src/udev/udev-builtin-path_id.c b/src/udev/udev-builtin-path_id.c index af4e2b2d40..f5d33cd246 100644 --- a/src/udev/udev-builtin-path_id.c +++ b/src/udev/udev-builtin-path_id.c @@ -472,18 +472,13 @@ static void handle_scsi_tape(sd_device *dev, char **path) { static int get_usb_revision(sd_device *dev) { uint8_t protocol; - const char *s; int r; assert(dev); /* Returns usb revision 1, 2, or 3. */ - r = sd_device_get_sysattr_value(dev, "bDeviceProtocol", &s); - if (r < 0) - return r; - - r = safe_atou8_full(s, 16, &protocol); + r = device_get_sysattr_u8_full(dev, "bDeviceProtocol", 16, &protocol); if (r < 0) return r; @@ -491,11 +486,10 @@ static int get_usb_revision(sd_device *dev) { case USB_HUB_PR_HS_NO_TT: /* Full speed hub (USB1) or Hi-speed hub without TT (USB2) */ /* See speed_show() in drivers/usb/core/sysfs.c of the kernel. */ - r = sd_device_get_sysattr_value(dev, "speed", &s); + r = device_get_sysattr_streq(dev, "speed", "480"); if (r < 0) return r; - - if (streq(s, "480")) + if (r > 0) return 2; return 1; From 2a1a026d72a2fff4f155ced6ab28977f74641355 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Mar 2026 14:37:37 +0900 Subject: [PATCH 20/20] udev-builtin-input-id: drop unused value --- src/udev/udev-builtin-input_id.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c index 8a31760889..ec232f3e93 100644 --- a/src/udev/udev-builtin-input_id.c +++ b/src/udev/udev-builtin-input_id.c @@ -384,9 +384,7 @@ static int builtin_input_id(UdevEvent *event, int argc, char *argv[]) { /* walk up the parental chain until we find the real input device; the * argument is very likely a subdevice of this, like eventN */ for (pdev = dev; pdev; ) { - const char *s; - - if (sd_device_get_sysattr_value(pdev, "capabilities/ev", &s) >= 0) + if (sd_device_get_sysattr_value(pdev, "capabilities/ev", /* ret= */ NULL) >= 0) break; if (sd_device_get_parent_with_subsystem_devtype(pdev, "input", NULL, &pdev) >= 0)