Hi On Wed, Mar 11, 2015 at 5:12 PM, Lucas De Marchi <lucas.de.mar...@gmail.com> wrote: > On Wed, Mar 11, 2015 at 12:55 PM, David Herrmann <dh.herrm...@gmail.com> > wrote: >> Hi >> >> On Tue, Mar 10, 2015 at 8:56 PM, <lucas.de.mar...@gmail.com> wrote: >>> From: Lucas De Marchi <lucas.demar...@intel.com> >>> >>> 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 +++ >>> >>> Now systemd-vconsole-setup exits with EXIT_FAILURE if the child failed >>> for whatever reason. >>> --- >>> src/vconsole/vconsole-setup.c | 17 ++++++++++++----- >>> 1 file changed, 12 insertions(+), 5 deletions(-) >>> >>> diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c >>> index bf681d9..601ca4a 100644 >>> --- a/src/vconsole/vconsole-setup.c >>> +++ b/src/vconsole/vconsole-setup.c >>> @@ -304,8 +304,12 @@ int main(int argc, char **argv) { >>> return EXIT_FAILURE; >>> } >>> >>> - if (font_pid > 0) >>> - wait_for_terminate_and_warn(KBD_SETFONT, font_pid, true); >>> + if (font_pid > 0) { >>> + r = wait_for_terminate_and_warn(KBD_SETFONT, font_pid, >>> true); >>> + if (r != 0) >>> + return EXIT_FAILURE; >>> + } >>> + >> >> Looks mostly good. However, I'd prefer if we continue on "r > 0" but >> remember the error for later. This way, we still try loading the right >> keymap even though the font might have failed. > > Is there any reason to continue on r > 0 but not on r < 0? I'm ok with > changing the current behavior, but just like to know which one is > better.
Yeah, not sure here. For most helpers here r<0 means serious error (fork() failed, etc.), r>0 means just the spawned application failed. I guess we should continue for all !=0. That is, don't return prematurely at all. Thanks David _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel