Re: [Xen-devel] [RFC 01/29] xen/arm: lr must be included in range [0-nr_lr[

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-04-29 at 00:01 +0100, Julien Grall wrote:

Typo in subject, [ at the end, did you mean ] or ) ? ( I can never
remember which one is inclusive)

> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
>  xen/arch/arm/gic.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index bc8faf2..bac2af2 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -512,7 +512,9 @@ static inline void gic_set_lr(int lr, unsigned int 
> virtual_irq,
>  {
>      int maintenance_int = GICH_LR_MAINTENANCE_IRQ;
>  
> -    BUG_ON(lr > nr_lrs);
> +    BUG_ON(lr >= nr_lrs);
> +    BUG_ON(lr < 0);

> +    BUG_ON(state & ~(GICH_LR_STATE_MASK<<GICH_LR_STATE_SHIFT));

This suggests that STATE_MASK is inconveniently defined, it'd be more
normal to include the SHIFT in the mask.

The same is true of all the other GICH_LR_*_MASK/SHIFT defines. The only
one which is used is GICH_LR_VIRTUAL_MASK+SHIFT, and I guess I can see
why in that case. 

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Ian.

>  
>      GICH[GICH_LR + lr] = state |
>          maintenance_int |



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel