Re: [Qemu-devel] [PULL 00/41] QAPI patches for 2017-06-09

[Markus Armbruster]

Peter Maydell <address@hidden> writes:

> On 9 June 2017 at 14:46, Markus Armbruster <address@hidden> wrote:
>> The following changes since commit 64175afc695c0672876fbbfc31b299c86d562cb4:
>>
>>   arm_gicv3: Fix ICC_BPR1 reset value when EL3 not implemented (2017-06-07 
>> 17:21:44 +0100)
>>
>> are available in the git repository at:
>>
>>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2017-06-09
>>
>> for you to fetch changes up to 9f70c3652654d59a1cc0e2ef4cc5b0e541d93579:
>>
>>   tests/qdict: check more get_try_int() cases (2017-06-09 13:48:45 +0200)
>>
>> ----------------------------------------------------------------
>> QAPI patches for 2017-06-09
>>
>
> Some compilers don't like the assert(0) idiom used in qnum.c to
> say "code flow doesn't reach end of function":
>
> /home/petmay01/linaro/qemu-for-merges/qobject/qnum.c: In function
> ‘qnum_get_double’:
> /home/petmay01/linaro/qemu-for-merges/qobject/qnum.c:162:1: error:
> control reaches end of non-void function [-Werror=return-type]
>  }
>  ^
> /home/petmay01/linaro/qemu-for-merges/qobject/qnum.c: In function
> ‘qnum_to_string’:
> /home/petmay01/linaro/qemu-for-merges/qobject/qnum.c:200:1: error:
> control reaches end of non-void function [-Werror=return-type]
>  }
>  ^
>
> (probably older gcc; that was the w32 gcc compiler; one of
> patchew's build environments fails the same way).
>
> I think one common working idiom for this is:
> int foo(void) {
>    switch (whatever) {
>        case X:
>           return 0;
>        case Y:
>           return 1;
>        default:
>           g_assert_not_reached();
>    }
> }

Two differences: (1) push the "not reached" assertion into the switch,
and (2) use g_assert_not_reached().

In theory, (1) shouldn't affect the warning, because either the compiler
groks "not reached" or it doesn't, and if it doesn't, whether we insert
another jump on the way to the implicit return shouldn't matter.  But
we're talking about a possibly confused compiler here, so who knows.

If it's (2), then /usr/include/assert.h is likely broken.  Meh, but
having to work around broken stuff is par for the course.

Elsewhere in the file, we do

    assert(0);
    return $DUMMY_VALUE;

I'll stick to that for local consistency.

> Can you fix and resend the pullreq, please?

In a couple of minutes.  Sorry for the inconvenience.