From 90005a4f0a2c0a229e2356fb75cc75ea538503d3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 25 Jan 2023 13:35:42 +0900 Subject: 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. --- src/locale/localed-util.c | 175 +++++++++++++++++++---------------------- src/locale/localed-util.h | 10 ++- src/locale/localed.c | 16 +++- src/locale/test-localed-util.c | 152 ++++++++++++++++------------------- 4 files changed, 169 insertions(+), 184 deletions(-) (limited to 'src/locale') diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index 9dccaf9d2b..3c95885e28 100644 --- 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 6e4f270f46..015fcf8901 100644 --- 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 4ba7dc1f7c..b91ebf79ec 100644 --- 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 d7089fa6c3..f57c9ba1f8 100644 --- 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) { -- cgit v1.2.1