On Mon, 03 Sep 2012 19:02:20 +0200 Stefan Weil <[email protected]> wrote:
> Am 03.09.2012 18:49, schrieb Luiz Capitulino: > > On Sat, 1 Sep 2012 09:34:15 +0200 > > Stefan Weil <[email protected]> wrote: > > > >> Report from smatch: > >> qemu-ga.c:117 register_signal_handlers(11) info: ignoring unreachable code. > >> qemu-ga.c:122 register_signal_handlers(16) info: ignoring unreachable code. > >> > >> g_error calls abort which terminates the program. > >> > >> Signed-off-by: Stefan Weil <[email protected]> > >> --- > >> qemu-ga.c | 2 -- > >> 1 file changed, 2 deletions(-) > >> > >> diff --git a/qemu-ga.c b/qemu-ga.c > >> index 7623079..b747470 100644 > >> --- a/qemu-ga.c > >> +++ b/qemu-ga.c > >> @@ -114,12 +114,10 @@ static gboolean register_signal_handlers(void) > >> ret = sigaction(SIGINT, &sigact, NULL); > >> if (ret == -1) { > >> g_error("error configuring signal handler: %s", strerror(errno)); > >> - return false; > > Good catch, but we should really drop g_error() usage as qemu-ga will not > > fail gracefully otherwise (will leak the pidfile, for example). We either > > just drop g_error() or replace it by fprintf(). > > Isn't that a classical case of an error which should never occur, > something which could also be handled by an assert statement? > > I don't expect a graceful exit after such errors. If they occur, > that's something which must be fixed in the code. > > When I read the documentation of sigaction, I don't see how > it could fail with the given function arguments. > > Therefore I'd apply the patch as it is. Yes, taking a look at the sigaction() manpages shows you're obviously correct. Please, disregard what I've said.
