From e0a720012d7bb0e2c132e99b62234b04c3031382 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jan 2023 14:46:49 +0900 Subject: [PATCH 01/11] locale: add missing logs --- src/locale/localed.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/locale/localed.c b/src/locale/localed.c index 68c014fd47..5b2dcb7904 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -252,11 +252,11 @@ static int method_set_locale(sd_bus_message *m, void *userdata, sd_bus_error *er r = sd_bus_message_read_strv(m, &l); if (r < 0) - return r; + return bus_log_parse_error(r); r = sd_bus_message_read_basic(m, 'b', &interactive); if (r < 0) - return r; + return bus_log_parse_error(r); use_localegen = locale_gen_check_available(); @@ -269,7 +269,7 @@ static int method_set_locale(sd_bus_message *m, void *userdata, sd_bus_error *er new_locale[VARIABLE_LANG] = strdup(l[0]); if (!new_locale[VARIABLE_LANG]) - return -ENOMEM; + return log_oom(); l = strv_free(l); } @@ -303,7 +303,7 @@ static int method_set_locale(sd_bus_message *m, void *userdata, sd_bus_error *er /* Merge with the current settings */ r = locale_context_merge(&c->locale_context, new_locale); if (r < 0) - return r; + return log_oom(); locale_variables_simplify(new_locale); @@ -374,7 +374,7 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro r = sd_bus_message_read(m, "ssbb", &keymap, &keymap_toggle, &convert, &interactive); if (r < 0) - return r; + return bus_log_parse_error(r); keymap = empty_to_null(keymap); keymap_toggle = empty_to_null(keymap_toggle); @@ -573,7 +573,7 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err r = sd_bus_message_read(m, "ssssbb", &in.layout, &in.model, &in.variant, &in.options, &convert, &interactive); if (r < 0) - return r; + return bus_log_parse_error(r); x11_context_empty_to_null(&in); From ba4a886f2d0d4c5409fb0bd7ec7faaa080a2ac0b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 25 Jan 2023 12:11:40 +0900 Subject: [PATCH 02/11] locale: introduce VCContext and several helper functions for the struct No functional changes, preparation for later commits. --- src/locale/localed-util.c | 97 ++++++++++++++++++++++++++-------- src/locale/localed-util.h | 12 ++++- src/locale/localed.c | 24 ++++----- src/locale/test-localed-util.c | 30 +++++------ 4 files changed, 110 insertions(+), 53 deletions(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index e742a8d0b1..9dccaf9d2b 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -152,11 +152,65 @@ static void context_clear_x11(Context *c) { x11_context_clear(&c->x11_from_vc); } -static void context_clear_vconsole(Context *c) { - assert(c); +static void vc_context_clear(VCContext *vc) { + assert(vc); - c->vc_keymap = mfree(c->vc_keymap); - c->vc_keymap_toggle = mfree(c->vc_keymap_toggle); + vc->keymap = mfree(vc->keymap); + vc->toggle = mfree(vc->toggle); +} + +static bool vc_context_isempty(const VCContext *vc) { + assert(vc); + + return + isempty(vc->keymap) && + isempty(vc->toggle); +} + +void vc_context_empty_to_null(VCContext *vc) { + assert(vc); + + /* Do not call vc_context_clear() for the passed object. */ + + vc->keymap = empty_to_null(vc->keymap); + vc->toggle = empty_to_null(vc->toggle); +} + +bool vc_context_equal(const VCContext *a, const VCContext *b) { + assert(a); + assert(b); + + return + streq_ptr(a->keymap, b->keymap) && + streq_ptr(a->toggle, b->toggle); +} + +int vc_context_copy(VCContext *dest, const VCContext *src) { + bool modified; + int r; + + assert(dest); + + if (dest == src) + return 0; + + if (!src) { + modified = !vc_context_isempty(dest); + vc_context_clear(dest); + return modified; + } + + r = free_and_strdup(&dest->keymap, src->keymap); + if (r < 0) + return r; + modified = r > 0; + + r = free_and_strdup(&dest->toggle, src->toggle); + if (r < 0) + return r; + modified = modified || r > 0; + + return modified; } void context_clear(Context *c) { @@ -164,7 +218,7 @@ void context_clear(Context *c) { locale_context_clear(&c->locale_context); context_clear_x11(c); - context_clear_vconsole(c); + vc_context_clear(&c->vc); c->locale_cache = sd_bus_message_unref(c->locale_cache); c->x11_cache = sd_bus_message_unref(c->x11_cache); @@ -223,7 +277,7 @@ int vconsole_read_data(Context *c, sd_bus_message *m) { fd = RET_NERRNO(open("/etc/vconsole.conf", O_CLOEXEC | O_PATH)); if (fd == -ENOENT) { c->vc_stat = (struct stat) {}; - context_clear_vconsole(c); + vc_context_clear(&c->vc); return 0; } if (fd < 0) @@ -237,12 +291,12 @@ int vconsole_read_data(Context *c, sd_bus_message *m) { return 0; c->vc_stat = st; - context_clear_vconsole(c); + vc_context_clear(&c->vc); x11_context_clear(&c->x11_from_vc); return parse_env_file_fd(fd, "/etc/vconsole.conf", - "KEYMAP", &c->vc_keymap, - "KEYMAP_TOGGLE", &c->vc_keymap_toggle, + "KEYMAP", &c->vc.keymap, + "KEYMAP_TOGGLE", &c->vc.toggle, "XKBLAYOUT", &c->x11_from_vc.layout, "XKBMODEL", &c->x11_from_vc.model, "XKBVARIANT", &c->x11_from_vc.variant, @@ -380,11 +434,11 @@ int vconsole_write_data(Context *c) { if (r < 0 && r != -ENOENT) return r; - r = strv_env_assign(&l, "KEYMAP", empty_to_null(c->vc_keymap)); + r = strv_env_assign(&l, "KEYMAP", empty_to_null(c->vc.keymap)); if (r < 0) return r; - r = strv_env_assign(&l, "KEYMAP_TOGGLE", empty_to_null(c->vc_keymap_toggle)); + r = strv_env_assign(&l, "KEYMAP_TOGGLE", empty_to_null(c->vc.toggle)); if (r < 0) return r; @@ -542,7 +596,7 @@ int vconsole_convert_to_x11(Context *c) { * vconsole_write_data() if necessary. */ xc = context_get_x11_context_safe(c); - if (isempty(c->vc_keymap)) { + if (isempty(c->vc.keymap)) { modified = !x11_context_isempty(xc); context_clear_x11(c); } else { @@ -563,7 +617,7 @@ int vconsole_convert_to_x11(Context *c) { if (r == 0) break; - if (!streq(c->vc_keymap, a[0])) + if (!streq(c->vc.keymap, a[0])) continue; r = x11_context_copy(xc, @@ -589,7 +643,7 @@ int vconsole_convert_to_x11(Context *c) { strempty(xc->options)); else if (modified < 0) log_notice("X11 keyboard layout was not modified: no conversion found for \"%s\".", - c->vc_keymap); + c->vc.keymap); else log_debug("X11 keyboard layout did not need to be modified."); @@ -778,11 +832,8 @@ int x11_convert_to_vconsole(Context *c) { xc = context_get_x11_context_safe(c); if (isempty(xc->layout)) { - modified = - !isempty(c->vc_keymap) || - !isempty(c->vc_keymap_toggle); - - context_clear_vconsole(c); + modified = !vc_context_isempty(&c->vc); + vc_context_clear(&c->vc); } else { _cleanup_free_ char *new_keymap = NULL; int r; @@ -798,16 +849,16 @@ int x11_convert_to_vconsole(Context *c) { * 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)) { - context_clear_vconsole(c); - c->vc_keymap = TAKE_PTR(new_keymap); + if (!streq_ptr(c->vc.keymap, new_keymap)) { + vc_context_clear(&c->vc); + c->vc.keymap = TAKE_PTR(new_keymap); modified = true; } } if (modified) log_info("Changing virtual console keymap to '%s' toggle '%s'", - strempty(c->vc_keymap), strempty(c->vc_keymap_toggle)); + strempty(c->vc.keymap), strempty(c->vc.toggle)); else log_debug("Virtual console keymap was not modified."); diff --git a/src/locale/localed-util.h b/src/locale/localed-util.h index a3e763f850..6e4f270f46 100644 --- a/src/locale/localed-util.h +++ b/src/locale/localed-util.h @@ -15,6 +15,11 @@ typedef struct X11Context { char *options; } X11Context; +typedef struct VCContext { + char *keymap; + char *toggle; +} VCContext; + typedef struct Context { sd_bus_message *locale_cache; LocaleContext locale_context; @@ -26,8 +31,7 @@ typedef struct Context { sd_bus_message *vc_cache; struct stat vc_stat; - char *vc_keymap; - char *vc_keymap_toggle; + VCContext vc; Hashmap *polkit_registry; } Context; @@ -39,6 +43,10 @@ int x11_context_copy(X11Context *dest, const X11Context *src); X11Context *context_get_x11_context_safe(Context *c); +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); + int find_converted_keymap(const X11Context *xc, char **ret); int find_legacy_keymap(const X11Context *xc, char **ret); int find_language_fallback(const char *lang, char **ret); diff --git a/src/locale/localed.c b/src/locale/localed.c index 5b2dcb7904..4ba7dc1f7c 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -118,9 +118,9 @@ static int property_get_vconsole( return r; if (streq(property, "VConsoleKeymap")) - return sd_bus_message_append_basic(reply, 's', c->vc_keymap); + return sd_bus_message_append_basic(reply, 's', c->vc.keymap); if (streq(property, "VConsoleKeymapToggle")) - return sd_bus_message_append_basic(reply, 's', c->vc_keymap_toggle); + return sd_bus_message_append_basic(reply, 's', c->vc.toggle); return -EINVAL; } @@ -367,17 +367,16 @@ static int method_set_locale(sd_bus_message *m, void *userdata, sd_bus_error *er static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_error *error) { Context *c = ASSERT_PTR(userdata); - const char *keymap, *keymap_toggle; int convert, interactive, r; + VCContext in; assert(m); - r = sd_bus_message_read(m, "ssbb", &keymap, &keymap_toggle, &convert, &interactive); + r = sd_bus_message_read(m, "ssbb", &in.keymap, &in.toggle, &convert, &interactive); if (r < 0) return bus_log_parse_error(r); - keymap = empty_to_null(keymap); - keymap_toggle = empty_to_null(keymap_toggle); + vc_context_empty_to_null(&in); r = vconsole_read_data(c, m); if (r < 0) { @@ -385,7 +384,7 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro return sd_bus_error_set_errnof(error, r, "Failed to read virtual console keymap data: %m"); } - FOREACH_STRING(name, keymap ?: keymap_toggle, keymap ? keymap_toggle : NULL) { + FOREACH_STRING(name, in.keymap ?: in.toggle, in.keymap ? in.toggle : NULL) { r = keymap_exists(name); /* This also verifies that the keymap name is kosher. */ if (r < 0) { log_error_errno(r, "Failed to check keymap %s: %m", name); @@ -395,8 +394,7 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Keymap %s is not installed.", name); } - if (streq_ptr(keymap, c->vc_keymap) && - streq_ptr(keymap_toggle, c->vc_keymap_toggle)) + if (vc_context_equal(&c->vc, &in)) return sd_bus_reply_method_return(m, NULL); r = bus_verify_polkit_async( @@ -413,9 +411,9 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro 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_strdup(&c->vc_keymap, keymap) < 0 || - free_and_strdup(&c->vc_keymap_toggle, keymap_toggle) < 0) - return -ENOMEM; + r = vc_context_copy(&c->vc, &in); + if (r < 0) + return log_oom(); if (convert) { r = x11_read_data(c, m); @@ -445,7 +443,7 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro } log_info("Changed virtual console keymap to '%s' toggle '%s'", - strempty(c->vc_keymap), strempty(c->vc_keymap_toggle)); + strempty(c->vc.keymap), strempty(c->vc.toggle)); (void) vconsole_reload(sd_bus_message_get_bus(m)); diff --git a/src/locale/test-localed-util.c b/src/locale/test-localed-util.c index 3d96b3aa85..d7089fa6c3 100644 --- a/src/locale/test-localed-util.c +++ b/src/locale/test-localed-util.c @@ -88,35 +88,35 @@ TEST(vconsole_convert_to_x11) { assert_se(xc->variant == NULL); log_info("/* test without variant, new mapping (es:) */"); - assert_se(free_and_strdup(&c.vc_keymap, "es") >= 0); + 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); log_info("/* test with known variant, new mapping (es:dvorak) */"); - assert_se(free_and_strdup(&c.vc_keymap, "es-dvorak") >= 0); + 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")); log_info("/* test with old mapping (fr:latin9) */"); - assert_se(free_and_strdup(&c.vc_keymap, "fr-latin9") >= 0); + 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")); log_info("/* test with a compound mapping (ru,us) */"); - assert_se(free_and_strdup(&c.vc_keymap, "ru") >= 0); + 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); log_info("/* test with a simple mapping (us) */"); - assert_se(free_and_strdup(&c.vc_keymap, "us") >= 0); + assert_se(free_and_strdup(&c.vc.keymap, "us") >= 0); assert_se(vconsole_convert_to_x11(&c) == 1); assert_se(streq(xc->layout, "us")); @@ -129,26 +129,26 @@ TEST(x11_convert_to_vconsole) { int r; log_info("/* test emptying first (:) */"); - assert_se(free_and_strdup(&c.vc_keymap, "foobar") >= 0); + assert_se(free_and_strdup(&c.vc.keymap, "foobar") >= 0); assert_se(x11_convert_to_vconsole(&c) == 1); - assert_se(c.vc_keymap == NULL); + assert_se(c.vc.keymap == NULL); log_info("/* test emptying second (:) */"); assert_se(x11_convert_to_vconsole(&c) == 0); - assert_se(c.vc_keymap == NULL); + assert_se(c.vc.keymap == NULL); 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(streq(c.vc.keymap, "es")); 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(streq(c.vc.keymap, "es")); log_info("/* test with known variant, new mapping (es:dvorak) */"); assert_se(free_and_strdup(&xc->variant, "dvorak") >= 0); @@ -160,28 +160,28 @@ TEST(x11_convert_to_vconsole) { } assert_se(r == 1); - assert_se(streq(c.vc_keymap, "es-dvorak")); + assert_se(streq(c.vc.keymap, "es-dvorak")); 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(streq(c.vc.keymap, "fr-latin9")); 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(streq(c.vc.keymap, "us")); 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(streq(c.vc.keymap, "ru")); /* https://bugzilla.redhat.com/show_bug.cgi?id=1333998 */ log_info("/* test with a simple new mapping (ru:) */"); @@ -189,7 +189,7 @@ TEST(x11_convert_to_vconsole) { 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(streq(c.vc.keymap, "ru")); } static int intro(void) { From 90005a4f0a2c0a229e2356fb75cc75ea538503d3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 25 Jan 2023 13:35:42 +0900 Subject: [PATCH 03/11] 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 | 183 ++++++++++++++++----------------- src/locale/localed-util.h | 10 +- src/locale/localed.c | 16 ++- src/locale/test-localed-util.c | 152 ++++++++++++--------------- 4 files changed, 173 insertions(+), 188 deletions(-) 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; +int vconsole_convert_to_x11(const VCContext *vc, X11Context *ret) { + _cleanup_fclose_ FILE *f = NULL; + const char *map; + int r; - assert(c); + assert(vc); + assert(ret); - /* 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); - - if (isempty(c->vc.keymap)) { - modified = !x11_context_isempty(xc); - context_clear_x11(c); - } else { - _cleanup_fclose_ FILE *f = NULL; - const char *map; - - map = systemd_kbd_model_map(); - f = fopen(map, "re"); - if (!f) - return -errno; - - 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; - - if (!streq(c->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; - - break; - } + if (isempty(vc->keymap)) { + *ret = (X11Context) {}; + 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."); + map = systemd_kbd_model_map(); + f = fopen(map, "re"); + if (!f) + return -errno; - return modified > 0; + 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) { + log_notice("No conversion found for virtual console keymap \"%s\".", vc->keymap); + *ret = (X11Context) {}; + return 0; + } + + if (!streq(vc->keymap, a[0])) + continue; + + 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; + + 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; + } } 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; +int x11_convert_to_vconsole(const X11Context *xc, VCContext *ret) { + _cleanup_free_ char *keymap = NULL; + int r; - assert(c); - - 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; + _cleanup_(x11_context_clear) X11Context xc = {}; + _cleanup_(vc_context_clear) VCContext vc = {}; - 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 (:) */"); - - 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) { From f59d83afaab95a60998428579da505144cc4906d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 26 Jan 2023 17:34:08 +0900 Subject: [PATCH 04/11] locale: replace context_get_x11_context() with context_get_x11_context_safe() Then, context_get_x11_context() always replies a valid X11 context. No functional change, just refactoring. --- src/locale/localed-util.c | 19 +++++++------------ src/locale/localed-util.h | 2 +- src/locale/localed.c | 6 +++--- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index 3c95885e28..87e232e08f 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -245,7 +245,7 @@ void context_clear(Context *c) { c->polkit_registry = bus_verify_polkit_async_registry_free(c->polkit_registry); }; -static X11Context *context_get_x11_context(Context *c) { +X11Context *context_get_x11_context(Context *c) { assert(c); if (!x11_context_isempty(&c->x11_from_vc)) @@ -254,12 +254,7 @@ static X11Context *context_get_x11_context(Context *c) { if (!x11_context_isempty(&c->x11_from_xorg)) return &c->x11_from_xorg; - return NULL; -} - -X11Context *context_get_x11_context_safe(Context *c) { - assert(c); - return context_get_x11_context(c) ?: &c->x11_from_vc; + return &c->x11_from_vc; } int locale_read_data(Context *c, sd_bus_message *m) { @@ -460,19 +455,19 @@ int vconsole_write_data(Context *c) { if (r < 0) return r; - r = strv_env_assign(&l, "XKBLAYOUT", xc ? empty_to_null(xc->layout) : NULL); + r = strv_env_assign(&l, "XKBLAYOUT", empty_to_null(xc->layout)); if (r < 0) return r; - r = strv_env_assign(&l, "XKBMODEL", xc ? empty_to_null(xc->model) : NULL); + r = strv_env_assign(&l, "XKBMODEL", empty_to_null(xc->model)); if (r < 0) return r; - r = strv_env_assign(&l, "XKBVARIANT", xc ? empty_to_null(xc->variant) : NULL); + r = strv_env_assign(&l, "XKBVARIANT", empty_to_null(xc->variant)); if (r < 0) return r; - r = strv_env_assign(&l, "XKBOPTIONS", xc ? empty_to_null(xc->options) : NULL); + r = strv_env_assign(&l, "XKBOPTIONS", empty_to_null(xc->options)); if (r < 0) return r; @@ -503,7 +498,7 @@ int x11_write_data(Context *c) { assert(c); xc = context_get_x11_context(c); - if (!xc) { + if (x11_context_isempty(xc)) { if (unlink("/etc/X11/xorg.conf.d/00-keyboard.conf") < 0) return errno == ENOENT ? 0 : -errno; diff --git a/src/locale/localed-util.h b/src/locale/localed-util.h index 015fcf8901..83d253c18d 100644 --- a/src/locale/localed-util.h +++ b/src/locale/localed-util.h @@ -44,7 +44,7 @@ bool x11_context_is_safe(const X11Context *xc); bool x11_context_equal(const X11Context *a, const X11Context *b); int x11_context_copy(X11Context *dest, const X11Context *src); -X11Context *context_get_x11_context_safe(Context *c); +X11Context *context_get_x11_context(Context *c); void vc_context_clear(VCContext *vc); void vc_context_replace(VCContext *dest, VCContext *src); diff --git a/src/locale/localed.c b/src/locale/localed.c index b91ebf79ec..dcd3b3e38d 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -144,7 +144,7 @@ static int property_get_xkb( if (r < 0) return r; - xc = context_get_x11_context_safe(c); + xc = context_get_x11_context(c); if (streq(property, "X11Layout")) return sd_bus_message_append_basic(reply, 's', xc->layout); @@ -432,7 +432,7 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro } /* save the result of conversion to emit changed properties later. */ - xc = context_get_x11_context_safe(c); + xc = context_get_x11_context(c); convert = !x11_context_equal(xc, &converted); x11_context_replace(xc, &converted); } @@ -586,7 +586,7 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err return sd_bus_error_set(error, SD_BUS_ERROR_FAILED, "Failed to read x11 keyboard layout data"); } - xc = context_get_x11_context_safe(c); + xc = context_get_x11_context(c); if (x11_context_equal(xc, &in)) return sd_bus_reply_method_return(m, NULL); From 800f65f827c9828d4c872d44b19ca8a008505690 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 26 Jan 2023 17:48:11 +0900 Subject: [PATCH 05/11] locale: always check input keyboard layout and friends earlier --- src/locale/localed.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/locale/localed.c b/src/locale/localed.c index dcd3b3e38d..88f5604f1e 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -378,12 +378,6 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro vc_context_empty_to_null(&in); - r = vconsole_read_data(c, m); - if (r < 0) { - log_error_errno(r, "Failed to read virtual console keymap data: %m"); - return sd_bus_error_set_errnof(error, r, "Failed to read virtual console keymap data: %m"); - } - FOREACH_STRING(name, in.keymap ?: in.toggle, in.keymap ? in.toggle : NULL) { r = keymap_exists(name); /* This also verifies that the keymap name is kosher. */ if (r < 0) { @@ -394,6 +388,12 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Keymap %s is not installed.", name); } + r = vconsole_read_data(c, m); + if (r < 0) { + log_error_errno(r, "Failed to read virtual console keymap data: %m"); + return sd_bus_error_set_errnof(error, r, "Failed to read virtual console keymap data: %m"); + } + if (vc_context_equal(&c->vc, &in)) return sd_bus_reply_method_return(m, NULL); @@ -580,17 +580,6 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err x11_context_empty_to_null(&in); - 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(error, SD_BUS_ERROR_FAILED, "Failed to read x11 keyboard layout data"); - } - - xc = context_get_x11_context(c); - - if (x11_context_equal(xc, &in)) - return sd_bus_reply_method_return(m, NULL); - if (!x11_context_is_safe(&in)) return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Received invalid keyboard data"); @@ -605,6 +594,17 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Specified keymap cannot be compiled, refusing as invalid."); } + 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(error, SD_BUS_ERROR_FAILED, "Failed to read x11 keyboard layout data"); + } + + xc = context_get_x11_context(c); + + if (x11_context_equal(xc, &in)) + return sd_bus_reply_method_return(m, NULL); + r = bus_verify_polkit_async( m, CAP_SYS_ADMIN, From 71fa933b46fa1160e4c4eb037e5e9cec9c05c082 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 26 Jan 2023 17:41:47 +0900 Subject: [PATCH 06/11] locale: sync two X11 contexts on update --- src/locale/localed-util.c | 13 --------- src/locale/localed.c | 59 +++++++++++++++++++++++++-------------- 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index 87e232e08f..2d5d76e741 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -329,11 +329,6 @@ int x11_read_data(Context *c, sd_bus_message *m) { if (r < 0) return r; - if (!x11_context_isempty(&c->x11_from_vc)) { - log_debug("XKB settings loaded from vconsole.conf, not reading xorg.conf.d/00-keyboard.conf."); - return 0; - } - /* Do not try to re-read the file within single bus operation. */ if (m) { if (m == c->x11_cache) @@ -423,9 +418,6 @@ int x11_read_data(Context *c, sd_bus_message *m) { in_section = false; } - if (!x11_context_isempty(&c->x11_from_xorg)) - log_debug("XKB settings loaded from xorg.conf.d/00-keyboard.conf."); - return 0; } @@ -438,11 +430,6 @@ int vconsole_write_data(Context *c) { xc = context_get_x11_context(c); - /* If the X11 context is from xorg.conf, then sync one from vconsole.conf with it. */ - r = x11_context_copy(&c->x11_from_vc, xc); - if (r < 0) - return r; - r = load_env_file(NULL, "/etc/vconsole.conf", &l); if (r < 0 && r != -ENOENT) return r; diff --git a/src/locale/localed.c b/src/locale/localed.c index 88f5604f1e..9070610b80 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -368,6 +368,7 @@ static int method_set_locale(sd_bus_message *m, void *userdata, sd_bus_error *er static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_error *error) { Context *c = ASSERT_PTR(userdata); int convert, interactive, r; + bool x_needs_update; VCContext in; assert(m); @@ -415,15 +416,14 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro if (r < 0) return log_oom(); + 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"); + } + 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(&in, &converted); if (r < 0) { @@ -431,17 +431,32 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro return sd_bus_error_set_errnof(error, r, "Failed to convert keymap data: %m"); } - /* save the result of conversion to emit changed properties later. */ - xc = context_get_x11_context(c); - convert = !x11_context_equal(xc, &converted); - x11_context_replace(xc, &converted); + x_needs_update = !x11_context_equal(&c->x11_from_vc, &converted) || !x11_context_equal(&c->x11_from_xorg, &converted); + + r = x11_context_copy(&c->x11_from_vc, &converted); + if (r < 0) + return log_oom(); + x11_context_replace(&c->x11_from_xorg, &converted); + } else { + const X11Context *xc = context_get_x11_context(c); + + /* Even if the conversion is not requested, sync the two X11 contexts. */ + x_needs_update = !x11_context_equal(&c->x11_from_vc, &c->x11_from_xorg); + + r = x11_context_copy(&c->x11_from_vc, xc); + if (r < 0) + return log_oom(); + + r = x11_context_copy(&c->x11_from_xorg, xc); + if (r < 0) + return log_oom(); } r = vconsole_write_data(c); if (r < 0) log_warning_errno(r, "Failed to write virtual console keymap, ignoring: %m"); - if (convert) { + if (x_needs_update) { r = x11_write_data(c); if (r < 0) log_warning_errno(r, "Failed to write X11 keyboard layout, ignoring: %m"); @@ -457,10 +472,10 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro "/org/freedesktop/locale1", "org.freedesktop.locale1", "VConsoleKeymap", "VConsoleKeymapToggle", - convert ? "X11Layout" : NULL, - convert ? "X11Model" : NULL, - convert ? "X11Variant" : NULL, - convert ? "X11Options" : NULL, + x_needs_update ? "X11Layout" : NULL, + x_needs_update ? "X11Model" : NULL, + x_needs_update ? "X11Variant" : NULL, + x_needs_update ? "X11Options" : NULL, NULL); return sd_bus_reply_method_return(m, NULL); @@ -570,7 +585,7 @@ static int verify_xkb_rmlvo(const char *model, const char *layout, const char *v static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_error *error) { Context *c = ASSERT_PTR(userdata); int convert, interactive, r; - X11Context *xc, in; + X11Context in; assert(m); @@ -600,9 +615,7 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err return sd_bus_error_set(error, SD_BUS_ERROR_FAILED, "Failed to read x11 keyboard layout data"); } - xc = context_get_x11_context(c); - - if (x11_context_equal(xc, &in)) + if (x11_context_equal(&c->x11_from_vc, &in) && x11_context_equal(&c->x11_from_xorg, &in)) return sd_bus_reply_method_return(m, NULL); r = bus_verify_polkit_async( @@ -619,7 +632,11 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - r = x11_context_copy(xc, &in); + r = x11_context_copy(&c->x11_from_vc, &in); + if (r < 0) + return log_oom(); + + r = x11_context_copy(&c->x11_from_xorg, &in); if (r < 0) return log_oom(); From f41338da6e69de3f9baf22ed75545fe2836a03e4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 26 Jan 2023 18:05:32 +0900 Subject: [PATCH 07/11] locale: also check if converted keymap or friends is same as the current settings Before this commit, if virtual console keymap is unchanged, localed just returns without modifying anything. However, the X11 part may need updating. So we should check for both and ensure they are unmodified. Replaces #26190. --- src/locale/localed.c | 94 +++++++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 45 deletions(-) diff --git a/src/locale/localed.c b/src/locale/localed.c index 9070610b80..b8031300f1 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -366,6 +366,7 @@ static int method_set_locale(sd_bus_message *m, void *userdata, sd_bus_error *er } static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_error *error) { + _cleanup_(x11_context_clear) X11Context converted = {}; Context *c = ASSERT_PTR(userdata); int convert, interactive, r; bool x_needs_update; @@ -395,7 +396,25 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro return sd_bus_error_set_errnof(error, r, "Failed to read virtual console keymap data: %m"); } - if (vc_context_equal(&c->vc, &in)) + 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"); + } + + if (convert) { + 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. */ + x_needs_update = !x11_context_equal(&c->x11_from_vc, &converted) || !x11_context_equal(&c->x11_from_xorg, &converted); + } else + x_needs_update = !x11_context_equal(&c->x11_from_vc, &c->x11_from_xorg); + + if (vc_context_equal(&c->vc, &in) && !x_needs_update) return sd_bus_reply_method_return(m, NULL); r = bus_verify_polkit_async( @@ -416,40 +435,24 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro if (r < 0) return log_oom(); - 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"); - } + if (x_needs_update) { + if (convert) { + r = x11_context_copy(&c->x11_from_vc, &converted); + if (r < 0) + return log_oom(); + x11_context_replace(&c->x11_from_xorg, &converted); + } else { + const X11Context *xc = context_get_x11_context(c); - if (convert) { - _cleanup_(x11_context_clear) X11Context converted = {}; + /* Even if the conversion is not requested, sync the two X11 contexts. */ + r = x11_context_copy(&c->x11_from_vc, xc); + if (r < 0) + return log_oom(); - 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"); + r = x11_context_copy(&c->x11_from_xorg, xc); + if (r < 0) + return log_oom(); } - - x_needs_update = !x11_context_equal(&c->x11_from_vc, &converted) || !x11_context_equal(&c->x11_from_xorg, &converted); - - r = x11_context_copy(&c->x11_from_vc, &converted); - if (r < 0) - return log_oom(); - x11_context_replace(&c->x11_from_xorg, &converted); - } else { - const X11Context *xc = context_get_x11_context(c); - - /* Even if the conversion is not requested, sync the two X11 contexts. */ - x_needs_update = !x11_context_equal(&c->x11_from_vc, &c->x11_from_xorg); - - r = x11_context_copy(&c->x11_from_vc, xc); - if (r < 0) - return log_oom(); - - r = x11_context_copy(&c->x11_from_xorg, xc); - if (r < 0) - return log_oom(); } r = vconsole_write_data(c); @@ -583,6 +586,7 @@ static int verify_xkb_rmlvo(const char *model, const char *layout, const char *v #endif static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_error *error) { + _cleanup_(vc_context_clear) VCContext converted = {}; Context *c = ASSERT_PTR(userdata); int convert, interactive, r; X11Context in; @@ -615,7 +619,18 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err return sd_bus_error_set(error, SD_BUS_ERROR_FAILED, "Failed to read x11 keyboard layout data"); } - if (x11_context_equal(&c->x11_from_vc, &in) && x11_context_equal(&c->x11_from_xorg, &in)) + if (convert) { + 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 = !vc_context_equal(&c->vc, &converted); + } + + if (x11_context_equal(&c->x11_from_vc, &in) && x11_context_equal(&c->x11_from_xorg, &in) && !convert) return sd_bus_reply_method_return(m, NULL); r = bus_verify_polkit_async( @@ -640,19 +655,8 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err if (r < 0) return log_oom(); - if (convert) { - _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 = !vc_context_equal(&c->vc, &converted); + if (convert) vc_context_replace(&c->vc, &converted); - } r = vconsole_write_data(c); if (r < 0) From 189cacabe9a8cfb6acf04dd9c9c525fdd88e7294 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jan 2023 15:01:38 +0900 Subject: [PATCH 08/11] locale: decouple vconsole_read_data() from x11_read_data() Let's make library-functions simple and independent as possible as they are. No functional change, just refactoring. --- src/locale/localed-util.c | 4 ---- src/locale/localed.c | 10 ++++++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index 2d5d76e741..54e32e2e1c 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -325,10 +325,6 @@ int x11_read_data(Context *c, sd_bus_message *m) { assert(c); - r = vconsole_read_data(c, m); - if (r < 0) - return r; - /* Do not try to re-read the file within single bus operation. */ if (m) { if (m == c->x11_cache) diff --git a/src/locale/localed.c b/src/locale/localed.c index b8031300f1..7355b2d0e2 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -140,6 +140,10 @@ static int property_get_xkb( assert(property); + r = vconsole_read_data(c, reply); + if (r < 0) + return r; + r = x11_read_data(c, reply); if (r < 0) return r; @@ -613,6 +617,12 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Specified keymap cannot be compiled, refusing as invalid."); } + r = vconsole_read_data(c, m); + if (r < 0) { + log_error_errno(r, "Failed to read virtual console keymap data: %m"); + return sd_bus_error_set_errnof(error, r, "Failed to read virtual console keymap data: %m"); + } + r = x11_read_data(c, m); if (r < 0) { log_error_errno(r, "Failed to read x11 keyboard layout data: %m"); From 8669bbb565294b01b9978884c23cdec8516d4f65 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 26 Jan 2023 23:30:27 +0900 Subject: [PATCH 09/11] locale: move logging from library-like functions to caller --- src/locale/localed-util.c | 17 +---------------- src/locale/localed.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index 54e32e2e1c..f5cc46c836 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -605,7 +605,6 @@ int vconsole_convert_to_x11(const VCContext *vc, X11Context *ret) { if (r < 0) return r; if (r == 0) { - log_notice("No conversion found for virtual console keymap \"%s\".", vc->keymap); *ret = (X11Context) {}; return 0; } @@ -613,19 +612,13 @@ int vconsole_convert_to_x11(const VCContext *vc, X11Context *ret) { if (!streq(vc->keymap, a[0])) continue; - r = x11_context_copy(ret, + return 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; - - 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; } } @@ -819,14 +812,6 @@ int x11_convert_to_vconsole(const X11Context *xc, VCContext *ret) { 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_info("The X11 keyboard layout '%s' is converted to virtual console keymap '%s'", - xc->layout, strempty(keymap)); *ret = (VCContext) { .keymap = TAKE_PTR(keymap), diff --git a/src/locale/localed.c b/src/locale/localed.c index 7355b2d0e2..267f27635b 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -413,6 +413,12 @@ static int method_set_vc_keyboard(sd_bus_message *m, void *userdata, sd_bus_erro return sd_bus_error_set_errnof(error, r, "Failed to convert keymap data: %m"); } + if (x11_context_isempty(&converted)) + log_notice("No conversion found for virtual console keymap \"%s\".", strempty(in.keymap)); + else + log_info("The virtual console keymap '%s' is converted to X11 keyboard layout '%s' model '%s' variant '%s' options '%s'", + in.keymap, strempty(converted.layout), strempty(converted.model), strempty(converted.variant), strempty(converted.options)); + /* save the result of conversion to emit changed properties later. */ x_needs_update = !x11_context_equal(&c->x11_from_vc, &converted) || !x11_context_equal(&c->x11_from_xorg, &converted); } else @@ -636,6 +642,15 @@ static int method_set_x11_keyboard(sd_bus_message *m, void *userdata, sd_bus_err return sd_bus_error_set_errnof(error, r, "Failed to convert keymap data: %m"); } + if (vc_context_isempty(&converted)) + /* 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\".", strempty(in.layout)); + else + log_info("The X11 keyboard layout '%s' is converted to virtual console keymap '%s'", + in.layout, converted.keymap); + /* save the result of conversion to emit changed properties later. */ convert = !vc_context_equal(&c->vc, &converted); } From 7bb322e9824ca5b5068e96ebe1190c8f5d40cc08 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 26 Jan 2023 23:31:10 +0900 Subject: [PATCH 10/11] locale: downgrade level of one more log message --- src/locale/localed-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index f5cc46c836..a64adcd9ca 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -567,7 +567,7 @@ static int read_next_mapping( length = strv_length(b); if (length < min_fields || length > max_fields) { - log_warning("Invalid line %s:%u, ignoring.", strna(filename), *n); + log_debug("Invalid line %s:%u, ignoring.", strna(filename), *n); continue; } From 21a9c9262546fa1b1d12cc8edb613aac378d1457 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 26 Jan 2023 23:09:08 +0900 Subject: [PATCH 11/11] test: add tests for localectl --no-convert --- test/units/testsuite-73.sh | 122 +++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/test/units/testsuite-73.sh b/test/units/testsuite-73.sh index 92576c5067..d8f668513c 100755 --- a/test/units/testsuite-73.sh +++ b/test/units/testsuite-73.sh @@ -431,12 +431,134 @@ XKBMODEL=pc105+inet" assert_not_in "X11 Options:" "$output" } +test_convert() { + if [[ -z "$(localectl list-keymaps)" ]]; then + echo "No vconsole keymap installed, skipping test." + return + fi + + if [[ -z "$(localectl list-x11-keymap-layouts)" ]]; then + echo "No x11 keymap installed, skipping test." + return + fi + + backup_keymap + trap restore_keymap RETURN + + # clear previous settings + systemctl stop systemd-localed.service + wait_vconsole_setup + rm -f /etc/vconsole.conf /etc/X11/xorg.conf.d/00-keyboard.conf /etc/default/keyboard + + # set VC keymap without conversion + assert_rc 0 localectl --no-convert set-keymap us + output=$(localectl) + + # check VC keymap + vc=$(cat /etc/vconsole.conf) + assert_in "KEYMAP=us" "$vc" + assert_in "VC Keymap: us" "$output" + + # check VC -> X11 keymap conversion (nothing set) + 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_not_in "XKBLAYOUT=" "$vc" + assert_not_in "XKBMODEL=" "$vc" + assert_not_in "XKBVARIANT=" "$vc" + assert_not_in "XKBOPTIONS=" "$vc" + + # set VC keymap with conversion + assert_rc 0 localectl set-keymap us + output=$(localectl) + + # check VC keymap + vc=$(cat /etc/vconsole.conf) + assert_in "KEYMAP=us" "$vc" + assert_in "VC Keymap: us" "$output" + + # check VC -> X11 keymap conversion + assert_in "X11 Layout: us" "$output" + assert_in "X11 Model: pc105\+inet" "$output" + assert_not_in "X11 Variant:" "$output" + assert_in "X11 Options: terminate:ctrl_alt_bksp" "$output" + + assert_in "XKBLAYOUT=us" "$vc" + assert_in "XKBMODEL=pc105\+inet" "$vc" + assert_not_in "XKBVARIANT" "$vc" + assert_in "XKBOPTIONS=terminate:ctrl_alt_bksp" "$vc" + + # clear previous settings + systemctl stop systemd-localed.service + wait_vconsole_setup + rm -f /etc/vconsole.conf /etc/X11/xorg.conf.d/00-keyboard.conf /etc/default/keyboard + + # set x11 keymap (layout) without conversion + assert_rc 0 localectl --no-convert set-x11-keymap us + + assert_not_in "KEYMAP=" "$(cat /etc/vconsole.conf)" + assert_in "VC Keymap: .unset." "$(localectl)" + + if [[ -f /etc/default/keyboard ]]; then + assert_eq "$(cat /etc/default/keyboard)" "XKBLAYOUT=us" + else + output=$(cat /etc/X11/xorg.conf.d/00-keyboard.conf) + assert_in 'Option "XkbLayout" "us"' "$output" + assert_not_in 'Option "XkbModel"' "$output" + assert_not_in 'Option "XkbVariant"' "$output" + assert_not_in 'Option "XkbOptions"' "$output" + + output=$(cat /etc/vconsole.conf) + assert_in 'XKBLAYOUT=us' "$output" + assert_not_in 'XKBMODEL=' "$output" + assert_not_in 'XKBVARIANT=' "$output" + assert_not_in 'XKBOPTIONS=' "$output" + fi + + output=$(localectl) + assert_in "X11 Layout: us" "$output" + assert_not_in "X11 Model:" "$output" + assert_not_in "X11 Variant:" "$output" + assert_not_in "X11 Options:" "$output" + + # set x11 keymap (layout, model) with conversion + assert_rc 0 localectl set-x11-keymap us + + assert_in "KEYMAP=us" "$(cat /etc/vconsole.conf)" + assert_in "VC Keymap: us" "$(localectl)" + + if [[ -f /etc/default/keyboard ]]; then + assert_eq "$(cat /etc/default/keyboard)" "XKBLAYOUT=us" + else + output=$(cat /etc/X11/xorg.conf.d/00-keyboard.conf) + assert_in 'Option "XkbLayout" "us"' "$output" + assert_not_in 'Option "XkbModel"' "$output" + assert_not_in 'Option "XkbVariant"' "$output" + assert_not_in 'Option "XkbOptions"' "$output" + + output=$(cat /etc/vconsole.conf) + assert_in 'XKBLAYOUT=us' "$output" + assert_not_in 'XKBMODEL=' "$output" + assert_not_in 'XKBVARIANT=' "$output" + assert_not_in 'XKBOPTIONS=' "$output" + fi + + output=$(localectl) + assert_in "X11 Layout: us" "$output" + assert_not_in "X11 Model:" "$output" + assert_not_in "X11 Variant:" "$output" + assert_not_in "X11 Options:" "$output" +} + : >/failed enable_debug test_locale test_vc_keymap test_x11_keymap +test_convert touch /testok rm /failed