On Fri, Oct 28, 2011 at 8:40 AM, Markus Armbruster <arm...@redhat.com> wrote: > Stefan Hajnoczi <stefa...@gmail.com> 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 <s...@weilnetz.de> >>> --- >>> 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