Re: [Qemu-devel] [PATCH] gic: avoid a warning from clang

[Blue Swirl]

On Mon, Sep 24, 2012 at 2:56 PM, Peter Maydell <address@hidden> wrote:
> On 24 September 2012 14:22, Peter Maydell <address@hidden> wrote:
>> On 23 September 2012 17:33, Blue Swirl <address@hidden> wrote:
>>> Avoid this warning:
>>>   CC    arm-softmmu/hw/arm/../arm_gic.o
>>> /src/qemu/hw/arm/../arm_gic.c:432:17: error: implicit truncation from 
>>> 'unsigned int' to bitfield changes value from 4294967040 to 0 
>>> [-Werror,-Wconstant-conversion]
>>>                 GIC_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
>>>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> /src/qemu/hw/arm/../arm_gic_internal.h:43:62: note: expanded from:
>>> #define GIC_CLEAR_PENDING(irq, cm) s->irq_state[irq].pending &= ~(cm)
>>>                                                              ^  ~~~~~
>>>
>>> 4294967040 is 0xffffff00 and field 'pending' is effectively 8 bits
>>> wide, so the masking has no effect except for avoiding the warning.
>>
>> foo &= ~SOME_FLAGS; is an entirely legitimate and common C idiom,
>> and I think clang is being overexuberant in warning here: we should
>> disable this warning instead of working around it in the code.

This is the only warning generated from QEMU sources, related to
-Wconstant-conversion (enabled by -Wall). It would be nice to work
around it. How about changing the macros to functions? The use of 's'
in the macros look bad and there's no do {} while(0) either to protect
the assignment.

Similar warning problems exist with -Winitializer-overrides and
-Wunused-value (also enabled by -Wall) though and there would be a lot
more fixing to do to avoid those.

> Also, what version of clang are you using? I don't see this warning
> either with MacOS X "Apple clang version 4.0 (tags/Apple/clang-421.0.60)
> (based on LLVM 3.1svn)" or with "Ubuntu clang version 3.0-6ubuntu3
> (tags/RELEASE_30/final) (based on LLVM 3.0)".

$ clang -v
Debian clang version 3.0-6 (tags/RELEASE_30/final) (based on LLVM 3.0)
Target: x86_64-pc-linux-gnu
Thread model: posix

I think this should be the same as Ubuntu.

>
> -- PMM