From 90abb64fd508e8efd937178b3379a62ac97b49ec Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 09:11:05 +0900 Subject: [PATCH 1/7] locale-util: fix argument for munmap() --- src/basic/locale-util.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/basic/locale-util.c b/src/basic/locale-util.c index c7095ece23..45ba70f04d 100644 --- a/src/basic/locale-util.c +++ b/src/basic/locale-util.c @@ -97,7 +97,6 @@ static int add_locales_from_archive(Set *locales) { const struct namehashent *e; const void *p = MAP_FAILED; _cleanup_close_ int fd = -EBADF; - size_t sz = 0; struct stat st; int r; @@ -154,9 +153,9 @@ static int add_locales_from_archive(Set *locales) { r = 0; - finish: +finish: if (p != MAP_FAILED) - munmap((void*) p, sz); + munmap((void*) p, st.st_size); return r; } From a4a5ee4cd1ce28a8ed4bbb211b4909f0bfa96438 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 09:13:39 +0900 Subject: [PATCH 2/7] locale-util: filter out non-UTF-8 locales even when failed to parse $SYSTEMD_LIST_NON_UTF8_LOCALES --- src/basic/locale-util.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/basic/locale-util.c b/src/basic/locale-util.c index 45ba70f04d..3fb9540f45 100644 --- a/src/basic/locale-util.c +++ b/src/basic/locale-util.c @@ -220,7 +220,10 @@ int get_locales(char ***ret) { locales = set_free(locales); r = getenv_bool("SYSTEMD_LIST_NON_UTF8_LOCALES"); - if (IN_SET(r, -ENXIO, 0)) { + if (r <= 0) { + if (!IN_SET(r, -ENXIO, 0)) + log_debug_errno(r, "Failed to parse $SYSTEMD_LIST_NON_UTF8_LOCALES as boolean, ignoring: %m"); + char **a, **b; /* Filter out non-UTF-8 locales, because it's 2019, by default */ @@ -234,9 +237,7 @@ int get_locales(char ***ret) { } *b = NULL; - - } else if (r < 0) - log_debug_errno(r, "Failed to parse $SYSTEMD_LIST_NON_UTF8_LOCALES as boolean"); + } strv_sort(l); From 4da43aeb0cc746439833f7da0753b084d774d14b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 09:15:06 +0900 Subject: [PATCH 3/7] locale-util: coding style cleanups --- src/basic/locale-util.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/basic/locale-util.c b/src/basic/locale-util.c index 3fb9540f45..2e4609b7fc 100644 --- a/src/basic/locale-util.c +++ b/src/basic/locale-util.c @@ -25,7 +25,7 @@ #include "strv.h" #include "utf8.h" -static char *normalize_locale(const char *name) { +static char* normalize_locale(const char *name) { const char *e; /* Locale names are weird: glibc has some magic rules when looking for the charset name on disk: it @@ -93,17 +93,15 @@ static int add_locales_from_archive(Set *locales) { uint32_t locrec_offset; }; - const struct locarhead *h; - const struct namehashent *e; - const void *p = MAP_FAILED; - _cleanup_close_ int fd = -EBADF; - struct stat st; int r; - fd = open("/usr/lib/locale/locale-archive", O_RDONLY|O_NOCTTY|O_CLOEXEC); + assert(locales); + + _cleanup_close_ int fd = open("/usr/lib/locale/locale-archive", O_RDONLY|O_NOCTTY|O_CLOEXEC); if (fd < 0) return errno == ENOENT ? 0 : -errno; + struct stat st; if (fstat(fd, &st) < 0) return -errno; @@ -116,11 +114,12 @@ static int add_locales_from_archive(Set *locales) { if (file_offset_beyond_memory_size(st.st_size)) return -EFBIG; - p = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0); + void *p = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0); if (p == MAP_FAILED) return -errno; - h = (const struct locarhead *) p; + const struct namehashent *e; + const struct locarhead *h = p; if (h->magic != 0xde020109 || h->namehash_offset + h->namehash_size > st.st_size || h->string_offset + h->string_size > st.st_size || @@ -164,6 +163,8 @@ static int add_locales_from_libdir(Set *locales) { _cleanup_closedir_ DIR *dir = NULL; int r; + assert(locales); + dir = opendir("/usr/lib/locale"); if (!dir) return errno == ENOENT ? 0 : -errno; @@ -179,7 +180,7 @@ static int add_locales_from_libdir(Set *locales) { return -ENOMEM; r = set_consume(locales, z); - if (r < 0 && r != -EEXIST) + if (r < 0) return r; } @@ -188,7 +189,6 @@ static int add_locales_from_libdir(Set *locales) { int get_locales(char ***ret) { _cleanup_set_free_free_ Set *locales = NULL; - _cleanup_strv_free_ char **l = NULL; int r; locales = set_new(&string_hash_ops); @@ -212,7 +212,7 @@ int get_locales(char ***ret) { free(set_remove(locales, locale)); } - l = set_get_strv(locales); + _cleanup_strv_free_ char **l = set_get_strv(locales); if (!l) return -ENOMEM; @@ -224,17 +224,13 @@ int get_locales(char ***ret) { if (!IN_SET(r, -ENXIO, 0)) log_debug_errno(r, "Failed to parse $SYSTEMD_LIST_NON_UTF8_LOCALES as boolean, ignoring: %m"); - char **a, **b; - /* Filter out non-UTF-8 locales, because it's 2019, by default */ - for (a = b = l; *a; a++) { - - if (endswith(*a, "UTF-8") || - strstr(*a, ".UTF-8@")) + char **b = l; + STRV_FOREACH(a, l) + if (endswith(*a, "UTF-8") || strstr(*a, ".UTF-8@")) *(b++) = *a; else free(*a); - } *b = NULL; } From 24655047b06895d3552f700f4a6cf006b4dbced8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 09:54:21 +0900 Subject: [PATCH 4/7] set: introduce set_to_strv() It is similar to set_get_strv(), but invalidates the set on success. --- src/basic/hashmap.c | 17 +++++++++++++++++ src/basic/set.h | 2 ++ src/test/test-set.c | 24 ++++++++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index 951f63ae6d..a547b8c31c 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -1805,6 +1805,23 @@ char** _hashmap_get_strv(HashmapBase *h) { return sv; } +char** set_to_strv(Set **s) { + assert(s); + + /* This is similar to set_get_strv(), but invalidates the set on success. */ + + char **v = new(char*, set_size(*s) + 1); + if (!v) + return NULL; + + for (char **p = v; (*p = set_steal_first(*s)); p++) + ; + + assert(set_isempty(*s)); + *s = set_free(*s); + return v; +} + void* ordered_hashmap_next(OrderedHashmap *h, const void *key) { struct ordered_hashmap_entry *e; unsigned hash, idx; diff --git a/src/basic/set.h b/src/basic/set.h index 618e729744..05d7d88875 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -114,6 +114,8 @@ static inline char **set_get_strv(Set *s) { return _hashmap_get_strv(HASHMAP_BASE(s)); } +char** set_to_strv(Set **s); + int _set_ensure_put(Set **s, const struct hash_ops *hash_ops, const void *key HASHMAP_DEBUG_PARAMS); #define set_ensure_put(s, hash_ops, key) _set_ensure_put(s, hash_ops, key HASHMAP_DEBUG_SRC_ARGS) diff --git a/src/test/test-set.c b/src/test/test-set.c index f1e6e9314b..19b46a529f 100644 --- a/src/test/test-set.c +++ b/src/test/test-set.c @@ -400,4 +400,28 @@ TEST(set_fnmatch) { assert_se(!set_fnmatch(match, nomatch, "cccXX")); } +TEST(set_to_strv) { + _cleanup_set_free_ Set *set = NULL; + _cleanup_strv_free_ char **a = NULL; + _cleanup_free_ char **b = NULL; + char **v = STRV_MAKE("aaa", "bbb", "ccc"); + + ASSERT_NOT_NULL(a = set_to_strv(&set)); + ASSERT_TRUE(strv_isempty(a)); + ASSERT_NULL(set); + a = strv_free(a); + + ASSERT_OK(set_put_strdupv(&set, v)); + ASSERT_EQ(set_size(set), strv_length(v)); + + ASSERT_NOT_NULL(b = set_get_strv(set)); + ASSERT_EQ(strv_length(b), strv_length(v)); + ASSERT_TRUE(strv_equal_ignore_order(b, v)); + + ASSERT_NOT_NULL(a = set_to_strv(&set)); + ASSERT_EQ(strv_length(a), strv_length(v)); + ASSERT_TRUE(strv_equal_ignore_order(a, v)); + ASSERT_NULL(set); +} + DEFINE_TEST_MAIN(LOG_INFO); From 3f23b37c5a4ba86d9ecb446dc8030bb404fb8da7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 09:57:07 +0900 Subject: [PATCH 5/7] strv: constify arguments for strv_equal_ignore_order() Follow-up for 5072f4268b89a71e47e59c434da0222f722c7f0e. --- src/basic/strv.c | 2 +- src/basic/strv.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/strv.c b/src/basic/strv.c index a4ddaa5e78..6a2ff09cbd 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -861,7 +861,7 @@ int strv_compare(char * const *a, char * const *b) { return 0; } -bool strv_equal_ignore_order(char **a, char **b) { +bool strv_equal_ignore_order(char * const *a, char * const *b) { /* Just like strv_equal(), but doesn't care about the order of elements or about redundant entries * (i.e. it's even ok if the number of entries in the array differ, as long as the difference just diff --git a/src/basic/strv.h b/src/basic/strv.h index 71aae2b20a..0ca90087b4 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -96,7 +96,7 @@ static inline bool strv_equal(char * const *a, char * const *b) { return strv_compare(a, b) == 0; } -bool strv_equal_ignore_order(char **a, char **b); +bool strv_equal_ignore_order(char * const *a, char * const *b); char** strv_new_internal(const char *x, ...) _sentinel_; char** strv_new_ap(const char *x, va_list ap); From 21d84e8905766ad7d33d5a189ae944dd06ab5257 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 10:04:06 +0900 Subject: [PATCH 6/7] locale-util: use string_hash_ops_free and set_to_strv() Then we can drop hackish combination of use of set_free_free() and set_free(). --- src/basic/locale-util.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/basic/locale-util.c b/src/basic/locale-util.c index 2e4609b7fc..2fbab01aeb 100644 --- a/src/basic/locale-util.c +++ b/src/basic/locale-util.c @@ -188,10 +188,10 @@ static int add_locales_from_libdir(Set *locales) { } int get_locales(char ***ret) { - _cleanup_set_free_free_ Set *locales = NULL; + _cleanup_set_free_ Set *locales = NULL; int r; - locales = set_new(&string_hash_ops); + locales = set_new(&string_hash_ops_free); if (!locales) return -ENOMEM; @@ -212,13 +212,10 @@ int get_locales(char ***ret) { free(set_remove(locales, locale)); } - _cleanup_strv_free_ char **l = set_get_strv(locales); + _cleanup_strv_free_ char **l = set_to_strv(&locales); if (!l) return -ENOMEM; - /* Now, all elements are owned by strv 'l'. Hence, do not call set_free_free(). */ - locales = set_free(locales); - r = getenv_bool("SYSTEMD_LIST_NON_UTF8_LOCALES"); if (r <= 0) { if (!IN_SET(r, -ENXIO, 0)) From 002ff90d8cb5e7f0924916802e0da86d18db1a79 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Apr 2025 11:43:32 +0900 Subject: [PATCH 7/7] kbd-util: use string_hash_ops_free and set_to_strv() --- src/shared/kbd-util.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/shared/kbd-util.c b/src/shared/kbd-util.c index 60e0429b82..fa06d8a965 100644 --- a/src/shared/kbd-util.c +++ b/src/shared/kbd-util.c @@ -83,7 +83,7 @@ static int keymap_recurse_dir_callback( } int get_keymaps(char ***ret) { - _cleanup_set_free_free_ Set *keymaps = NULL; + _cleanup_set_free_ Set *keymaps = NULL; _cleanup_strv_free_ char **keymap_dirs = NULL; int r; @@ -91,7 +91,7 @@ int get_keymaps(char ***ret) { if (r < 0) return r; - keymaps = set_new(&string_hash_ops); + keymaps = set_new(&string_hash_ops_free); if (!keymaps) return -ENOMEM; @@ -114,14 +114,10 @@ int get_keymaps(char ***ret) { log_debug_errno(r, "Failed to read keymap list from %s, ignoring: %m", *dir); } - _cleanup_strv_free_ char **l = set_get_strv(keymaps); + _cleanup_strv_free_ char **l = set_to_strv(&keymaps); if (!l) return -ENOMEM; - keymaps = set_free(keymaps); /* If we got the strv above, then do a set_free() rather than - * set_free_free() since the entries of the set are now owned by the - * strv */ - if (strv_isempty(l)) return -ENOENT;