locale: make vconsole_convert_to_x11() not update Context

This also makes x11_convert_to_vconsole() changed in the same way.
Then, their callers update Context if necessary.

No functional change, just preparation for later commits.

diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c
index 9dccaf9d2b47f0134f872d0b18560fb07d3e8973..3c95885e28e2595910f3fc6c87f4484c1b46978b 100644 (file)
--- a/src/locale/localed-util.c
+++ b/src/locale/localed-util.c
@@ -56,7 +56,7 @@ static const char* systemd_language_fallback_map(void) {
         return SYSTEMD_LANGUAGE_FALLBACK_MAP;
 }
 
-static void x11_context_clear(X11Context *xc) {
+void x11_context_clear(X11Context *xc) {
         assert(xc);
 
         xc->layout  = mfree(xc->layout);
@@ -65,7 +65,16 @@ static void x11_context_clear(X11Context *xc) {
         xc->variant = mfree(xc->variant);
 }
 
-static bool x11_context_isempty(const X11Context *xc) {
+void x11_context_replace(X11Context *dest, X11Context *src) {
+        assert(dest);
+        assert(src);
+
+        x11_context_clear(dest);
+        *dest = *src;
+        *src = (X11Context) {};
+}
+
+bool x11_context_isempty(const X11Context *xc) {
         assert(xc);
 
         return
@@ -152,14 +161,23 @@ static void context_clear_x11(Context *c) {
         x11_context_clear(&c->x11_from_vc);
 }
 
-static void vc_context_clear(VCContext *vc) {
+void vc_context_clear(VCContext *vc) {
         assert(vc);
 
         vc->keymap = mfree(vc->keymap);
         vc->toggle = mfree(vc->toggle);
 }
 
-static bool vc_context_isempty(const VCContext *vc) {
+void vc_context_replace(VCContext *dest, VCContext *src) {
+        assert(dest);
+        assert(src);
+
+        vc_context_clear(dest);
+        *dest = *src;
+        *src = (VCContext) {};
+}
+
+bool vc_context_isempty(const VCContext *vc) {
         assert(vc);
 
         return
@@ -584,70 +602,53 @@ static int read_next_mapping(
         return 0;
 }
 
-int vconsole_convert_to_x11(Context *c) {
-        int r, modified = -1;
-        X11Context *xc;
-
-        assert(c);
+int vconsole_convert_to_x11(const VCContext *vc, X11Context *ret) {
+        _cleanup_fclose_ FILE *f = NULL;
+        const char *map;
+        int r;
 
-        /* If Context.x11_from_vc is empty, then here we update Context.x11_from_xorg, as the caller may
-         * already have been called context_get_x11_context() or _safe(), and otherwise the caller's
-         * X11Context may be outdated. The updated context will be copied to Context.x11_from_vc in
-         * vconsole_write_data() if necessary. */
-        xc = context_get_x11_context_safe(c);
+        assert(vc);
+        assert(ret);
 
-        if (isempty(c->vc.keymap)) {
-                modified = !x11_context_isempty(xc);
-                context_clear_x11(c);
-        } else {
-                _cleanup_fclose_ FILE *f = NULL;
-                const char *map;
+        if (isempty(vc->keymap)) {
+                *ret = (X11Context) {};
+                return 0;
+        }
 
-                map = systemd_kbd_model_map();
-                f = fopen(map, "re");
-                if (!f)
-                        return -errno;
+        map = systemd_kbd_model_map();
+        f = fopen(map, "re");
+        if (!f)
+                return -errno;
 
-                for (unsigned n = 0;;) {
-                        _cleanup_strv_free_ char **a = NULL;
+        for (unsigned n = 0;;) {
+                _cleanup_strv_free_ char **a = NULL;
 
-                        r = read_next_mapping(map, 5, UINT_MAX, f, &n, &a);
-                        if (r < 0)
-                                return r;
-                        if (r == 0)
-                                break;
+                r = read_next_mapping(map, 5, UINT_MAX, f, &n, &a);
+                if (r < 0)
+                        return r;
+                if (r == 0) {
+                        log_notice("No conversion found for virtual console keymap \"%s\".", vc->keymap);
+                        *ret = (X11Context) {};
+                        return 0;
+                }
 
-                        if (!streq(c->vc.keymap, a[0]))
-                                continue;
+                if (!streq(vc->keymap, a[0]))
+                        continue;
 
-                        r = x11_context_copy(xc,
-                                             &(X11Context) {
-                                                     .layout  = empty_or_dash_to_null(a[1]),
-                                                     .model   = empty_or_dash_to_null(a[2]),
-                                                     .variant = empty_or_dash_to_null(a[3]),
-                                                     .options = empty_or_dash_to_null(a[4]),
-                                             });
-                        if (r < 0)
-                                return r;
-                        modified = r > 0;
+                r = x11_context_copy(ret,
+                                     &(X11Context) {
+                                             .layout  = empty_or_dash_to_null(a[1]),
+                                             .model   = empty_or_dash_to_null(a[2]),
+                                             .variant = empty_or_dash_to_null(a[3]),
+                                             .options = empty_or_dash_to_null(a[4]),
+                                     });
+                if (r < 0)
+                        return r;
 
-                        break;
-                }
+                log_info("The virtual console keymap '%s' is converted to X11 keyboard layout '%s' model '%s' variant '%s' options '%s'",
+                         vc->keymap, strempty(ret->layout), strempty(ret->model), strempty(ret->variant), strempty(ret->options));
+                return 0;
         }
-
-        if (modified > 0)
-                log_info("Changing X11 keyboard layout to '%s' model '%s' variant '%s' options '%s'",
-                         strempty(xc->layout),
-                         strempty(xc->model),
-                         strempty(xc->variant),
-                         strempty(xc->options));
-        else if (modified < 0)
-                log_notice("X11 keyboard layout was not modified: no conversion found for \"%s\".",
-                           c->vc.keymap);
-        else
-                log_debug("X11 keyboard layout did not need to be modified.");
-
-        return modified > 0;
 }
 
 int find_converted_keymap(const X11Context *xc, char **ret) {
@@ -823,46 +824,36 @@ int find_language_fallback(const char *lang, char **ret) {
         }
 }
 
-int x11_convert_to_vconsole(Context *c) {
-        bool modified = false;
-        const X11Context *xc;
-
-        assert(c);
+int x11_convert_to_vconsole(const X11Context *xc, VCContext *ret) {
+        _cleanup_free_ char *keymap = NULL;
+        int r;
 
-        xc = context_get_x11_context_safe(c);
+        assert(xc);
+        assert(ret);
 
         if (isempty(xc->layout)) {
-                modified = !vc_context_isempty(&c->vc);
-                vc_context_clear(&c->vc);
-        } else {
-                _cleanup_free_ char *new_keymap = NULL;
-                int r;
-
-                r = find_converted_keymap(xc, &new_keymap);
-                if (r == 0)
-                        r = find_legacy_keymap(xc, &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\".", xc->layout);
-
-                if (!streq_ptr(c->vc.keymap, new_keymap)) {
-                        vc_context_clear(&c->vc);
-                        c->vc.keymap = TAKE_PTR(new_keymap);
-                        modified = true;
-                }
+                *ret = (VCContext) {};
+                return 0;
         }
 
-        if (modified)
-                log_info("Changing virtual console keymap to '%s' toggle '%s'",
-                         strempty(c->vc.keymap), strempty(c->vc.toggle));
+        r = find_converted_keymap(xc, &keymap);
+        if (r == 0)
+                r = find_legacy_keymap(xc, &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\".", xc->layout);
         else
-                log_debug("Virtual console keymap was not modified.");
+                log_info("The X11 keyboard layout '%s' is converted to virtual console keymap '%s'",
+                         xc->layout, strempty(keymap));
 
-        return modified;
+        *ret = (VCContext) {
+                .keymap = TAKE_PTR(keymap),
+        };
+        return 0;
 }
 
 bool locale_gen_check_available(void) {

diff --git a/src/locale/localed-util.h b/src/locale/localed-util.h
index 6e4f270f4621e5ca6443c09479ec2c829cc70cc6..015fcf8901786b4d978c5d704d482b4f119f8af9 100644 (file)
--- a/src/locale/localed-util.h
+++ b/src/locale/localed-util.h
@@ -36,6 +36,9 @@ typedef struct Context {
         Hashmap *polkit_registry;
 } Context;
 
+void x11_context_clear(X11Context *xc);
+void x11_context_replace(X11Context *dest, X11Context *src);
+bool x11_context_isempty(const X11Context *xc);
 void x11_context_empty_to_null(X11Context *xc);
 bool x11_context_is_safe(const X11Context *xc);
 bool x11_context_equal(const X11Context *a, const X11Context *b);
@@ -43,6 +46,9 @@ int x11_context_copy(X11Context *dest, const X11Context *src);
 
 X11Context *context_get_x11_context_safe(Context *c);
 
+void vc_context_clear(VCContext *vc);
+void vc_context_replace(VCContext *dest, VCContext *src);
+bool vc_context_isempty(const VCContext *vc);
 void vc_context_empty_to_null(VCContext *vc);
 bool vc_context_equal(const VCContext *a, const VCContext *b);
 int vc_context_copy(VCContext *dest, const VCContext *src);
@@ -56,9 +62,9 @@ int vconsole_read_data(Context *c, sd_bus_message *m);
 int x11_read_data(Context *c, sd_bus_message *m);
 
 void context_clear(Context *c);
-int vconsole_convert_to_x11(Context *c);
+int vconsole_convert_to_x11(const VCContext *vc, X11Context *ret);
 int vconsole_write_data(Context *c);
-int x11_convert_to_vconsole(Context *c);
+int x11_convert_to_vconsole(const X11Context *xc, VCContext *ret);
 int x11_write_data(Context *c);
 
 bool locale_gen_check_available(void);

diff --git a/src/locale/localed.c b/src/locale/localed.c
index 4ba7dc1f7c71710e897f9a5e976c8ba979e825bc..b91ebf79ec70e25db891182aabe368a21b3b2374 100644 (file)
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -416,20 +416,25 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro
                 return log_oom();
 
         if (convert) {
+                _cleanup_(x11_context_clear) X11Context converted = {};
+                X11Context *xc;
+
                 r = x11_read_data(c, m);
                 if (r < 0) {
                         log_error_errno(r, "Failed to read X11 keyboard layout data: %m");
                         return sd_bus_error_set_errnof(error, r, "Failed to read X11 keyboard layout data: %m");
                 }
 
-                r = vconsole_convert_to_x11(c);
+                r = vconsole_convert_to_x11(&in, &converted);
                 if (r < 0) {
                         log_error_errno(r, "Failed to convert keymap data: %m");
                         return sd_bus_error_set_errnof(error, r, "Failed to convert keymap data: %m");
                 }
 
                 /* save the result of conversion to emit changed properties later. */
-                convert = r > 0;
+                xc = context_get_x11_context_safe(c);
+                convert = !x11_context_equal(xc, &converted);
+                x11_context_replace(xc, &converted);
         }
 
         r = vconsole_write_data(c);
@@ -619,14 +624,17 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err
                 return log_oom();
 
         if (convert) {
-                r = x11_convert_to_vconsole(c);
+                _cleanup_(vc_context_clear) VCContext converted = {};
+
+                r = x11_convert_to_vconsole(&in, &converted);
                 if (r < 0) {
                         log_error_errno(r, "Failed to convert keymap data: %m");
                         return sd_bus_error_set_errnof(error, r, "Failed to convert keymap data: %m");
                 }
 
                 /* save the result of conversion to emit changed properties later. */
-                convert = r > 0;
+                convert = !vc_context_equal(&c->vc, &converted);
+                vc_context_replace(&c->vc, &converted);
         }
 
         r = vconsole_write_data(c);

diff --git a/src/locale/test-localed-util.c b/src/locale/test-localed-util.c
index d7089fa6c35ed5aca528a1fae83524871b360d61..f57c9ba1f88d04fe3dc2a494628576af0b522da5 100644 (file)
--- a/src/locale/test-localed-util.c
+++ b/src/locale/test-localed-util.c
@@ -71,125 +71,105 @@ TEST(find_legacy_keymap) {
 }
 
 TEST(vconsole_convert_to_x11) {
-        _cleanup_(context_clear) Context c = {};
-        X11Context *xc = &c.x11_from_vc;
+        _cleanup_(x11_context_clear) X11Context xc = {};
+        _cleanup_(vc_context_clear) VCContext vc = {};
 
-        log_info("/* test emptying first (:) */");
-        assert_se(free_and_strdup(&xc->layout, "foo") >= 0);
-        assert_se(free_and_strdup(&xc->variant, "bar") >= 0);
-        assert_se(vconsole_convert_to_x11(&c) == 1);
-        assert_se(xc->layout == NULL);
-        assert_se(xc->variant == NULL);
-
-        log_info("/* test emptying second (:) */");
-
-        assert_se(vconsole_convert_to_x11(&c) == 0);
-        assert_se(xc->layout == NULL);
-        assert_se(xc->variant == NULL);
+        log_info("/* test empty keymap */");
+        assert_se(vconsole_convert_to_x11(&vc, &xc) >= 0);
+        assert_se(x11_context_isempty(&xc));
 
         log_info("/* test without variant, new mapping (es:) */");
-        assert_se(free_and_strdup(&c.vc.keymap, "es") >= 0);
-
-        assert_se(vconsole_convert_to_x11(&c) == 1);
-        assert_se(streq(xc->layout, "es"));
-        assert_se(xc->variant == NULL);
+        assert_se(free_and_strdup(&vc.keymap, "es") >= 0);
+        assert_se(vconsole_convert_to_x11(&vc, &xc) >= 0);
+        assert_se(streq(xc.layout, "es"));
+        assert_se(xc.variant == NULL);
+        x11_context_clear(&xc);
 
         log_info("/* test with known variant, new mapping (es:dvorak) */");
-        assert_se(free_and_strdup(&c.vc.keymap, "es-dvorak") >= 0);
-
-        assert_se(vconsole_convert_to_x11(&c) == 1);
-        assert_se(streq(xc->layout, "es"));
-        assert_se(streq(xc->variant, "dvorak"));
+        assert_se(free_and_strdup(&vc.keymap, "es-dvorak") >= 0);
+        assert_se(vconsole_convert_to_x11(&vc, &xc) >= 0);
+        assert_se(streq(xc.layout, "es"));
+        assert_se(streq(xc.variant, "dvorak"));
+        x11_context_clear(&xc);
 
         log_info("/* test with old mapping (fr:latin9) */");
-        assert_se(free_and_strdup(&c.vc.keymap, "fr-latin9") >= 0);
-
-        assert_se(vconsole_convert_to_x11(&c) == 1);
-        assert_se(streq(xc->layout, "fr"));
-        assert_se(streq(xc->variant, "latin9"));
+        assert_se(free_and_strdup(&vc.keymap, "fr-latin9") >= 0);
+        assert_se(vconsole_convert_to_x11(&vc, &xc) >= 0);
+        assert_se(streq(xc.layout, "fr"));
+        assert_se(streq(xc.variant, "latin9"));
+        x11_context_clear(&xc);
 
         log_info("/* test with a compound mapping (ru,us) */");
-        assert_se(free_and_strdup(&c.vc.keymap, "ru") >= 0);
-
-        assert_se(vconsole_convert_to_x11(&c) == 1);
-        assert_se(streq(xc->layout, "ru,us"));
-        assert_se(xc->variant == NULL);
+        assert_se(free_and_strdup(&vc.keymap, "ru") >= 0);
+        assert_se(vconsole_convert_to_x11(&vc, &xc) >= 0);
+        assert_se(streq(xc.layout, "ru,us"));
+        assert_se(xc.variant == NULL);
+        x11_context_clear(&xc);
 
         log_info("/* test with a simple mapping (us) */");
-        assert_se(free_and_strdup(&c.vc.keymap, "us") >= 0);
-
-        assert_se(vconsole_convert_to_x11(&c) == 1);
-        assert_se(streq(xc->layout, "us"));
-        assert_se(xc->variant == NULL);
+        assert_se(free_and_strdup(&vc.keymap, "us") >= 0);
+        assert_se(vconsole_convert_to_x11(&vc, &xc) >= 0);
+        assert_se(streq(xc.layout, "us"));
+        assert_se(xc.variant == NULL);
 }
 
 TEST(x11_convert_to_vconsole) {
-        _cleanup_(context_clear) Context c = {};
-        X11Context *xc = &c.x11_from_xorg;
-        int r;
-
-        log_info("/* test emptying first (:) */");
-        assert_se(free_and_strdup(&c.vc.keymap, "foobar") >= 0);
-        assert_se(x11_convert_to_vconsole(&c) == 1);
-        assert_se(c.vc.keymap == NULL);
-
-        log_info("/* test emptying second (:) */");
+        _cleanup_(x11_context_clear) X11Context xc = {};
+        _cleanup_(vc_context_clear) VCContext vc = {};
 
-        assert_se(x11_convert_to_vconsole(&c) == 0);
-        assert_se(c.vc.keymap == NULL);
+        log_info("/* test empty layout (:) */");
+        assert_se(x11_convert_to_vconsole(&xc, &vc) >= 0);
+        assert_se(vc_context_isempty(&vc));
 
         log_info("/* test without variant, new mapping (es:) */");
-        assert_se(free_and_strdup(&xc->layout, "es") >= 0);
-
-        assert_se(x11_convert_to_vconsole(&c) == 1);
-        assert_se(streq(c.vc.keymap, "es"));
+        assert_se(free_and_strdup(&xc.layout, "es") >= 0);
+        assert_se(x11_convert_to_vconsole(&xc, &vc) >= 0);
+        assert_se(streq(vc.keymap, "es"));
+        vc_context_clear(&vc);
 
         log_info("/* test with unknown variant, new mapping (es:foobar) */");
-        assert_se(free_and_strdup(&xc->variant, "foobar") >= 0);
-
-        assert_se(x11_convert_to_vconsole(&c) == 0);
-        assert_se(streq(c.vc.keymap, "es"));
+        assert_se(free_and_strdup(&xc.variant, "foobar") >= 0);
+        assert_se(x11_convert_to_vconsole(&xc, &vc) >= 0);
+        assert_se(streq(vc.keymap, "es"));
+        vc_context_clear(&vc);
 
         log_info("/* test with known variant, new mapping (es:dvorak) */");
-        assert_se(free_and_strdup(&xc->variant, "dvorak") >= 0);
-
-        r = x11_convert_to_vconsole(&c);
-        if (r == 0) {
+        assert_se(free_and_strdup(&xc.variant, "dvorak") >= 0);
+        assert_se(x11_convert_to_vconsole(&xc, &vc) >= 0);
+        if (vc_context_isempty(&vc)) {
                 log_info("Skipping rest of %s: keymaps are not installed", __func__);
                 return;
         }
-
-        assert_se(r == 1);
-        assert_se(streq(c.vc.keymap, "es-dvorak"));
+        assert_se(streq(vc.keymap, "es-dvorak"));
+        vc_context_clear(&vc);
 
         log_info("/* test with old mapping (fr:latin9) */");
-        assert_se(free_and_strdup(&xc->layout, "fr") >= 0);
-        assert_se(free_and_strdup(&xc->variant, "latin9") >= 0);
-
-        assert_se(x11_convert_to_vconsole(&c) == 1);
-        assert_se(streq(c.vc.keymap, "fr-latin9"));
+        assert_se(free_and_strdup(&xc.layout, "fr") >= 0);
+        assert_se(free_and_strdup(&xc.variant, "latin9") >= 0);
+        assert_se(x11_convert_to_vconsole(&xc, &vc) >= 0);
+        assert_se(streq(vc.keymap, "fr-latin9"));
+        vc_context_clear(&vc);
 
         log_info("/* test with a compound mapping (us,ru:) */");
-        assert_se(free_and_strdup(&xc->layout, "us,ru") >= 0);
-        assert_se(free_and_strdup(&xc->variant, NULL) >= 0);
-
-        assert_se(x11_convert_to_vconsole(&c) == 1);
-        assert_se(streq(c.vc.keymap, "us"));
+        assert_se(free_and_strdup(&xc.layout, "us,ru") >= 0);
+        assert_se(free_and_strdup(&xc.variant, NULL) >= 0);
+        assert_se(x11_convert_to_vconsole(&xc, &vc) >= 0);
+        assert_se(streq(vc.keymap, "us"));
+        vc_context_clear(&vc);
 
         log_info("/* test with a compound mapping (ru,us:) */");
-        assert_se(free_and_strdup(&xc->layout, "ru,us") >= 0);
-        assert_se(free_and_strdup(&xc->variant, NULL) >= 0);
-
-        assert_se(x11_convert_to_vconsole(&c) == 1);
-        assert_se(streq(c.vc.keymap, "ru"));
+        assert_se(free_and_strdup(&xc.layout, "ru,us") >= 0);
+        assert_se(free_and_strdup(&xc.variant, NULL) >= 0);
+        assert_se(x11_convert_to_vconsole(&xc, &vc) >= 0);
+        assert_se(streq(vc.keymap, "ru"));
+        vc_context_clear(&vc);
 
         /* https://bugzilla.redhat.com/show_bug.cgi?id=1333998 */
         log_info("/* test with a simple new mapping (ru:) */");
-        assert_se(free_and_strdup(&xc->layout, "ru") >= 0);
-        assert_se(free_and_strdup(&xc->variant, NULL) >= 0);
-
-        assert_se(x11_convert_to_vconsole(&c) == 0);
-        assert_se(streq(c.vc.keymap, "ru"));
+        assert_se(free_and_strdup(&xc.layout, "ru") >= 0);
+        assert_se(free_and_strdup(&xc.variant, NULL) >= 0);
+        assert_se(x11_convert_to_vconsole(&xc, &vc) >= 0);
+        assert_se(streq(vc.keymap, "ru"));
 }
 
 static int intro(void) {