locale-util,kbd-util: several cleanups (systemd#37090)

Author: yuwata

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;

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,18 +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;
-        size_t sz = 0;
-        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;
 
@@ -117,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 ||
@@ -154,9 +152,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;
 }
@@ -165,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;
@@ -180,19 +180,18 @@ 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;
         }
 
         return 0;
 }
 
 int get_locales(char ***ret) {
-        _cleanup_set_free_free_ Set *locales = NULL;
-        _cleanup_strv_free_ char **l = 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;
 
@@ -213,31 +212,25 @@ int get_locales(char ***ret) {
                         free(set_remove(locales, locale));
         }
 
-        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 (IN_SET(r, -ENXIO, 0)) {
-                char **a, **b;
+        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");
 
                 /* 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;
-
-        } else if (r < 0)
-                log_debug_errno(r, "Failed to parse $SYSTEMD_LIST_NON_UTF8_LOCALES as boolean");
+        }
 
         strv_sort(l);
 

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)
 

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

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);

src/shared/kbd-util.c

@@ -83,15 +83,15 @@ 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;
 
         r = keymap_directories(&keymap_dirs);
         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;
 

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);