Am 17.08.2012 17:02, schrieb Luiz Capitulino:
On Fri, 17 Aug 2012 16:41:34 +0200
Markus Armbruster <arm...@redhat.com> wrote:
Luiz Capitulino <lcapitul...@redhat.com> writes:
On Fri, 17 Aug 2012 16:10:12 +0200
Markus Armbruster <arm...@redhat.com> wrote:
Stefan Weil <s...@weilnetz.de> writes:
ccc-analyzer reports these warnings:
monitor.c:3532:21: warning: Division by zero
val %= val2;
^
monitor.c:3530:21: warning: Division by zero
val /= val2;
^
Rewriting the code fixes this (and also a style issue).
I'm afraid this doesn't actually fix anything, because...
Signed-off-by: Stefan Weil <s...@weilnetz.de>
---
monitor.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/monitor.c b/monitor.c
index 0c34934..0ea2c14 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3524,12 +3524,13 @@ static int64_t expr_prod(Monitor *mon)
break;
case '/':
case '%':
- if (val2 == 0)
+ if (val2 == 0) {
expr_error(mon, "division by zero");
- if (op == '/')
+ } else if (op == '/') {
val /= val2;
- else
+ } else {
val %= val2;
+ }
break;
}
}
... expr_error() longjmp()s out. The expression evaluator commonly
exploits that.
And that's correct. As far far I understood it's fixing clang, not qemu.
If expr_error() returned, the code would be just as wrong after your
patch as before.
Hmm, how? It checks for val2 == 0 first.
It would evaluate A % 0 into A, which is wrong.
Oh, you're talking about the result that would be returned by expr_prod().
I thought you were saying that val2 == 0 was still possible.
Perhaps the checker can be shut up by making expr_error() QEMU_NORETURN.
That's indeed a better solution.
Stefan, could you try that for us?
Adding QEMU_NORETURN to function expr_error also
fixes the warning from ccc-analyzer.
I'll send a patch series which adds this and some more
QEMU_NORETURN attributes.
What about using above patch in addition? IMHO it
improves readability, and it fixes the coding style.
Regards,
Stefan W.