qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] Fix compiler warning (always return a value)


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH] Fix compiler warning (always return a value)
Date: Fri, 28 Oct 2011 09:49:52 +0100

On Fri, Oct 28, 2011 at 8:40 AM, Markus Armbruster <address@hidden> wrote:
> Stefan Hajnoczi <address@hidden> writes:
>
>> On Mon, Oct 24, 2011 at 10:18:43PM +0200, Stefan Weil wrote:
>>> For compilations with -DNDEBUG, the default case did not return
>>> a value which caused a compiler warning.
>>>
>>> Signed-off-by: Stefan Weil <address@hidden>
>>> ---
>>>  hw/ppce500_spin.c |   11 ++++++++---
>>>  1 files changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c
>>> index cccd940..5b5ffe0 100644
>>> --- a/hw/ppce500_spin.c
>>> +++ b/hw/ppce500_spin.c
>>> @@ -168,17 +168,22 @@ static uint64_t spin_read(void *opaque, 
>>> target_phys_addr_t addr, unsigned len)
>>>  {
>>>      SpinState *s = opaque;
>>>      uint8_t *spin_p = &((uint8_t*)s->spin)[addr];
>>> +    uint64_t result = 0;
>>>
>>>      switch (len) {
>>>      case 1:
>>> -        return ldub_p(spin_p);
>>> +        result = ldub_p(spin_p);
>>> +        break;
>>>      case 2:
>>> -        return lduw_p(spin_p);
>>> +        result = lduw_p(spin_p);
>>> +        break;
>>>      case 4:
>>> -        return ldl_p(spin_p);
>>> +        result = ldl_p(spin_p);
>>> +        break;
>>>      default:
>>>          assert(0);
>>
>> I would replace assert(3) with abort(3).  If this ever happens the
>> program is broken - returning 0 instead of an undefined value doesn't
>> help.
>
> Why is it useful to make failed assertions stop the program regardless
> of NDEBUG only when the assertion happens to be "can't reach"?

My point is that it should not be an assertion.  The program has a
control flow path which should never be taken.  In any "safe"
programming environment the program will terminate with a diagnostic.

That's exactly what we need to do here too.  assert(3) is the wrong
tool for this; we're not even asserting anything.

Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]