[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] monitor: Fix warning from clang
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] monitor: Fix warning from clang |
Date: |
Fri, 17 Aug 2012 16:41:34 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) |
Luiz Capitulino <address@hidden> writes:
> On Fri, 17 Aug 2012 16:10:12 +0200
> Markus Armbruster <address@hidden> wrote:
>
>> Stefan Weil <address@hidden> 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 <address@hidden>
>> > ---
>> > 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.
>> 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?