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

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

Am 08.10.2010 23:43, schrieb Stefan Weil:
> 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

Hollis, do you still see problems with my patch?
Or can it be committed?

Regards,
Stefan