From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41680) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8atc-0005Lp-8L for qemu-devel@nongnu.org; Mon, 03 Sep 2012 13:57:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T8atb-0005Ko-Cf for qemu-devel@nongnu.org; Mon, 03 Sep 2012 13:57:12 -0400 Date: Mon, 3 Sep 2012 14:57:52 -0300 From: Luiz Capitulino Message-ID: <20120903145752.5edfafa5@doriath.home> In-Reply-To: <5044E29C.1030802@weilnetz.de> References: <1346484855-29649-1-git-send-email-sw@weilnetz.de> <20120903134920.2dff4a3d@doriath.home> <5044E29C.1030802@weilnetz.de> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qemu-ga: Remove unreachable code after g_error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: qemu-trivial@nongnu.org, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com On Mon, 03 Sep 2012 19:02:20 +0200 Stefan Weil wrote: > Am 03.09.2012 18:49, schrieb Luiz Capitulino: > > On Sat, 1 Sep 2012 09:34:15 +0200 > > Stefan Weil 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 > >> --- > >> 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.