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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 13 Nov 2019, Julien Grall wrote:
> 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.

That would be better actually

 
>       > ---
>       >  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
>       >
> 
> 
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel