locale: several coding style fixlets

- add missing assertions,
- rename arguments for storing results,
- always initialize results on success,
- use _cleanup_ attribute at more places, etc.

diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c
index 4f2d3aaeb868305962810afa4a7b9ed673ff0324..726d41e3995ea1e79c77f9f78eab17a0a006cdf4 100644 (file)
--- a/src/locale/localed-util.c
+++ b/src/locale/localed-util.c
@@ -26,6 +26,9 @@
 #include "tmpfile-util.h"
 
 static bool startswith_comma(const char *s, const char *prefix) {
+        assert(s);
+        assert(prefix);
+
         s = startswith(s, prefix);
         if (!s)
                 return false;
@@ -54,6 +57,8 @@ static const char* systemd_language_fallback_map(void) {
 }
 
 static void context_free_x11(Context *c) {
+        assert(c);
+
         c->x11_layout = mfree(c->x11_layout);
         c->x11_options = mfree(c->x11_options);
         c->x11_model = mfree(c->x11_model);
@@ -61,11 +66,15 @@ static void context_free_x11(Context *c) {
 }
 
 static void context_free_vconsole(Context *c) {
+        assert(c);
+
         c->vc_keymap = mfree(c->vc_keymap);
         c->vc_keymap_toggle = mfree(c->vc_keymap_toggle);
 }
 
 void context_clear(Context *c) {
+        assert(c);
+
         locale_context_clear(&c->locale_context);
         context_free_x11(c);
         context_free_vconsole(c);
@@ -96,6 +105,8 @@ int vconsole_read_data(Context *c, sd_bus_message *m) {
         _cleanup_close_ int fd = -EBADF;
         struct stat st;
 
+        assert(c);
+
         /* Do not try to re-read the file within single bus operation. */
         if (m) {
                 if (m == c->vc_cache)
@@ -136,6 +147,8 @@ int x11_read_data(Context *c, sd_bus_message *m) {
         struct stat st;
         int r;
 
+        assert(c);
+
         /* Do not try to re-read the file within single bus operation. */
         if (m) {
                 if (m == c->x11_cache)
@@ -232,6 +245,8 @@ int vconsole_write_data(Context *c) {
         _cleanup_strv_free_ char **l = NULL;
         int r;
 
+        assert(c);
+
         r = load_env_file(NULL, "/etc/vconsole.conf", &l);
         if (r < 0 && r != -ENOENT)
                 return r;
@@ -267,6 +282,8 @@ int x11_write_data(Context *c) {
         _cleanup_(unlink_and_freep) char *temp_path = NULL;
         int r;
 
+        assert(c);
+
         if (isempty(c->x11_layout) &&
             isempty(c->x11_model) &&
             isempty(c->x11_variant) &&
@@ -320,17 +337,23 @@ int x11_write_data(Context *c) {
         return 0;
 }
 
-static int read_next_mapping(const char* filename,
-                             unsigned min_fields, unsigned max_fields,
-                             FILE *f, unsigned *n, char ***a) {
+static int read_next_mapping(
+                const char *filename,
+                unsigned min_fields,
+                unsigned max_fields,
+                FILE *f,
+                unsigned *n,
+                char ***ret) {
+
         assert(f);
         assert(n);
-        assert(a);
+        assert(ret);
 
         for (;;) {
+                _cleanup_strv_free_ char **b = NULL;
                 _cleanup_free_ char *line = NULL;
                 size_t length;
-                char *l, **b;
+                const char *l;
                 int r;
 
                 r = read_line(f, LONG_LINE_MAX, &line);
@@ -351,24 +374,23 @@ static int read_next_mapping(const char* filename,
 
                 length = strv_length(b);
                 if (length < min_fields || length > max_fields) {
-                        log_error("Invalid line %s:%u, ignoring.", filename, *n);
-                        strv_free(b);
+                        log_warning("Invalid line %s:%u, ignoring.", strna(filename), *n);
                         continue;
 
                 }
 
-                *a = b;
+                *ret = TAKE_PTR(b);
                 return 1;
         }
 
+        *ret = NULL;
         return 0;
 }
 
 int vconsole_convert_to_x11(Context *c) {
-        const char *map;
-        int modified = -1;
+        int r, modified = -1;
 
-        map = systemd_kbd_model_map();
+        assert(c);
 
         if (isempty(c->vc_keymap)) {
                 modified =
@@ -380,15 +402,15 @@ int vconsole_convert_to_x11(Context *c) {
                 context_free_x11(c);
         } else {
                 _cleanup_fclose_ FILE *f = NULL;
-                unsigned n = 0;
+                const char *map;
 
+                map = systemd_kbd_model_map();
                 f = fopen(map, "re");
                 if (!f)
                         return -errno;
 
-                for (;;) {
+                for (unsigned n = 0;;) {
                         _cleanup_strv_free_ char **a = NULL;
-                        int r;
 
                         r = read_next_mapping(map, 5, UINT_MAX, f, &n, &a);
                         if (r < 0)
@@ -432,9 +454,12 @@ int vconsole_convert_to_x11(Context *c) {
         return modified > 0;
 }
 
-int find_converted_keymap(const char *x11_layout, const char *x11_variant, char **new_keymap) {
+int find_converted_keymap(const char *x11_layout, const char *x11_variant, char **ret) {
         _cleanup_free_ char *n = NULL;
 
+        assert(x11_layout);
+        assert(ret);
+
         if (x11_variant)
                 n = strjoin(x11_layout, "-", x11_variant);
         else
@@ -453,14 +478,13 @@ int find_converted_keymap(const char *x11_layout, const char *x11_variant, char
 
                 uncompressed = access(p, F_OK) == 0;
                 if (uncompressed || access(pz, F_OK) == 0) {
-                        log_debug("Found converted keymap %s at %s",
-                                  n, uncompressed ? p : pz);
-
-                        *new_keymap = TAKE_PTR(n);
+                        log_debug("Found converted keymap %s at %s", n, uncompressed ? p : pz);
+                        *ret = TAKE_PTR(n);
                         return 1;
                 }
         }
 
+        *ret = NULL;
         return 0;
 }
 
@@ -468,19 +492,18 @@ int find_legacy_keymap(Context *c, char **ret) {
         const char *map;
         _cleanup_fclose_ FILE *f = NULL;
         _cleanup_free_ char *new_keymap = NULL;
-        unsigned n = 0;
         unsigned best_matching = 0;
         int r;
 
+        assert(c);
         assert(!isempty(c->x11_layout));
 
         map = systemd_kbd_model_map();
-
         f = fopen(map, "re");
         if (!f)
                 return -errno;
 
-        for (;;) {
+        for (unsigned n = 0;;) {
                 _cleanup_strv_free_ char **a = NULL;
                 unsigned matching = 0;
 
@@ -492,7 +515,7 @@ int find_legacy_keymap(Context *c, char **ret) {
 
                 /* Determine how well matching this entry is */
                 if (streq(c->x11_layout, a[1]))
-                        /* If we got an exact match, this is best */
+                        /* If we got an exact match, this is the best */
                         matching = 10;
                 else {
                         /* We have multiple X layouts, look for an
@@ -500,7 +523,7 @@ int find_legacy_keymap(Context *c, char **ret) {
                          * but the first layout stripped off. */
                         if (startswith_comma(c->x11_layout, a[1]))
                                 matching = 5;
-                        else  {
+                        else {
                                 _cleanup_free_ char *x = NULL;
 
                                 /* If that didn't work, strip off the
@@ -528,8 +551,7 @@ int find_legacy_keymap(Context *c, char **ret) {
 
                 /* The best matching entry so far, then let's save that */
                 if (matching >= MAX(best_matching, 1u)) {
-                        log_debug("Found legacy keymap %s with score %u",
-                                  a[0], matching);
+                        log_debug("Found legacy keymap %s with score %u", a[0], matching);
 
                         if (matching > best_matching) {
                                 best_matching = matching;
@@ -560,26 +582,25 @@ int find_legacy_keymap(Context *c, char **ret) {
         }
 
         *ret = TAKE_PTR(new_keymap);
-        return (bool) *ret;
+        return !!*ret;
 }
 
-int find_language_fallback(const char *lang, char **language) {
+int find_language_fallback(const char *lang, char **ret) {
         const char *map;
         _cleanup_fclose_ FILE *f = NULL;
         unsigned n = 0;
+        int r;
 
         assert(lang);
-        assert(language);
+        assert(ret);
 
         map = systemd_language_fallback_map();
-
         f = fopen(map, "re");
         if (!f)
                 return -errno;
 
         for (;;) {
                 _cleanup_strv_free_ char **a = NULL;
-                int r;
 
                 r = read_next_mapping(map, 2, 2, f, &n, &a);
                 if (r <= 0)
@@ -587,17 +608,17 @@ int find_language_fallback(const char *lang, char **language) {
 
                 if (streq(lang, a[0])) {
                         assert(strv_length(a) == 2);
-                        *language = TAKE_PTR(a[1]);
+                        *ret = TAKE_PTR(a[1]);
                         return 1;
                 }
         }
-
-        assert_not_reached();
 }
 
 int x11_convert_to_vconsole(Context *c) {
         bool modified = false;
 
+        assert(c);
+
         if (isempty(c->x11_layout)) {
                 modified =
                         !isempty(c->vc_keymap) ||
@@ -609,19 +630,15 @@ int x11_convert_to_vconsole(Context *c) {
                 int r;
 
                 r = find_converted_keymap(c->x11_layout, c->x11_variant, &new_keymap);
+                if (r == 0)
+                        r = find_legacy_keymap(c, &new_keymap);
                 if (r < 0)
                         return r;
-                else if (r == 0) {
-                        r = find_legacy_keymap(c, &new_keymap);
-                        if (r < 0)
-                                return r;
-                }
                 if (r == 0)
                         /* We search for layout-variant match first, but then we also look
                          * for anything which matches just the layout. So it's accurate to say
                          * that we couldn't find anything which matches the layout. */
-                        log_notice("No conversion to virtual console map found for \"%s\".",
-                                   c->x11_layout);
+                        log_notice("No conversion to virtual console map found for \"%s\".", c->x11_layout);
 
                 if (!streq_ptr(c->vc_keymap, new_keymap)) {
                         free_and_replace(c->vc_keymap, new_keymap);

diff --git a/src/locale/localed-util.h b/src/locale/localed-util.h
index b09ed80b83f87382f42873263880060905c64128..9235cff00f9d19c7ce670ff32ace834d17986d0c 100644 (file)
--- a/src/locale/localed-util.h
+++ b/src/locale/localed-util.h
@@ -27,9 +27,9 @@ typedef struct Context {
         Hashmap *polkit_registry;
 } Context;
 
-int find_converted_keymap(const char *x11_layout, const char *x11_variant, char **new_keymap);
-int find_legacy_keymap(Context *c, char **new_keymap);
-int find_language_fallback(const char *lang, char **language);
+int find_converted_keymap(const char *x11_layout, const char *x11_variant, char **ret);
+int find_legacy_keymap(Context *c, char **ret);
+int find_language_fallback(const char *lang, char **ret);
 
 int locale_read_data(Context *c, sd_bus_message *m);
 int vconsole_read_data(Context *c, sd_bus_message *m);

diff --git a/src/locale/localed.c b/src/locale/localed.c
index 83947515368a7dac80c0c29537fe669c428f4327..7a9be93ccb7b1d376c0f3032ca2a4a272b9c2a37 100644 (file)
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -78,6 +78,7 @@ static int vconsole_reload(sd_bus *bus) {
 static int vconsole_convert_to_x11_and_emit(Context *c, sd_bus_message *m) {
         int r;
 
+        assert(c);
         assert(m);
 
         r = x11_read_data(c, m);
@@ -104,6 +105,7 @@ static int vconsole_convert_to_x11_and_emit(Context *c, sd_bus_message *m) {
 static int x11_convert_to_vconsole_and_emit(Context *c, sd_bus_message *m) {
         int r;
 
+        assert(c);
         assert(m);
 
         r = vconsole_read_data(c, m);
@@ -136,7 +138,7 @@ static int property_get_locale(
                 void *userdata,
                 sd_bus_error *error) {
 
-        Context *c = userdata;
+        Context *c = ASSERT_PTR(userdata);
         _cleanup_strv_free_ char **l = NULL;
         int r;
 
@@ -160,16 +162,18 @@ static int property_get_vconsole(
                 void *userdata,
                 sd_bus_error *error) {
 
-        Context *c = userdata;
+        Context *c = ASSERT_PTR(userdata);
         int r;
 
+        assert(property);
+
         r = vconsole_read_data(c, reply);
         if (r < 0)
                 return r;
 
         if (streq(property, "VConsoleKeymap"))
                 return sd_bus_message_append_basic(reply, 's', c->vc_keymap);
-        else if (streq(property, "VConsoleKeymapToggle"))
+        if (streq(property, "VConsoleKeymapToggle"))
                 return sd_bus_message_append_basic(reply, 's', c->vc_keymap_toggle);
 
         return -EINVAL;
@@ -184,20 +188,22 @@ static int property_get_xkb(
                 void *userdata,
                 sd_bus_error *error) {
 
-        Context *c = userdata;
+        Context *c = ASSERT_PTR(userdata);
         int r;
 
+        assert(property);
+
         r = x11_read_data(c, reply);
         if (r < 0)
                 return r;
 
         if (streq(property, "X11Layout"))
                 return sd_bus_message_append_basic(reply, 's', c->x11_layout);
-        else if (streq(property, "X11Model"))
+        if (streq(property, "X11Model"))
                 return sd_bus_message_append_basic(reply, 's', c->x11_model);
-        else if (streq(property, "X11Variant"))
+        if (streq(property, "X11Variant"))
                 return sd_bus_message_append_basic(reply, 's', c->x11_variant);
-        else if (streq(property, "X11Options"))
+        if (streq(property, "X11Options"))
                 return sd_bus_message_append_basic(reply, 's', c->x11_options);
 
         return -EINVAL;
@@ -243,9 +249,9 @@ static int process_locale_list_item(
         return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Locale assignment %s not valid, refusing.", assignment);
 }
 
-static int locale_gen_process_locale(char *new_locale[static _VARIABLE_LC_MAX],
-                                     sd_bus_error *error) {
+static int locale_gen_process_locale(char *new_locale[static _VARIABLE_LC_MAX], sd_bus_error *error) {
         int r;
+
         assert(new_locale);
 
         for (LocaleVariable p = 0; p < _VARIABLE_LC_MAX; p++) {
@@ -263,13 +269,15 @@ static int locale_gen_process_locale(char *new_locale[static _VARIABLE_LC_MAX],
                                                  SD_BUS_ERROR_INVALID_ARGS,
                                                  "Specified locale is not installed and non-UTF-8 locale will not be auto-generated: %s",
                                                  new_locale[p]);
-                } else if (r == -EINVAL) {
+                }
+                if (r == -EINVAL) {
                         log_error_errno(r, "Failed to enable invalid locale %s for generation.", new_locale[p]);
                         return sd_bus_error_setf(error,
                                                  SD_BUS_ERROR_INVALID_ARGS,
                                                  "Can not enable locale generation for invalid locale: %s",
                                                  new_locale[p]);
-                } else if (r < 0) {
+                }
+                if (r < 0) {
                         log_error_errno(r, "Failed to enable locale for generation: %m");
                         return sd_bus_error_set_errnof(error, r, "Failed to enable locale generation: %m");
                 }