From 390bab53923eea822ac525d0e4aa4b7f86d8cd92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 7 Nov 2024 15:45:24 +0100 Subject: [PATCH 1/3] tmpfiles: reduce quoting in warning message We printed: systemd-tmpfiles[705]: /usr/lib/tmpfiles.d/20-systemd-shell-extra.conf:10: Unknown modifiers in command 'L$'. systemd-tmpfiles[705]: /usr/lib/tmpfiles.d/systemd-network.conf:10: Unknown modifiers in command 'd$'. systemd-tmpfiles[705]: /usr/lib/tmpfiles.d/systemd-network.conf:11: Unknown modifiers in command 'd$'. ... There's a lot of additional characters here make the message harder to parse. We know that the command is a word without any whitespace, so quoting isn't really necessary. Change this to: ... unknown modifiers in command: L$ --- src/tmpfiles/tmpfiles.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 86bf16356d..bff05cda6f 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -3684,7 +3684,7 @@ static int parse_line( else { *invalid_config = true; return log_syntax(NULL, LOG_ERR, fname, line, SYNTHETIC_ERRNO(EBADMSG), - "Unknown modifiers in command '%s'.", action); + "Unknown modifiers in command: %s", action); } if (boot && !arg_boot) { From a814fd7897057c53d74d5b8d3c3c4dbd2be0861e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 13 Nov 2024 13:10:57 +0100 Subject: [PATCH 2/3] vconsole-setup: use normal variables instead of an array We had this elaborate scheme with an array of strings instead of a bunch of a normal string fields. If there were hundreds of those strings, this would make sense. But we had just five and one was actually a bit different because it had a fallback, so overall, the code is easier to read when normal fields are used. The fallback was implemented in the accessor function, now it's actually implemented in the place where it's used. While at it, rename the variables so that they match the config keys for legibility. --- src/vconsole/vconsole-setup.c | 147 ++++++++++++++-------------------- 1 file changed, 61 insertions(+), 86 deletions(-) diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c index ba742dda69..a2d3938dc3 100644 --- a/src/vconsole/vconsole-setup.c +++ b/src/vconsole/vconsole-setup.c @@ -39,50 +39,32 @@ #include "terminal-util.h" #include "virt.h" -typedef enum VCMeta { - VC_KEYMAP, - VC_KEYMAP_TOGGLE, - VC_FONT, - VC_FONT_MAP, - VC_FONT_UNIMAP, - _VC_META_MAX, - _VC_META_INVALID = -EINVAL, -} VCMeta; - typedef struct Context { - char *config[_VC_META_MAX]; + char *keymap; + char *keymap_toggle; + char *font; + char *font_map; + char *font_unimap; } Context; -static const char * const vc_meta_names[_VC_META_MAX] = { - [VC_KEYMAP] = "vconsole.keymap", - [VC_KEYMAP_TOGGLE] = "vconsole.keymap_toggle", - [VC_FONT] = "vconsole.font", - [VC_FONT_MAP] = "vconsole.font_map", - [VC_FONT_UNIMAP] = "vconsole.font_unimap", -}; - -/* compatibility with obsolete multiple-dot scheme */ -static const char * const vc_meta_compat_names[_VC_META_MAX] = { - [VC_KEYMAP_TOGGLE] = "vconsole.keymap.toggle", - [VC_FONT_MAP] = "vconsole.font.map", - [VC_FONT_UNIMAP] = "vconsole.font.unimap", -}; - -static const char * const vc_env_names[_VC_META_MAX] = { - [VC_KEYMAP] = "KEYMAP", - [VC_KEYMAP_TOGGLE] = "KEYMAP_TOGGLE", - [VC_FONT] = "FONT", - [VC_FONT_MAP] = "FONT_MAP", - [VC_FONT_UNIMAP] = "FONT_UNIMAP", -}; - static void context_done(Context *c) { assert(c); - FOREACH_ARRAY(cc, c->config, _VC_META_MAX) - free(*cc); + free(c->keymap); + free(c->keymap_toggle); + free(c->font); + free(c->font_map); + free(c->font_unimap); } +#define context_merge(dst, src, src_compat, name) \ + ({ \ + if (src->name) \ + free_and_replace(dst->name, src->name); \ + else if (src_compat && src_compat->name) \ + free_and_replace(dst->name, src_compat->name); \ + }) + static void context_merge_config( Context *dst, Context *src, @@ -91,21 +73,11 @@ static void context_merge_config( assert(dst); assert(src); - for (VCMeta i = 0; i < _VC_META_MAX; i++) - if (src->config[i]) - free_and_replace(dst->config[i], src->config[i]); - else if (src_compat && src_compat->config[i]) - free_and_replace(dst->config[i], src_compat->config[i]); -} - -static const char* context_get_config(Context *c, VCMeta meta) { - assert(c); - assert(meta >= 0 && meta < _VC_META_MAX); - - if (meta == VC_KEYMAP) - return isempty(c->config[VC_KEYMAP]) ? SYSTEMD_DEFAULT_KEYMAP : c->config[VC_KEYMAP]; - - return empty_to_null(c->config[meta]); + context_merge(dst, src, src_compat, keymap); + context_merge(dst, src, src_compat, keymap_toggle); + context_merge(dst, src, src_compat, font); + context_merge(dst, src, src_compat, font_map); + context_merge(dst, src, src_compat, font_unimap); } static int context_read_creds(Context *c) { @@ -115,11 +87,11 @@ static int context_read_creds(Context *c) { assert(c); r = read_credential_strings_many( - vc_meta_names[VC_KEYMAP], &v.config[VC_KEYMAP], - vc_meta_names[VC_KEYMAP_TOGGLE], &v.config[VC_KEYMAP_TOGGLE], - vc_meta_names[VC_FONT], &v.config[VC_FONT], - vc_meta_names[VC_FONT_MAP], &v.config[VC_FONT_MAP], - vc_meta_names[VC_FONT_UNIMAP], &v.config[VC_FONT_UNIMAP]); + "vconsole.keymap", &v.keymap, + "vconsole.keymap_toggle", &v.keymap_toggle, + "vconsole.font", &v.font, + "vconsole.font_map", &v.font_map, + "vconsole.font_unimap", &v.font_unimap); if (r < 0) log_warning_errno(r, "Failed to import credentials, ignoring: %m"); @@ -135,11 +107,11 @@ static int context_read_env(Context *c) { r = parse_env_file( NULL, "/etc/vconsole.conf", - vc_env_names[VC_KEYMAP], &v.config[VC_KEYMAP], - vc_env_names[VC_KEYMAP_TOGGLE], &v.config[VC_KEYMAP_TOGGLE], - vc_env_names[VC_FONT], &v.config[VC_FONT], - vc_env_names[VC_FONT_MAP], &v.config[VC_FONT_MAP], - vc_env_names[VC_FONT_UNIMAP], &v.config[VC_FONT_UNIMAP]); + "KEYMAP", &v.keymap, + "KEYMAP_TOGGLE", &v.keymap_toggle, + "FONT", &v.font, + "FONT_MAP", &v.font_map, + "FONT_UNIMAP", &v.font_unimap); if (r < 0) { if (r != -ENOENT) log_warning_errno(r, "Failed to read /etc/vconsole.conf, ignoring: %m"); @@ -158,14 +130,15 @@ static int context_read_proc_cmdline(Context *c) { r = proc_cmdline_get_key_many( PROC_CMDLINE_STRIP_RD_PREFIX, - vc_meta_names[VC_KEYMAP], &v.config[VC_KEYMAP], - vc_meta_names[VC_KEYMAP_TOGGLE], &v.config[VC_KEYMAP_TOGGLE], - vc_meta_names[VC_FONT], &v.config[VC_FONT], - vc_meta_names[VC_FONT_MAP], &v.config[VC_FONT_MAP], - vc_meta_names[VC_FONT_UNIMAP], &v.config[VC_FONT_UNIMAP], - vc_meta_compat_names[VC_KEYMAP_TOGGLE], &w.config[VC_KEYMAP_TOGGLE], - vc_meta_compat_names[VC_FONT_MAP], &w.config[VC_FONT_MAP], - vc_meta_compat_names[VC_FONT_UNIMAP], &w.config[VC_FONT_UNIMAP]); + "vconsole.keymap", &v.keymap, + "vconsole.keymap_toggle", &v.keymap_toggle, + "vconsole.font", &v.font, + "vconsole.font_map", &v.font_map, + "vconsole.font_unimap", &v.font_unimap, + /* compatibility with obsolete multiple-dot scheme */ + "vconsole.keymap.toggle", &w.keymap_toggle, + "vconsole.font.map", &w.font_map, + "vconsole.font.unimap", &w.font_unimap); if (r < 0) { if (r != -ENOENT) log_warning_errno(r, "Failed to read /proc/cmdline, ignoring: %m"); @@ -286,7 +259,7 @@ static int toggle_utf8_sysfs(bool utf8) { } static int keyboard_load_and_wait(const char *vc, Context *c, bool utf8) { - const char *map, *map_toggle, *args[8]; + const char* args[8]; unsigned i = 0; pid_t pid; int r; @@ -294,11 +267,12 @@ static int keyboard_load_and_wait(const char *vc, Context *c, bool utf8) { assert(vc); assert(c); - map = context_get_config(c, VC_KEYMAP); - map_toggle = context_get_config(c, VC_KEYMAP_TOGGLE); + const char + *keymap = empty_to_null(c->keymap) ?: SYSTEMD_DEFAULT_KEYMAP, + *keymap_toggle = empty_to_null(c->keymap_toggle); /* An empty map means kernel map */ - if (isempty(map) || streq(map, "@kernel")) + if (!keymap || streq(keymap, "@kernel")) return 0; args[i++] = KBD_LOADKEYS; @@ -307,9 +281,9 @@ static int keyboard_load_and_wait(const char *vc, Context *c, bool utf8) { args[i++] = vc; if (utf8) args[i++] = "-u"; - args[i++] = map; - if (map_toggle) - args[i++] = map_toggle; + args[i++] = keymap; + if (keymap_toggle) + args[i++] = keymap_toggle; args[i++] = NULL; if (DEBUG_LOGGING) { @@ -331,7 +305,7 @@ static int keyboard_load_and_wait(const char *vc, Context *c, bool utf8) { } static int font_load_and_wait(const char *vc, Context *c) { - const char *font, *map, *unimap, *args[9]; + const char* args[9]; unsigned i = 0; pid_t pid; int r; @@ -339,24 +313,25 @@ static int font_load_and_wait(const char *vc, Context *c) { assert(vc); assert(c); - font = context_get_config(c, VC_FONT); - map = context_get_config(c, VC_FONT_MAP); - unimap = context_get_config(c, VC_FONT_UNIMAP); + const char + *font = empty_to_null(c->font), + *font_map = empty_to_null(c->font_map), + *font_unimap = empty_to_null(c->font_unimap); /* Any part can be set independently */ - if (!font && !map && !unimap) + if (!font && !font_map && !font_unimap) return 0; args[i++] = KBD_SETFONT; args[i++] = "-C"; args[i++] = vc; - if (map) { + if (font_map) { args[i++] = "-m"; - args[i++] = map; + args[i++] = font_map; } - if (unimap) { + if (font_unimap) { args[i++] = "-u"; - args[i++] = unimap; + args[i++] = font_unimap; } if (font) args[i++] = font; @@ -377,7 +352,7 @@ static int font_load_and_wait(const char *vc, Context *c) { _exit(EXIT_FAILURE); } - /* setfont returns EX_OSERR when ioctl(KDFONTOP/PIO_FONTX/PIO_FONTX) fails. This might mean various + /* setfont returns EX_OSERR when ioctl(KDFONTOP/PIO_FONTX/PIO_FONTX) fails. This might mean various * things, but in particular lack of a graphical console. Let's be generous and not treat this as an * error. */ r = wait_for_terminate_and_check(KBD_SETFONT, pid, WAIT_LOG_ABNORMAL); From f393b2f99f8405754d698359b0e511ed69022299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 13 Nov 2024 13:19:39 +0100 Subject: [PATCH 3/3] vconsole-setup: drop impossible condition --- src/vconsole/vconsole-setup.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c index a2d3938dc3..3fa0cbba61 100644 --- a/src/vconsole/vconsole-setup.c +++ b/src/vconsole/vconsole-setup.c @@ -258,6 +258,9 @@ static int toggle_utf8_sysfs(bool utf8) { return 0; } +/* SYSTEMD_DEFAULT_KEYMAP must not be empty */ +assert_cc(STRLEN(SYSTEMD_DEFAULT_KEYMAP) > 0); + static int keyboard_load_and_wait(const char *vc, Context *c, bool utf8) { const char* args[8]; unsigned i = 0; @@ -271,8 +274,7 @@ static int keyboard_load_and_wait(const char *vc, Context *c, bool utf8) { *keymap = empty_to_null(c->keymap) ?: SYSTEMD_DEFAULT_KEYMAP, *keymap_toggle = empty_to_null(c->keymap_toggle); - /* An empty map means kernel map */ - if (!keymap || streq(keymap, "@kernel")) + if (streq(keymap, "@kernel")) return 0; args[i++] = KBD_LOADKEYS;