From cff31876dabdfdc0d70c0b72917d6b66ab973a54 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 24 Jun 2022 13:00:34 +0900 Subject: [PATCH 1/3] sd-device: change type of properties nulstr from uint8_t* to char* --- src/libsystemd/sd-device/device-internal.h | 2 +- src/libsystemd/sd-device/device-monitor.c | 4 ++-- src/libsystemd/sd-device/device-private.c | 28 ++++++++++------------ src/libsystemd/sd-device/device-private.h | 4 ++-- src/libsystemd/sd-device/test-sd-device.c | 7 +++--- 5 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/libsystemd/sd-device/device-internal.h b/src/libsystemd/sd-device/device-internal.h index 7ea964acf9..09325aae04 100644 --- a/src/libsystemd/sd-device/device-internal.h +++ b/src/libsystemd/sd-device/device-internal.h @@ -55,7 +55,7 @@ struct sd_device { dev_t devnum; char **properties_strv; /* the properties hashmap as a strv */ - uint8_t *properties_nulstr; /* the same as a nulstr */ + char *properties_nulstr; /* the same as a nulstr */ size_t properties_nulstr_len; char *syspath; diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index 80cb0ce622..e8913c3d1f 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -508,7 +508,7 @@ int device_monitor_receive_device(sd_device_monitor *m, sd_device **ret) { "sd-device-monitor: Invalid message header"); } - r = device_new_from_nulstr(&device, (uint8_t*) &buf.raw[bufpos], buflen - bufpos); + r = device_new_from_nulstr(&device, &buf.raw[bufpos], buflen - bufpos); if (r < 0) return log_debug_errno(r, "sd-device-monitor: Failed to create device from received message: %m"); @@ -574,7 +574,7 @@ int device_monitor_send_device( assert(m); assert(device); - r = device_get_properties_nulstr(device, (const uint8_t **) &buf, &blen); + r = device_get_properties_nulstr(device, &buf, &blen); if (r < 0) return log_device_debug_errno(device, r, "sd-device-monitor: Failed to get device properties: %m"); if (blen < 32) diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index 4a5a110364..44875242f4 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -467,7 +467,7 @@ int device_new_from_strv(sd_device **ret, char **strv) { return 0; } -int device_new_from_nulstr(sd_device **ret, uint8_t *nulstr, size_t len) { +int device_new_from_nulstr(sd_device **ret, char *nulstr, size_t len) { _cleanup_(sd_device_unrefp) sd_device *device = NULL; const char *major = NULL, *minor = NULL; int r; @@ -484,7 +484,7 @@ int device_new_from_nulstr(sd_device **ret, uint8_t *nulstr, size_t len) { char *key; const char *end; - key = (char*) &nulstr[i]; + key = nulstr + i; end = memchr(key, '\0', len - i); if (!end) return log_device_debug_errno(device, SYNTHETIC_ERRNO(EINVAL), @@ -517,10 +517,9 @@ int device_new_from_nulstr(sd_device **ret, uint8_t *nulstr, size_t len) { } static int device_update_properties_bufs(sd_device *device) { + _cleanup_free_ char **buf_strv = NULL, *buf_nulstr = NULL; + size_t nulstr_len = 0, num = 0; const char *val, *prop; - _cleanup_free_ char **buf_strv = NULL; - _cleanup_free_ uint8_t *buf_nulstr = NULL; - size_t nulstr_len = 0, num = 0, i = 0; assert(device); @@ -536,32 +535,31 @@ static int device_update_properties_bufs(sd_device *device) { if (!buf_nulstr) return -ENOMEM; - strscpyl((char *)buf_nulstr + nulstr_len, len + 1, prop, "=", val, NULL); + strscpyl(buf_nulstr + nulstr_len, len + 1, prop, "=", val, NULL); nulstr_len += len + 1; - ++num; + num++; } /* build buf_strv from buf_nulstr */ - buf_strv = new0(char *, num + 1); + buf_strv = new0(char*, num + 1); if (!buf_strv) return -ENOMEM; - NULSTR_FOREACH(val, (char*) buf_nulstr) { - buf_strv[i] = (char *) val; - assert(i < num); - i++; - } + size_t i = 0; + char *p; + NULSTR_FOREACH(p, buf_nulstr) + buf_strv[i++] = p; + assert(i == num); free_and_replace(device->properties_nulstr, buf_nulstr); device->properties_nulstr_len = nulstr_len; free_and_replace(device->properties_strv, buf_strv); device->properties_buf_outdated = false; - return 0; } -int device_get_properties_nulstr(sd_device *device, const uint8_t **nulstr, size_t *len) { +int device_get_properties_nulstr(sd_device *device, const char **nulstr, size_t *len) { int r; assert(device); diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 5160f58006..2ffd6d967c 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -10,7 +10,7 @@ #include "macro.h" -int device_new_from_nulstr(sd_device **ret, uint8_t *nulstr, size_t len); +int device_new_from_nulstr(sd_device **ret, char *nulstr, size_t len); int device_new_from_strv(sd_device **ret, char **strv); int device_new_from_watch_handle_at(sd_device **ret, int dirfd, int wd); static inline int device_new_from_watch_handle(sd_device **ret, int wd) { @@ -48,7 +48,7 @@ uint64_t device_get_tags_generation(sd_device *device); uint64_t device_get_devlinks_generation(sd_device *device); int device_properties_prepare(sd_device *device); -int device_get_properties_nulstr(sd_device *device, const uint8_t **nulstr, size_t *len); +int device_get_properties_nulstr(sd_device *device, const char **nulstr, size_t *len); int device_get_properties_strv(sd_device *device, char ***strv); int device_rename(sd_device *device, const char *name); diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index 6a03582fd9..701a68269e 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -320,9 +320,8 @@ TEST(sd_device_new_from_nulstr) { "\0"; _cleanup_(sd_device_unrefp) sd_device *device = NULL, *from_nulstr = NULL; - _cleanup_free_ uint8_t *nulstr_copy = NULL; - const char *devlink; - const uint8_t *nulstr; + _cleanup_free_ char *nulstr_copy = NULL; + const char *devlink, *nulstr; size_t len; assert_se(sd_device_new_from_syspath(&device, "/sys/class/net/lo") >= 0); @@ -340,7 +339,7 @@ TEST(sd_device_new_from_nulstr) { assert_se(device_add_property_internal(device, "ACTION", "change") >= 0); assert_se(device_get_properties_nulstr(device, &nulstr, &len) >= 0); - assert_se(nulstr_copy = newdup(uint8_t, nulstr, len)); + assert_se(nulstr_copy = newdup(char, nulstr, len)); assert_se(device_new_from_nulstr(&from_nulstr, nulstr_copy, len) >= 0); NULSTR_FOREACH(devlink, devlinks) { From 793ab3e9dd733d743e1d3825a26ff65384ac3cbb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 9 Jul 2022 04:13:49 +0900 Subject: [PATCH 2/3] sd-device: make device_get_properties_{nulstr,strv}() take NULL for result value In most cases, it is not necessary to call them without retrieving result. But, most of other getter functions for sd-device can take NULL. Let's follow the way for consistency. --- src/libsystemd/sd-device/device-private.c | 16 ++++++++-------- src/libsystemd/sd-device/device-private.h | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index 44875242f4..90ffd29c57 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -559,34 +559,34 @@ static int device_update_properties_bufs(sd_device *device) { return 0; } -int device_get_properties_nulstr(sd_device *device, const char **nulstr, size_t *len) { +int device_get_properties_nulstr(sd_device *device, const char **ret_nulstr, size_t *ret_len) { int r; assert(device); - assert(nulstr); - assert(len); r = device_update_properties_bufs(device); if (r < 0) return r; - *nulstr = device->properties_nulstr; - *len = device->properties_nulstr_len; + if (ret_nulstr) + *ret_nulstr = device->properties_nulstr; + if (ret_len) + *ret_len = device->properties_nulstr_len; return 0; } -int device_get_properties_strv(sd_device *device, char ***strv) { +int device_get_properties_strv(sd_device *device, char ***ret) { int r; assert(device); - assert(strv); r = device_update_properties_bufs(device); if (r < 0) return r; - *strv = device->properties_strv; + if (ret) + *ret = device->properties_strv; return 0; } diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 2ffd6d967c..9602f9eda3 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -48,8 +48,8 @@ uint64_t device_get_tags_generation(sd_device *device); uint64_t device_get_devlinks_generation(sd_device *device); int device_properties_prepare(sd_device *device); -int device_get_properties_nulstr(sd_device *device, const char **nulstr, size_t *len); -int device_get_properties_strv(sd_device *device, char ***strv); +int device_get_properties_nulstr(sd_device *device, const char **ret_nulstr, size_t *ret_len); +int device_get_properties_strv(sd_device *device, char ***ret); int device_rename(sd_device *device, const char *name); int device_shallow_clone(sd_device *device, sd_device **ret); From 4bc4040bc0a57e8bdd811c53b0db7cd443315f33 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 24 Jun 2022 13:05:13 +0900 Subject: [PATCH 3/3] sd-device: send udev database version Otherwise, sd-device object received through sd-device-monitor does not show current tags. Fixes #23799. --- src/libsystemd/sd-device/device-private.c | 13 +++++++++++++ src/libsystemd/sd-device/test-sd-device.c | 15 +++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index 90ffd29c57..a453899910 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -356,6 +356,10 @@ static int device_amend(sd_device *device, const char *key, const char *value) { if (r < 0) return log_device_debug_errno(device, r, "sd-device: Failed to add tag '%s': %m", word); } + } else if (streq(key, "UDEV_DATABASE_VERSION")) { + r = safe_atou(value, &device->database_version); + if (r < 0) + return log_device_debug_errno(device, r, "sd-device: Failed to parse udev database version '%s': %m", value); } else { r = device_add_property_internal(device, key, value); if (r < 0) @@ -526,6 +530,15 @@ static int device_update_properties_bufs(sd_device *device) { if (!device->properties_buf_outdated) return 0; + /* append udev database version */ + buf_nulstr = newdup(char, "UDEV_DATABASE_VERSION=" STRINGIFY(LATEST_UDEV_DATABASE_VERSION) "\0", + STRLEN("UDEV_DATABASE_VERSION=" STRINGIFY(LATEST_UDEV_DATABASE_VERSION)) + 2); + if (!buf_nulstr) + return -ENOMEM; + + nulstr_len += STRLEN("UDEV_DATABASE_VERSION=" STRINGIFY(LATEST_UDEV_DATABASE_VERSION)) + 1; + num++; + FOREACH_DEVICE_PROPERTY(device, prop, val) { size_t len = 0; diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index 701a68269e..91e358e464 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -334,6 +334,12 @@ TEST(sd_device_new_from_nulstr) { assert_se(set_contains(device->devlinks, devlink)); } + /* For issue #23799 */ + assert_se(device_add_tag(device, "tag1", false) >= 0); + assert_se(device_add_tag(device, "tag2", false) >= 0); + assert_se(device_add_tag(device, "current-tag1", true) >= 0); + assert_se(device_add_tag(device, "current-tag2", true) >= 0); + /* These properties are necessary for device_new_from_nulstr(). See device_verify(). */ assert_se(device_add_property_internal(device, "SEQNUM", "1") >= 0); assert_se(device_add_property_internal(device, "ACTION", "change") >= 0); @@ -342,6 +348,15 @@ TEST(sd_device_new_from_nulstr) { assert_se(nulstr_copy = newdup(char, nulstr, len)); assert_se(device_new_from_nulstr(&from_nulstr, nulstr_copy, len) >= 0); + assert_se(sd_device_has_tag(from_nulstr, "tag1") == 1); + assert_se(sd_device_has_tag(from_nulstr, "tag2") == 1); + assert_se(sd_device_has_tag(from_nulstr, "current-tag1") == 1); + assert_se(sd_device_has_tag(from_nulstr, "current-tag2") == 1); + assert_se(sd_device_has_current_tag(from_nulstr, "tag1") == 0); + assert_se(sd_device_has_current_tag(from_nulstr, "tag2") == 0); + assert_se(sd_device_has_current_tag(from_nulstr, "current-tag1") == 1); + assert_se(sd_device_has_current_tag(from_nulstr, "current-tag2") == 1); + NULSTR_FOREACH(devlink, devlinks) { log_device_info(from_nulstr, "checking devlink: %s", devlink); assert_se(set_contains(from_nulstr->devlinks, devlink));