From 8931278c8a9ef7b60f6b6963486a9e9febb20a0d Mon Sep 17 00:00:00 2001 From: Lucas De Marchi Date: Fri, 13 Mar 2015 17:47:28 +0000 Subject: [PATCH] vconsole-setup: check error of child process If we don't check the error of the child process, systemd-vconsole-setup would exit with 0 even if it could not really setup the console. For a simple test, move loadkeys elsewhere and execute systemd-vconsole-setup: [root@localhost ~]# strace -f -e execve /usr/lib/systemd/systemd-vconsole-setup execve("/usr/lib/systemd/systemd-vconsole-setup", ["/usr/lib/systemd/systemd-vconsol"...], [/* 15 vars */]) = 0 Process 171 attached [pid 171] execve("/usr/bin/loadkeys", ["/usr/bin/loadkeys", "-q", "-C", "/dev/tty0", "br-abnt2"], [/* 15 vars */]) = -1 ENOENT (No such file or directory) [pid 171] +++ exited with 1 +++ --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=171, si_uid=0, si_status=1, si_utime=0, si_stime=0} --- +++ exited with 0 +++ Note that loadkeys returned 1 while systemd-vconsole-setup return 0. Since the font and keyboard setup are already serialized, refactor the code a little bit so the functions do the wait by themselves. One change in behavior in this patch is that we don't return early, but we do try to setup the keyboard even if the font load failed. --- src/vconsole/vconsole-setup.c | 68 +++++++++++++---------------------- 1 file changed, 24 insertions(+), 44 deletions(-) diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c index bf681d9f3..fe8573f92 100644 --- a/src/vconsole/vconsole-setup.c +++ b/src/vconsole/vconsole-setup.c @@ -96,16 +96,14 @@ static int enable_utf8(int fd) { return r; } -static int keymap_load(const char *vc, const char *map, const char *map_toggle, bool utf8, pid_t *_pid) { +static bool keyboard_load_and_wait(const char *vc, const char *map, const char *map_toggle, bool utf8) { const char *args[8]; int i = 0; pid_t pid; - if (isempty(map)) { - /* An empty map means kernel map */ - *_pid = 0; - return 0; - } + /* An empty map means kernel map */ + if (isempty(map)) + return true; args[i++] = KBD_LOADKEYS; args[i++] = "-q"; @@ -119,27 +117,25 @@ static int keymap_load(const char *vc, const char *map, const char *map_toggle, args[i++] = NULL; pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - else if (pid == 0) { + if (pid < 0) { + log_error_errno(errno, "Failed to fork: %m"); + return false; + } else if (pid == 0) { execv(args[0], (char **) args); _exit(EXIT_FAILURE); } - *_pid = pid; - return 0; + return wait_for_terminate_and_warn(KBD_LOADKEYS, pid, true) == 0; } -static int font_load(const char *vc, const char *font, const char *map, const char *unimap, pid_t *_pid) { +static bool font_load_and_wait(const char *vc, const char *font, const char *map, const char *unimap) { const char *args[9]; int i = 0; pid_t pid; - if (isempty(font)) { - /* An empty font means kernel font */ - *_pid = 0; - return 0; - } + /* An empty font means kernel font */ + if (isempty(font)) + return true; args[i++] = KBD_SETFONT; args[i++] = "-C"; @@ -156,15 +152,15 @@ static int font_load(const char *vc, const char *font, const char *map, const ch args[i++] = NULL; pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - else if (pid == 0) { + if (pid < 0) { + log_error_errno(errno, "Failed to fork: %m"); + return false; + } else if (pid == 0) { execv(args[0], (char **) args); _exit(EXIT_FAILURE); } - *_pid = pid; - return 0; + return wait_for_terminate_and_warn(KBD_SETFONT, pid, true) == 0; } /* @@ -238,8 +234,7 @@ int main(int argc, char **argv) { *vc_font = NULL, *vc_font_map = NULL, *vc_font_unimap = NULL; _cleanup_close_ int fd = -1; bool utf8; - pid_t font_pid = 0, keymap_pid = 0; - bool font_copy = false; + bool font_copy = false, font_ok, keyboard_ok; int r = EXIT_FAILURE; log_set_target(LOG_TARGET_AUTO); @@ -298,27 +293,12 @@ int main(int argc, char **argv) { else disable_utf8(fd); - r = font_load(vc, vc_font, vc_font_map, vc_font_unimap, &font_pid); - if (r < 0) { - log_error_errno(r, "Failed to start " KBD_SETFONT ": %m"); - return EXIT_FAILURE; - } - - if (font_pid > 0) - wait_for_terminate_and_warn(KBD_SETFONT, font_pid, true); - - r = keymap_load(vc, vc_keymap, vc_keymap_toggle, utf8, &keymap_pid); - if (r < 0) { - log_error_errno(r, "Failed to start " KBD_LOADKEYS ": %m"); - return EXIT_FAILURE; - } - - if (keymap_pid > 0) - wait_for_terminate_and_warn(KBD_LOADKEYS, keymap_pid, true); + font_ok = font_load_and_wait(vc, vc_font, vc_font_map, vc_font_unimap); + keyboard_ok = keyboard_load_and_wait(vc, vc_keymap, vc_keymap_toggle, utf8); - /* Only copy the font when we started setfont successfully */ - if (font_copy && font_pid > 0) + /* Only copy the font when we executed setfont successfully */ + if (font_copy && font_ok) font_copy_to_all_vcs(fd); - return EXIT_SUCCESS; + return font_ok && keyboard_ok ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.30.2