* [Qemu-trivial] [PATCH] qemu-ga: Remove unreachable code after g_error
@ 2012-09-01 7:34 Stefan Weil
2012-09-03 16:49 ` [Qemu-trivial] [Qemu-devel] " Luiz Capitulino
2012-09-22 15:25 ` Stefan Hajnoczi
0 siblings, 2 replies; 5+ messages in thread
From: Stefan Weil @ 2012-09-01 7:34 UTC (permalink / raw)
Cc: qemu-trivial, Stefan Weil, qemu-devel
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;
}
ret = sigaction(SIGTERM, &sigact, NULL);
if (ret == -1) {
g_error("error configuring signal handler: %s", strerror(errno));
- return false;
}
return true;
--
1.7.10
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH] qemu-ga: Remove unreachable code after g_error
2012-09-01 7:34 [Qemu-trivial] [PATCH] qemu-ga: Remove unreachable code after g_error Stefan Weil
@ 2012-09-03 16:49 ` Luiz Capitulino
2012-09-03 17:02 ` Stefan Weil
2012-09-22 15:25 ` Stefan Hajnoczi
1 sibling, 1 reply; 5+ messages in thread
From: Luiz Capitulino @ 2012-09-03 16:49 UTC (permalink / raw)
To: Stefan Weil; +Cc: qemu-trivial, qemu-devel
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().
> }
> ret = sigaction(SIGTERM, &sigact, NULL);
> if (ret == -1) {
> g_error("error configuring signal handler: %s", strerror(errno));
> - return false;
> }
>
> return true;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH] qemu-ga: Remove unreachable code after g_error
2012-09-03 16:49 ` [Qemu-trivial] [Qemu-devel] " Luiz Capitulino
@ 2012-09-03 17:02 ` Stefan Weil
2012-09-03 17:57 ` Luiz Capitulino
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Weil @ 2012-09-03 17:02 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-trivial, qemu-devel
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.
Regards,
- sw
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH] qemu-ga: Remove unreachable code after g_error
2012-09-03 17:02 ` Stefan Weil
@ 2012-09-03 17:57 ` Luiz Capitulino
0 siblings, 0 replies; 5+ messages in thread
From: Luiz Capitulino @ 2012-09-03 17:57 UTC (permalink / raw)
To: Stefan Weil; +Cc: qemu-trivial, qemu-devel
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH] qemu-ga: Remove unreachable code after g_error
2012-09-01 7:34 [Qemu-trivial] [PATCH] qemu-ga: Remove unreachable code after g_error Stefan Weil
2012-09-03 16:49 ` [Qemu-trivial] [Qemu-devel] " Luiz Capitulino
@ 2012-09-22 15:25 ` Stefan Hajnoczi
1 sibling, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2012-09-22 15:25 UTC (permalink / raw)
To: Stefan Weil; +Cc: qemu-trivial, qemu-devel
On Sat, Sep 01, 2012 at 09:34:15AM +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 <sw@weilnetz.de>
> ---
> qemu-ga.c | 2 --
> 1 file changed, 2 deletions(-)
Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-09-22 15:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-01 7:34 [Qemu-trivial] [PATCH] qemu-ga: Remove unreachable code after g_error Stefan Weil
2012-09-03 16:49 ` [Qemu-trivial] [Qemu-devel] " Luiz Capitulino
2012-09-03 17:02 ` Stefan Weil
2012-09-03 17:57 ` Luiz Capitulino
2012-09-22 15:25 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).