[systemd-commits] 8 commits - man/systemd-bus-proxyd.xml src/core src/locale src/shared TODO

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Wed Sep 3 20:17:05 PDT 2014


 TODO                       |    5 
 man/systemd-bus-proxyd.xml |    2 
 src/core/dbus-manager.c    |    8 -
 src/locale/localed.c       |  253 ++++++++++++++++++++++++---------------------
 src/shared/util.c          |   18 ---
 src/shared/util.h          |    2 
 6 files changed, 145 insertions(+), 143 deletions(-)

New commits:
commit 83a1ff25e5228b0a5b2cc942fd4f964d10bb73b0
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Wed Sep 3 22:12:51 2014 -0400

    Update TODO

diff --git a/TODO b/TODO
index 208a4ce..f0f2bcb 100644
--- a/TODO
+++ b/TODO
@@ -19,6 +19,11 @@ External:
 
 * Fedora: remove /etc/resolv.conf tmpfiles hack
 
+* wiki: update journal format documentation for lz4 additions
+
+* When lz4 gets an API for lz4 command output, make use of it to
+  compress coredumps in a way compatible with /usr/bin/lz4.
+
 Features:
 
 * introduce machines.target to order after all nspawn instances

commit 7a465961c19036eba2c08223e769ad9e85a766a8
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Wed Sep 3 22:07:29 2014 -0400

    man: fix typo

diff --git a/man/systemd-bus-proxyd.xml b/man/systemd-bus-proxyd.xml
index 91a6fe3..f9400f0 100644
--- a/man/systemd-bus-proxyd.xml
+++ b/man/systemd-bus-proxyd.xml
@@ -80,7 +80,7 @@ along with systemd; If not, see <http://www.gnu.org/licenses/>.
 
         <listitem>
           <para>Connect to the bus specified by
-          <replaceable>ADDRESS</replaceable>. Multiple colon-seperated
+          <replaceable>ADDRESS</replaceable>. Multiple colon-separated
           addresses can be specified, in which case
           <command>systemd-bus-proxyd</command> will attempt to
           connect to them in turn.</para>

commit eb6c7d20756b60a7c79a373fd27a682a31b5647a
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Wed Sep 3 22:22:38 2014 -0400

    systemd: fix argument ordering in UnsetAndSetEnvironment
    
    Fixup for v208-615-g718db96199.

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index e792fe7..2fe9d19 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1357,18 +1357,18 @@ static int method_unset_and_set_environment(sd_bus *bus, sd_bus_message *message
         if (r < 0)
                 return r;
 
-        r = sd_bus_message_read_strv(message, &plus);
+        r = sd_bus_message_read_strv(message, &minus);
         if (r < 0)
                 return r;
 
-        r = sd_bus_message_read_strv(message, &minus);
+        r = sd_bus_message_read_strv(message, &plus);
         if (r < 0)
                 return r;
 
-        if (!strv_env_is_valid(plus))
-                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid environment assignments");
         if (!strv_env_name_or_assignment_is_valid(minus))
                 return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid environment variable names or assignments");
+        if (!strv_env_is_valid(plus))
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid environment assignments");
 
         r = manager_environment_add(m, minus, plus);
         if (r < 0)

commit 78bd12a05a9252cf573da28394b23e2b891cbba8
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Wed Sep 3 22:55:54 2014 -0400

    localed: check for partially matching converted keymaps
    
    If a user specifies multiple X11 keymaps, with a (at least the first
    one) nonempty variant, and we don't match the whole combo, use
    a converted keymap which includes the variant in preference to
    the default, variantless, keymap.
    
    E.g.: We would convert X11 config "layout=fr variant=mac" to "fr-mac",
    but "layout=fr,us variant=mac," to "fr", because we don't have a
    converted keymap which would match "fr,us", and we don't have a legacy
    mapping for "fr,us". This is unexpected, and if we cannot match both,
    it is still better to match the primary mapping and use "fr-mac".

diff --git a/src/locale/localed.c b/src/locale/localed.c
index f3e1589..2252080 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -606,8 +606,10 @@ static int vconsole_convert_to_x11(Context *c, sd_bus *bus) {
                 int r;
 
                 r = write_data_x11(c);
-                if (r < 0)
+                if (r < 0) {
                         log_error("Failed to set X11 keyboard layout: %s", strerror(-r));
+                        return r;
+                }
 
                 log_info("Changed X11 keyboard layout to '%s' model '%s' variant '%s' options '%s'",
                          strempty(c->x11_layout),
@@ -625,14 +627,14 @@ static int vconsole_convert_to_x11(Context *c, sd_bus *bus) {
         return 0;
 }
 
-static int find_converted_keymap(Context *c, char **new_keymap) {
+static int find_converted_keymap(const char *x11_layout, const char *x11_variant, char **new_keymap) {
         const char *dir;
         _cleanup_free_ char *n;
 
-        if (c->x11_variant)
-                n = strjoin(c->x11_layout, "-", c->x11_variant, NULL);
+        if (x11_variant)
+                n = strjoin(x11_layout, "-", x11_variant, NULL);
         else
-                n = strdup(c->x11_layout);
+                n = strdup(x11_layout);
         if (!n)
                 return -ENOMEM;
 
@@ -663,7 +665,7 @@ static int find_legacy_keymap(Context *c, char **new_keymap) {
         _cleanup_fclose_ FILE *f;
         unsigned n = 0;
         unsigned best_matching = 0;
-
+        int r;
 
         f = fopen(SYSTEMD_KBD_MODEL_MAP, "re");
         if (!f)
@@ -672,7 +674,6 @@ static int find_legacy_keymap(Context *c, char **new_keymap) {
         for (;;) {
                 _cleanup_strv_free_ char **a = NULL;
                 unsigned matching = 0;
-                int r;
 
                 r = read_next_mapping(f, &n, &a);
                 if (r < 0)
@@ -729,6 +730,23 @@ static int find_legacy_keymap(Context *c, char **new_keymap) {
                 }
         }
 
+        if (best_matching < 10 && c->x11_layout) {
+                /* The best match is only the first part of the X11
+                 * keymap. Check if we have a converted map which
+                 * matches just the first layout.
+                 */
+                char *l, *v = NULL, *converted;
+
+                l = strndupa(c->x11_layout, strcspn(c->x11_layout, ","));
+                if (c->x11_variant)
+                        v = strndupa(c->x11_variant, strcspn(c->x11_variant, ","));
+                r = find_converted_keymap(l, v, &converted);
+                if (r < 0)
+                        return r;
+                if (r > 0)
+                        free_and_replace(new_keymap, converted);
+        }
+
         return 0;
 }
 
@@ -748,7 +766,7 @@ static int x11_convert_to_vconsole(Context *c, sd_bus *bus) {
         } else {
                 char *new_keymap = NULL;
 
-                r = find_converted_keymap(c, &new_keymap);
+                r = find_converted_keymap(c->x11_layout, c->x11_variant, &new_keymap);
                 if (r < 0)
                         return r;
                 else if (r == 0) {

commit 81fd105a5f9762fa2f2e42bc949876e32b3a126f
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Wed Sep 3 22:55:52 2014 -0400

    localed: introduce helper function to simplify matching

diff --git a/src/locale/localed.c b/src/locale/localed.c
index 777da62..f3e1589 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -101,6 +101,12 @@ static void free_and_replace(char **s, char *v) {
         *s = v;
 }
 
+static bool startswith_comma(const char *s, const char *prefix) {
+        const char *t;
+
+        return s && (t = startswith(s, prefix)) && (*t == ',');
+}
+
 static void context_free_x11(Context *c) {
         free_and_replace(&c->x11_layout, NULL);
         free_and_replace(&c->x11_model, NULL);
@@ -679,26 +685,18 @@ static int find_legacy_keymap(Context *c, char **new_keymap) {
                         /* If we got an exact match, this is best */
                         matching = 10;
                 else {
-                        size_t x;
-
-                        x = strcspn(c->x11_layout, ",");
-
                         /* We have multiple X layouts, look for an
                          * entry that matches our key with everything
                          * but the first layout stripped off. */
-                        if (x > 0 &&
-                            strlen(a[1]) == x &&
-                            strneq(c->x11_layout, a[1], x))
+                        if (startswith_comma(c->x11_layout, a[1]))
                                 matching = 5;
                         else  {
-                                size_t w;
+                                char *x;
 
                                 /* If that didn't work, strip off the
                                  * other layouts from the entry, too */
-                                w = strcspn(a[1], ",");
-
-                                if (x > 0 && x == w &&
-                                    memcmp(c->x11_layout, a[1], x) == 0)
+                                x = strndupa(a[1], strcspn(a[1], ","));
+                                if (startswith_comma(c->x11_layout, x))
                                         matching = 1;
                         }
                 }

commit 502f961425f9dea1a85239766a3189695194da63
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Wed Sep 3 22:55:16 2014 -0400

    localed: log locale/keymap changes in detail
    
    Converting X11 to legacy keymaps and back is a fucking mess. Let's
    make it at least possible to request detailed logs of what is being
    changed and why (LOG_DEBUG level).
    
    At LOG_INFO level, we would log the requested change of X11 or console
    keymap, but not the resulting change after conversion to console or X11.
    Make sure that every change of configuration on disk has a matching
    line in the logs.

diff --git a/src/locale/localed.c b/src/locale/localed.c
index ac8477a..777da62 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -265,10 +265,12 @@ static int context_read_data(Context *c) {
         return r < 0 ? r : q < 0 ? q : p;
 }
 
-static int locale_write_data(Context *c) {
+static int locale_write_data(Context *c, char ***settings) {
         int r, p;
         _cleanup_strv_free_ char **l = NULL;
 
+        /* Set values will be returned as strv in *settings on success. */
+
         r = load_env_file(NULL, "/etc/locale.conf", NULL, &l);
         if (r < 0 && r != -ENOENT)
                 return r;
@@ -302,7 +304,13 @@ static int locale_write_data(Context *c) {
                 return 0;
         }
 
-        return write_env_file_label("/etc/locale.conf", l);
+        r = write_env_file_label("/etc/locale.conf", l);
+        if (r < 0)
+                return r;
+
+        *settings = l;
+        l = NULL;
+        return 0;
 }
 
 static int locale_update_system_manager(Context *c, sd_bus *bus) {
@@ -595,11 +603,18 @@ static int vconsole_convert_to_x11(Context *c, sd_bus *bus) {
                 if (r < 0)
                         log_error("Failed to set X11 keyboard layout: %s", strerror(-r));
 
+                log_info("Changed X11 keyboard layout to '%s' model '%s' variant '%s' options '%s'",
+                         strempty(c->x11_layout),
+                         strempty(c->x11_model),
+                         strempty(c->x11_variant),
+                         strempty(c->x11_options));
+
                 sd_bus_emit_properties_changed(bus,
                                 "/org/freedesktop/locale1",
                                 "org.freedesktop.locale1",
                                 "X11Layout", "X11Model", "X11Variant", "X11Options", NULL);
-        }
+        } else
+                log_debug("X11 keyboard layout was not modified.");
 
         return 0;
 }
@@ -617,13 +632,18 @@ static int find_converted_keymap(Context *c, char **new_keymap) {
 
         NULSTR_FOREACH(dir, KBD_KEYMAP_DIRS) {
                 _cleanup_free_ char *p = NULL, *pz = NULL;
+                bool uncompressed;
 
                 p = strjoin(dir, "xkb/", n, ".map", NULL);
                 pz = strjoin(dir, "xkb/", n, ".map.gz", NULL);
                 if (!p || !pz)
                         return -ENOMEM;
 
-                if (access(p, F_OK) == 0 || access(pz, F_OK) == 0) {
+                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 = n;
                         n = NULL;
                         return 1;
@@ -697,12 +717,17 @@ static int find_legacy_keymap(Context *c, char **new_keymap) {
                 }
 
                 /* The best matching entry so far, then let's save that */
-                if (matching > best_matching) {
-                        best_matching = matching;
+                if (matching >= MAX(best_matching, 1u)) {
+                        log_debug("Found legacy keymap %s with score %u",
+                                  a[0], matching);
 
-                        r = free_and_strdup(new_keymap, a[0]);
-                        if (r < 0)
-                                return r;
+                        if (matching > best_matching) {
+                                best_matching = matching;
+
+                                r = free_and_strdup(new_keymap, a[0]);
+                                if (r < 0)
+                                        return r;
+                        }
                 }
         }
 
@@ -747,13 +772,17 @@ static int x11_convert_to_vconsole(Context *c, sd_bus *bus) {
                 if (r < 0)
                         log_error("Failed to set virtual console keymap: %s", strerror(-r));
 
+                log_info("Changed virtual console keymap to '%s' toggle '%s'",
+                         strempty(c->vc_keymap), strempty(c->vc_keymap_toggle));
+
                 sd_bus_emit_properties_changed(bus,
                                 "/org/freedesktop/locale1",
                                 "org.freedesktop.locale1",
                                 "VConsoleKeymap", "VConsoleKeymapToggle", NULL);
 
                 return vconsole_reload(bus);
-        }
+        } else
+                log_debug("Virtual console keymap was not modified.");
 
         return 0;
 }
@@ -808,7 +837,7 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_
         if (r < 0)
                 return r;
 
-        /* Check whether a variable changed and if so valid */
+        /* Check whether a variable changed and if it is valid */
         STRV_FOREACH(i, l) {
                 bool valid = false;
 
@@ -842,6 +871,8 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_
                         }
 
         if (modified) {
+                _cleanup_strv_free_ char **settings = NULL;
+
                 r = bus_verify_polkit_async(m, CAP_SYS_ADMIN, "org.freedesktop.locale1.set-locale", interactive, &c->polkit_registry, error);
                 if (r < 0)
                         return r;
@@ -870,7 +901,7 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_
 
                 locale_simplify(c);
 
-                r = locale_write_data(c);
+                r = locale_write_data(c, &settings);
                 if (r < 0) {
                         log_error("Failed to set locale: %s", strerror(-r));
                         return sd_bus_error_set_errnof(error, r, "Failed to set locale: %s", strerror(-r));
@@ -878,13 +909,21 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_
 
                 locale_update_system_manager(c, bus);
 
-                log_info("Changed locale information.");
+                if (settings) {
+                        _cleanup_free_ char *line;
+
+                        line = strv_join(settings, ", ");
+                        log_info("Changed locale to %s.", strnull(line));
+                } else
+                        log_info("Changed locale to unset.");
 
                 sd_bus_emit_properties_changed(bus,
                                 "/org/freedesktop/locale1",
                                 "org.freedesktop.locale1",
                                 "Locale", NULL);
-        }
+        } else
+                log_debug("Locale settings were not modified.");
+
 
         return sd_bus_reply_method_return(m, NULL);
 }
@@ -928,7 +967,8 @@ static int method_set_vc_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata
                         return sd_bus_error_set_errnof(error, r, "Failed to set virtual console keymap: %s", strerror(-r));
                 }
 
-                log_info("Changed virtual console keymap to '%s'", strempty(c->vc_keymap));
+                log_info("Changed virtual console keymap to '%s' toggle '%s'",
+                         strempty(c->vc_keymap), strempty(c->vc_keymap_toggle));
 
                 r = vconsole_reload(bus);
                 if (r < 0)
@@ -1000,7 +1040,11 @@ static int method_set_x11_keyboard(sd_bus *bus, sd_bus_message *m, void *userdat
                         return sd_bus_error_set_errnof(error, r, "Failed to set X11 keyboard layout: %s", strerror(-r));
                 }
 
-                log_info("Changed X11 keyboard layout to '%s'", strempty(c->x11_layout));
+                log_info("Changed X11 keyboard layout to '%s' model '%s' variant '%s' options '%s'",
+                         strempty(c->x11_layout),
+                         strempty(c->x11_model),
+                         strempty(c->x11_variant),
+                         strempty(c->x11_options));
 
                 sd_bus_emit_properties_changed(bus,
                                 "/org/freedesktop/locale1",

commit af76d302c1e26f916494202f1b3663f15710bdcd
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Wed Sep 3 22:53:23 2014 -0400

    localed: remove free_and_copy
    
    It was mostly a duplicate of free_and_strdup().

diff --git a/src/locale/localed.c b/src/locale/localed.c
index c9f7105..ac8477a 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -92,20 +92,8 @@ typedef struct Context {
         Hashmap *polkit_registry;
 } Context;
 
-static int free_and_copy(char **s, const char *v) {
-        int r;
-        char *t;
-
-        assert(s);
-
-        r = strdup_or_null(isempty(v) ? NULL : v, &t);
-        if (r < 0)
-                return r;
-
-        free(*s);
-        *s = t;
-
-        return 0;
+static const char* nonempty(const char *s) {
+        return isempty(s) ? NULL : s;
 }
 
 static void free_and_replace(char **s, char *v) {
@@ -144,10 +132,8 @@ static void locale_simplify(Context *c) {
         int p;
 
         for (p = LOCALE_LANG+1; p < _LOCALE_MAX; p++)
-                if (isempty(c->locale[p]) || streq_ptr(c->locale[LOCALE_LANG], c->locale[p])) {
-                        free(c->locale[p]);
-                        c->locale[p] = NULL;
-                }
+                if (isempty(c->locale[p]) || streq_ptr(c->locale[LOCALE_LANG], c->locale[p]))
+                        free_and_replace(&c->locale[p], NULL);
 }
 
 static int locale_read_data(Context *c) {
@@ -179,7 +165,8 @@ static int locale_read_data(Context *c) {
                 for (p = 0; p < _LOCALE_MAX; p++) {
                         assert(names[p]);
 
-                        r = free_and_copy(&c->locale[p], getenv(names[p]));
+                        r = free_and_strdup(&c->locale[p],
+                                            nonempty(getenv(names[p])));
                         if (r < 0)
                                 return r;
                 }
@@ -503,8 +490,8 @@ static int vconsole_reload(sd_bus *bus) {
         return r;
 }
 
-static char *strnulldash(const char *s) {
-        return s == NULL || *s == 0 || (s[0] == '-' && s[1] == 0) ? NULL : (char*) s;
+static const char* strnulldash(const char *s) {
+        return isempty(s) || streq(s, "-") ? NULL : s;
 }
 
 static int read_next_mapping(FILE *f, unsigned *n, char ***a) {
@@ -588,10 +575,10 @@ static int vconsole_convert_to_x11(Context *c, sd_bus *bus) {
                             !streq_ptr(c->x11_variant, strnulldash(a[3])) ||
                             !streq_ptr(c->x11_options, strnulldash(a[4]))) {
 
-                                if (free_and_copy(&c->x11_layout, strnulldash(a[1])) < 0 ||
-                                    free_and_copy(&c->x11_model, strnulldash(a[2])) < 0 ||
-                                    free_and_copy(&c->x11_variant, strnulldash(a[3])) < 0 ||
-                                    free_and_copy(&c->x11_options, strnulldash(a[4])) < 0)
+                                if (free_and_strdup(&c->x11_layout, strnulldash(a[1])) < 0 ||
+                                    free_and_strdup(&c->x11_model, strnulldash(a[2])) < 0 ||
+                                    free_and_strdup(&c->x11_variant, strnulldash(a[3])) < 0 ||
+                                    free_and_strdup(&c->x11_options, strnulldash(a[4])) < 0)
                                         return -ENOMEM;
 
                                 modified = true;
@@ -713,10 +700,9 @@ static int find_legacy_keymap(Context *c, char **new_keymap) {
                 if (matching > best_matching) {
                         best_matching = matching;
 
-                        free(*new_keymap);
-                        *new_keymap = strdup(a[0]);
-                        if (!*new_keymap)
-                                return -ENOMEM;
+                        r = free_and_strdup(new_keymap, a[0]);
+                        if (r < 0)
+                                return r;
                 }
         }
 
@@ -868,14 +854,9 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_
 
                                 k = strlen(names[p]);
                                 if (startswith(*i, names[p]) && (*i)[k] == '=') {
-                                        char *t;
-
-                                        t = strdup(*i + k + 1);
-                                        if (!t)
-                                                return -ENOMEM;
-
-                                        free(c->locale[p]);
-                                        c->locale[p] = t;
+                                        r = free_and_strdup(&c->locale[p], *i + k + 1);
+                                        if (r < 0)
+                                                return r;
                                         break;
                                 }
                         }
@@ -937,8 +918,8 @@ static int method_set_vc_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata
                 if (r == 0)
                         return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
-                if (free_and_copy(&c->vc_keymap, keymap) < 0 ||
-                    free_and_copy(&c->vc_keymap_toggle, keymap_toggle) < 0)
+                if (free_and_strdup(&c->vc_keymap, keymap) < 0 ||
+                    free_and_strdup(&c->vc_keymap_toggle, keymap_toggle) < 0)
                         return -ENOMEM;
 
                 r = vconsole_write_data(c);
@@ -1007,10 +988,10 @@ static int method_set_x11_keyboard(sd_bus *bus, sd_bus_message *m, void *userdat
                 if (r == 0)
                         return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
-                if (free_and_copy(&c->x11_layout, layout) < 0 ||
-                    free_and_copy(&c->x11_model, model) < 0 ||
-                    free_and_copy(&c->x11_variant, variant) < 0 ||
-                    free_and_copy(&c->x11_options, options) < 0)
+                if (free_and_strdup(&c->x11_layout, layout) < 0 ||
+                    free_and_strdup(&c->x11_model, model) < 0 ||
+                    free_and_strdup(&c->x11_variant, variant) < 0 ||
+                    free_and_strdup(&c->x11_options, options) < 0)
                         return -ENOMEM;
 
                 r = write_data_x11(c);
diff --git a/src/shared/util.c b/src/shared/util.c
index cf9d487..502b367 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -4981,24 +4981,6 @@ bool kexec_loaded(void) {
        return loaded;
 }
 
-int strdup_or_null(const char *a, char **b) {
-        char *c;
-
-        assert(b);
-
-        if (!a) {
-                *b = NULL;
-                return 0;
-        }
-
-        c = strdup(a);
-        if (!c)
-                return -ENOMEM;
-
-        *b = c;
-        return 0;
-}
-
 int prot_from_flags(int flags) {
 
         switch (flags & O_ACCMODE) {
diff --git a/src/shared/util.h b/src/shared/util.h
index 3401280..08d556f 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -583,8 +583,6 @@ int block_get_whole_disk(dev_t d, dev_t *ret);
 
 int file_is_priv_sticky(const char *p);
 
-int strdup_or_null(const char *a, char **b);
-
 #define NULSTR_FOREACH(i, l)                                    \
         for ((i) = (l); (i) && *(i); (i) = strchr((i), 0)+1)
 

commit 28efac0d37ceb5093a804da6a00c620034c5484f
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Wed Sep 3 10:28:24 2014 -0400

    localed: double free in error path and modernization
    
    Very unlikely to trigger, but in principle strv_free
    could be called twice: once explictly, and once from cleanup.

diff --git a/src/locale/localed.c b/src/locale/localed.c
index 4d22568..c9f7105 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -208,7 +208,7 @@ static int vconsole_read_data(Context *c) {
 }
 
 static int x11_read_data(Context *c) {
-        FILE *f;
+        _cleanup_fclose_ FILE *f;
         char line[LINE_MAX];
         bool in_section = false;
         int r;
@@ -229,13 +229,11 @@ static int x11_read_data(Context *c) {
                         continue;
 
                 if (in_section && first_word(l, "Option")) {
-                        char **a;
+                        _cleanup_strv_free_ char **a = NULL;
 
                         r = strv_split_quoted(&a, l);
-                        if (r < 0) {
-                                fclose(f);
+                        if (r < 0)
                                 return r;
-                        }
 
                         if (strv_length(a) == 3) {
                                 if (streq(a[1], "XkbLayout")) {
@@ -253,27 +251,20 @@ static int x11_read_data(Context *c) {
                                 }
                         }
 
-                        strv_free(a);
-
                 } else if (!in_section && first_word(l, "Section")) {
-                        char **a;
+                        _cleanup_strv_free_ char **a = NULL;
 
                         r = strv_split_quoted(&a, l);
-                        if (r < 0) {
-                                fclose(f);
+                        if (r < 0)
                                 return -ENOMEM;
-                        }
 
                         if (strv_length(a) == 2 && streq(a[1], "InputClass"))
                                 in_section = true;
 
-                        strv_free(a);
                 } else if (in_section && first_word(l, "EndSection"))
                         in_section = false;
         }
 
-        fclose(f);
-
         return 0;
 }
 
@@ -289,14 +280,15 @@ static int context_read_data(Context *c) {
 
 static int locale_write_data(Context *c) {
         int r, p;
-        char **l = NULL;
+        _cleanup_strv_free_ char **l = NULL;
 
         r = load_env_file(NULL, "/etc/locale.conf", NULL, &l);
         if (r < 0 && r != -ENOENT)
                 return r;
 
         for (p = 0; p < _LOCALE_MAX; p++) {
-                char *t, **u;
+                _cleanup_free_ char *t = NULL;
+                char **u;
 
                 assert(names[p]);
 
@@ -305,34 +297,25 @@ static int locale_write_data(Context *c) {
                         continue;
                 }
 
-                if (asprintf(&t, "%s=%s", names[p], c->locale[p]) < 0) {
-                        strv_free(l);
+                if (asprintf(&t, "%s=%s", names[p], c->locale[p]) < 0)
                         return -ENOMEM;
-                }
 
                 u = strv_env_set(l, t);
-                free(t);
-                strv_free(l);
-
                 if (!u)
                         return -ENOMEM;
 
+                strv_free(l);
                 l = u;
         }
 
         if (strv_isempty(l)) {
-                strv_free(l);
-
                 if (unlink("/etc/locale.conf") < 0)
                         return errno == ENOENT ? 0 : -errno;
 
                 return 0;
         }
 
-        r = write_env_file_label("/etc/locale.conf", l);
-        strv_free(l);
-
-        return r;
+        return write_env_file_label("/etc/locale.conf", l);
 }
 
 static int locale_update_system_manager(Context *c, sd_bus *bus) {
@@ -403,38 +386,36 @@ static int vconsole_write_data(Context *c) {
         if (isempty(c->vc_keymap))
                 l = strv_env_unset(l, "KEYMAP");
         else {
-                char *s, **u;
+                _cleanup_free_ char *s = NULL;
+                char **u;
 
                 s = strappend("KEYMAP=", c->vc_keymap);
                 if (!s)
                         return -ENOMEM;
 
                 u = strv_env_set(l, s);
-                free(s);
-                strv_free(l);
-
                 if (!u)
                         return -ENOMEM;
 
+                strv_free(l);
                 l = u;
         }
 
         if (isempty(c->vc_keymap_toggle))
                 l = strv_env_unset(l, "KEYMAP_TOGGLE");
         else  {
-                char *s, **u;
+                _cleanup_free_ char *s = NULL;
+                char **u;
 
                 s = strappend("KEYMAP_TOGGLE=", c->vc_keymap_toggle);
                 if (!s)
                         return -ENOMEM;
 
                 u = strv_env_set(l, s);
-                free(s);
-                strv_free(l);
-
                 if (!u)
                         return -ENOMEM;
 
+                strv_free(l);
                 l = u;
         }
 
@@ -445,8 +426,7 @@ static int vconsole_write_data(Context *c) {
                 return 0;
         }
 
-        r = write_env_file_label("/etc/vconsole.conf", l);
-        return r;
+        return write_env_file_label("/etc/vconsole.conf", l);
 }
 
 static int write_data_x11(Context *c) {
@@ -868,13 +848,12 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_
         }
 
         /* Check whether a variable is unset */
-        if (!modified)  {
+        if (!modified)
                 for (p = 0; p < _LOCALE_MAX; p++)
                         if (!isempty(c->locale[p]) && !passed[p]) {
                                 modified = true;
                                 break;
                         }
-        }
 
         if (modified) {
                 r = bus_verify_polkit_async(m, CAP_SYS_ADMIN, "org.freedesktop.locale1.set-locale", interactive, &c->polkit_registry, error);
@@ -883,7 +862,7 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_
                 if (r == 0)
                         return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
-                STRV_FOREACH(i, l) {
+                STRV_FOREACH(i, l)
                         for (p = 0; p < _LOCALE_MAX; p++) {
                                 size_t k;
 
@@ -900,7 +879,6 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_
                                         break;
                                 }
                         }
-                }
 
                 for (p = 0; p < _LOCALE_MAX; p++) {
                         if (passed[p])
@@ -1112,7 +1090,7 @@ static int connect_bus(Context *c, sd_event *event, sd_bus **_bus) {
 }
 
 int main(int argc, char *argv[]) {
-        Context context = {};
+        _cleanup_(context_free) Context context = {};
         _cleanup_event_unref_ sd_event *event = NULL;
         _cleanup_bus_close_unref_ sd_bus *bus = NULL;
         int r;
@@ -1155,7 +1133,5 @@ int main(int argc, char *argv[]) {
         }
 
 finish:
-        context_free(&context);
-
         return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
 }



More information about the systemd-commits mailing list