[Qemu-devel] Re: [PATCH] tcg: Fix compiler error (comparison of unsigned expression)

[Stefan Weil <weil@mail.berlios.de>]

Am 08.10.2010 18:57, schrieb Hollis Blanchard:
> On Fri, Oct 8, 2010 at 1:32 AM, Stefan Weil <weil@mail.berlios.de> wrote:
>> When qemu is configured with --enable-debug-tcg,
>> gcc throws this warning (or error with -Werror):
>>
>> tcg/tcg.c:1030: error: comparison of unsigned expression >= 0 is 
>> always true
>>
>> Fix it by removing the >= 0 part.
>> The type cast to 'unsigned' catches negative values of op
>> (which should never happen).
>>
>> This is a modification of Hollis Blanchard's patch.
>>
>> Cc: Hollis Blanchard <hollis@penguinppc.org>
>> Cc: Blue Swirl <blauwirbel@gmail.com>
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>> ---
>>  tcg/tcg.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/tcg/tcg.c b/tcg/tcg.c
>> index e0a9030..0cdef0d 100644
>> --- a/tcg/tcg.c
>> +++ b/tcg/tcg.c
>> @@ -1027,7 +1027,7 @@ void tcg_add_target_add_op_defs(const 
>> TCGTargetOpDef *tdefs)
>>         if (tdefs->op == (TCGOpcode)-1)
>>             break;
>>         op = tdefs->op;
>> -        assert(op >= 0 && op < NB_OPS);
>> +        assert((unsigned)op < NB_OPS);
>>         def = &tcg_op_defs[op];
>>  #if defined(CONFIG_DEBUG_TCG)
>>         /* Duplicate entry in op definitions? */
>
> According to the warning, op is already unsigned, so this simply
> removes the >=0 test, which was my original patch.
>
> In contrast, Blue wanted a cast to int, as seen in
> 95ee3914bfd551aeec49932a400530141865acad.
>
> -Hollis

I read the original thread. Michael already proposed the
unsigned cast in that thread.

Your patch is correct as long as we use gcc and as long
as gcc uses unsigned enums when possible (it is possible here).

Other compilers or new versions of gcc might use signed enums.
For those (and also for current gcc versions), my patch works.
Blue's solution also works but is a bit more code without
having advantages.

There are even more solutions which are accepted by the
compiler: ((op > first && op <= last) || (op == first))
with first, last being the first or last enum member.

- Stefan