Xen project Mailing List

On Tue, 12 Nov 2019, 05:52 Stefano Stabellini, <sstabellini@xxxxxxxxxx> wrote:

On Wed, 6 Nov 2019, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@xxxxxxxx>
>
> Also armclang complains about the condition always true,
> because `sgi` is of type enum with all its values under 16.
>
> Signed-off-by: Andrii Anisov <andrii_anisov@xxxxxxxx>

Although I am not completely opposed to this, given the choice I would
prefer to keep the ASSERTs.

Why? What would that prevent? It is an enum, so unless you do an horrible hack on the other side, this should always be valid.

But then, why would this be an issue here and not in the tens other place where enum is used?

Given that I would imagine that the ARM C Compiler will also complain
about many other ASSERTs, I wonder if it wouldn't be better to just
disable *all* ASSERTs when building with armcc by changing the
implementation of the ASSERT MACRO.

ARM C compiler is valid here and I would not be surprised this will come up in Clang and GCC in the future.

If you are worry that the enum is going to grow more than 16 items, then you should use a BUILD_BUG_ON.

> ---
> xen/arch/arm/gic.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 113655a..58c6141 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -294,8 +294,6 @@ void __init gic_init(void)
>
> void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi)
> {
> - ASSERT(sgi < 16); /* There are only 16 SGIs */
> -
> gic_hw_ops->send_SGI(sgi, SGI_TARGET_LIST, cpumask);
> }
>
> @@ -306,15 +304,11 @@ void send_SGI_one(unsigned int cpu, enum gic_sgi sgi)
>
> void send_SGI_self(enum gic_sgi sgi)
> {
> - ASSERT(sgi < 16); /* There are only 16 SGIs */
> -
> gic_hw_ops->send_SGI(sgi, SGI_TARGET_SELF, NULL);
> }
>
> void send_SGI_allbutself(enum gic_sgi sgi)
> {
> - ASSERT(sgi < 16); /* There are only 16 SGIs */
> -
> gic_hw_ops->send_SGI(sgi, SGI_TARGET_OTHERS, NULL);
> }
>
> --
> 2.7.4
>

Re: [Xen-devel] [RFC 4/7] arm/gic: Drop pointless assertions