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

[Peter Maydell]

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.

> Signed-off-by: Blue Swirl <address@hidden>
> ---
>  hw/arm_gic_internal.h |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/hw/arm_gic_internal.h b/hw/arm_gic_internal.h
> index db4fad5..219aef3 100644
> --- a/hw/arm_gic_internal.h
> +++ b/hw/arm_gic_internal.h
> @@ -40,7 +40,8 @@
>  #define GIC_CLEAR_ENABLED(irq, cm) s->irq_state[irq].enabled &= ~(cm)
>  #define GIC_TEST_ENABLED(irq, cm) ((s->irq_state[irq].enabled & (cm)) != 0)
>  #define GIC_SET_PENDING(irq, cm) s->irq_state[irq].pending |= (cm)
> -#define GIC_CLEAR_PENDING(irq, cm) s->irq_state[irq].pending &= ~(cm)
> +#define GIC_CLEAR_PENDING(irq, cm)                      \
> +    s->irq_state[irq].pending &= ~(cm) & ALL_CPU_MASK
>  #define GIC_TEST_PENDING(irq, cm) ((s->irq_state[irq].pending & (cm)) != 0)
>  #define GIC_SET_ACTIVE(irq, cm) s->irq_state[irq].active |= (cm)
>  #define GIC_CLEAR_ACTIVE(irq, cm) s->irq_state[irq].active &= ~(cm)

This patch makes GIC_CLEAR_PENDING different from GIC_CLEAR_ACTIVE,
GIC_CLEAR_LEVEL and GIC_CLEAR_ENABLED, which is odd since the
operation should be identical for all those cases.

-- PMM