Re: [Qemu-trivial] [Qemu-devel] [PATCH] qemu-ga: Remove unreachable code after g_error

[Luiz Capitulino <lcapitulino@redhat.com>]

On Mon, 03 Sep 2012 19:02:20 +0200
Stefan Weil <sw@weilnetz.de> wrote:

> Am 03.09.2012 18:49, schrieb Luiz Capitulino:
> > On Sat,  1 Sep 2012 09:34:15 +0200
> > Stefan Weil <sw@weilnetz.de> 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 <sw@weilnetz.de>
> >> ---
> >>   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.