From 3c7012cdda46f82324bb5a2ead86a6974bba8b91 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 19 Apr 2023 20:54:13 +0200 Subject: [PATCH 1/3] localed-util: make use of strdupcspn() --- src/locale/localed-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index 17d96aea1d..65b6108e4e 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -817,12 +817,12 @@ int find_legacy_keymap(const X11Context *xc, char **ret) { * matches just the first layout. */ - l = strndup(xc->layout, strcspn(xc->layout, ",")); + l = strdupcspn(xc->layout, ","); if (!l) return -ENOMEM; if (!isempty(xc->variant)) { - v = strndup(xc->variant, strcspn(xc->variant, ",")); + v = strdupcspn(xc->variant, ","); if (!v) return -ENOMEM; } From c821ad7d607b21e807c56d9fa8843821c0fd383e Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 20 Apr 2023 09:27:52 +0200 Subject: [PATCH 2/3] locale: convert generated vconsole keymap to x11 layout automatically When doing x11->console conversions, find_converted_keymap() searches automatically for a candidate in the converted keymap directory for a given x11 layout. However doing console->x11 conversions, this automatic search is not done hence simple conversion in this direction can't be achieved without populating kbd-model-map with entries for converted keymaps. For example, let's consider "at" layout which is not part of kbd-model-map. The "at" x11 layout has a generated keymap "/usr/share/kbd/keymaps/xkb/at.map.gz". If we configure "at" for the x11 layout, localed is able to automatically find the "at" converted vc layout and the conversion just works : $ localectl set-x11-keymap at $ localectl System Locale: LANG=en_US.UTF-8 VC Keymap: at X11 Layout: at However in the opposite direction, ie when setting the vc keymap to "at", no conversion is done and the x11 layout is not defined: $ localectl set-keymap at $ localectl System Locale: LANG=en_US.UTF-8 VC Keymap: at X11 Layout: (unset) This patch fixes this limitation as the implemenation is relatively simple and it removes the need to populate kbd-model-map with (many) entries for converted keymaps. However the patch doesn't remove the existing entries in kbd-model-map which became unneeded after this change to be on the safe side. Note: by default the automatically generated x11 keyboard configs use keyboard model "microsoftpro" which should be equivalent to "pc105" model but with the internet/media key mapping added. --- src/locale/localed-util.c | 44 ++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index 65b6108e4e..bcd49fd618 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -655,6 +655,7 @@ static int read_next_mapping( int vconsole_convert_to_x11(const VCContext *vc, X11Context *ret) { _cleanup_fclose_ FILE *f = NULL; const char *map; + X11Context xc; int r; assert(vc); @@ -672,15 +673,12 @@ int vconsole_convert_to_x11(const VCContext *vc, X11Context *ret) { for (unsigned n = 0;;) { _cleanup_strv_free_ char **a = NULL; - X11Context xc; r = read_next_mapping(map, 5, UINT_MAX, f, &n, &a); if (r < 0) return r; - if (r == 0) { - *ret = (X11Context) {}; - return 0; - } + if (r == 0) + break; if (!streq(vc->keymap, a[0])) continue; @@ -697,6 +695,42 @@ int vconsole_convert_to_x11(const VCContext *vc, X11Context *ret) { return x11_context_copy(ret, &xc); } + + /* No custom mapping has been found, see if the keymap is a converted one. In such case deducing the + * corresponding x11 layout is easy. */ + _cleanup_free_ char *xlayout = NULL, *converted = NULL; + char *xvariant; + + xlayout = strdup(vc->keymap); + if (!xlayout) + return -ENOMEM; + xvariant = strchr(xlayout, '-'); + if (xvariant) { + xvariant[0] = '\0'; + xvariant++; + } + + /* Note: by default we use keyboard model "microsoftpro" which should be equivalent to "pc105" but + * with the internet/media key mapping added. */ + xc = (X11Context) { + .layout = xlayout, + .model = (char*) "microsoftpro", + .variant = xvariant, + .options = (char*) "terminate:ctrl_alt_bksp", + }; + + /* This sanity check seems redundant with the verification of the X11 layout done on the next + * step. However xkbcommon is an optional dependency hence the verification might be a NOP. */ + r = find_converted_keymap(&xc, &converted); + if (r < 0) + return r; + + if (r == 0 || x11_context_verify(&xc) < 0) { + *ret = (X11Context) {}; + return 0; + } + + return x11_context_copy(ret, &xc); } int find_converted_keymap(const X11Context *xc, char **ret) { From b35f71ac1a8b8669cf399711a0396d78581b89eb Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 20 Apr 2023 09:47:35 +0200 Subject: [PATCH 3/3] locale: when no xvariant match select the entry with an empty xvariant When doing a conversion and the specified 'xc->xvariant' has no match, select the x11 layout entry with a matching layout and an empty xvariant if such entry exists. It's still better than no conversion at all. --- src/locale/localed-util.c | 16 +++++++++++++++- src/locale/test-localed-util.c | 30 ++++++++++++++++++++++++++++++ test/units/testsuite-73.sh | 16 ++++++++-------- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index bcd49fd618..02fac9786b 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -722,6 +722,11 @@ int vconsole_convert_to_x11(const VCContext *vc, X11Context *ret) { /* This sanity check seems redundant with the verification of the X11 layout done on the next * step. However xkbcommon is an optional dependency hence the verification might be a NOP. */ r = find_converted_keymap(&xc, &converted); + if (r == 0 && xc.variant) { + /* If we still haven't find a match, try with no variant, it's still better than nothing. */ + xc.variant = NULL; + r = find_converted_keymap(&xc, &converted); + } if (r < 0) return r; @@ -890,8 +895,17 @@ int x11_convert_to_vconsole(const X11Context *xc, VCContext *ret) { } r = find_converted_keymap(xc, &keymap); - if (r == 0) + if (r == 0) { r = find_legacy_keymap(xc, &keymap); + if (r == 0 && xc->variant) + /* If we still haven't find a match, try with no variant, it's still better than + * nothing. */ + r = find_converted_keymap( + &(X11Context) { + .layout = xc->layout, + }, + &keymap); + } if (r < 0) return r; diff --git a/src/locale/test-localed-util.c b/src/locale/test-localed-util.c index f57c9ba1f8..cb66dffd48 100644 --- a/src/locale/test-localed-util.c +++ b/src/locale/test-localed-util.c @@ -44,6 +44,7 @@ TEST(find_converted_keymap) { assert_se(r == 1); assert_se(streq(ans, "pl")); + ans = mfree(ans); assert_se(find_converted_keymap( &(X11Context) { @@ -73,6 +74,7 @@ TEST(find_legacy_keymap) { TEST(vconsole_convert_to_x11) { _cleanup_(x11_context_clear) X11Context xc = {}; _cleanup_(vc_context_clear) VCContext vc = {}; + int r; log_info("/* test empty keymap */"); assert_se(vconsole_convert_to_x11(&vc, &xc) >= 0); @@ -111,6 +113,34 @@ TEST(vconsole_convert_to_x11) { assert_se(vconsole_convert_to_x11(&vc, &xc) >= 0); assert_se(streq(xc.layout, "us")); assert_se(xc.variant == NULL); + x11_context_clear(&xc); + + /* "gh" has no mapping in kbd-model-map and kbd provides a converted keymap for this layout. */ + log_info("/* test with a converted keymap (gh:) */"); + assert_se(free_and_strdup(&vc.keymap, "gh") >= 0); + r = vconsole_convert_to_x11(&vc, &xc); + if (r == 0) { + log_info("Skipping rest of %s: keymaps are not installed", __func__); + return; + } + assert_se(r > 0); + assert_se(streq(xc.layout, "gh")); + assert_se(xc.variant == NULL); + x11_context_clear(&xc); + + log_info("/* test with converted keymap and with a known variant (gh:ewe) */"); + assert_se(free_and_strdup(&vc.keymap, "gh-ewe") >= 0); + assert_se(vconsole_convert_to_x11(&vc, &xc) > 0); + assert_se(streq(xc.layout, "gh")); + assert_se(streq(xc.variant, "ewe")); + x11_context_clear(&xc); + + log_info("/* test with converted keymap and with an unknown variant (gh:ewe) */"); + assert_se(free_and_strdup(&vc.keymap, "gh-foobar") > 0); + assert_se(vconsole_convert_to_x11(&vc, &xc) > 0); + assert_se(streq(xc.layout, "gh")); + assert_se(xc.variant == NULL); + x11_context_clear(&xc); } TEST(x11_convert_to_vconsole) { diff --git a/test/units/testsuite-73.sh b/test/units/testsuite-73.sh index e5ac09c2e3..56000b96d2 100755 --- a/test/units/testsuite-73.sh +++ b/test/units/testsuite-73.sh @@ -273,15 +273,15 @@ test_vc_keymap() { assert_in "XKBVARIANT=intl" "$vc" assert_in "XKBOPTIONS=terminate:ctrl_alt_bksp" "$vc" elif [[ "$i" =~ ^us-.* ]]; then - assert_in "X11 Layout: .unset." "$output" - assert_not_in "X11 Model:" "$output" - assert_not_in "X11 Variant:" "$output" - assert_not_in "X11 Options:" "$output" + assert_in "X11 Layout: us" "$output" + assert_in "X11 Model: microsoftpro" "$output" + assert_in "X11 Variant:" "$output" + assert_in "X11 Options: terminate:ctrl_alt_bksp" "$output" - assert_not_in "XKBLAYOUT" "$vc" - assert_not_in "XKBMODEL" "$vc" - assert_not_in "XKBVARIANT" "$vc" - assert_not_in "XKBOPTIONS" "$vc" + assert_in "XKBLAYOUT=us" "$vc" + assert_in "XKBMODEL=microsoftpro" "$vc" + assert_in "XKBVARIANT=" "$vc" + assert_in "XKBOPTIONS=terminate:ctrl_alt_bksp" "$vc" fi done