On 28/08/2024 8:06 am, Michal Orzel wrote:
>
> On 28/08/2024 01:45, Andrew Cooper wrote:
>>
>> for_each_set_bit() allocates its own variable intentionally as loop-scope
>> only. Unfortunately, this causes the inner 'i' to shadow the outer 'i'.
> NIT: I'd mention it violates MISRA R5.3
Done.
>
>> Drop the outermost 'i' and 'vcpuid' variables, moving them into a more narrow
>> scope and correcting them to be unsigned which they should have been all
>> along.
>>
>> Fixes: 9429f1a6c475 ("ARM/vgic: Use for_each_set_bit() in vgic_to_sgi()")
>> Signed-off-by: Andrew Cooper <[email protected]>
>> ---
>> CC: Stefano Stabellini <[email protected]>
>> CC: Julien Grall <[email protected]>
>> CC: Volodymyr Babchuk <[email protected]>
>> CC: Bertrand Marquis <[email protected]>
>> CC: Michal Orzel <[email protected]>
>> ---
>> xen/arch/arm/vgic.c | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
>> index 8ffe099bcb5f..6ecd9146511c 100644
>> --- a/xen/arch/arm/vgic.c
>> +++ b/xen/arch/arm/vgic.c
>> @@ -468,8 +468,6 @@ bool vgic_to_sgi(struct vcpu *v, register_t sgir, enum
>> gic_sgi_mode irqmode,
>> int virq, const struct sgi_target *target)
>> {
>> struct domain *d = v->domain;
>> - int vcpuid;
>> - int i;
>> unsigned int base, bitmap;
>>
>> ASSERT( virq < 16 );
>> @@ -483,7 +481,8 @@ bool vgic_to_sgi(struct vcpu *v, register_t sgir, enum
>> gic_sgi_mode irqmode,
>>
>> for_each_set_bit ( i, bitmap )
>> {
>> - vcpuid = base + i;
>> + unsigned int vcpuid = base + i;
> With this change you should replace the printk specifier from %d to %u for
> vcpuid.
So I should, yes.
>
> Apart from that:
> Reviewed-by: Michal Orzel <[email protected]>
Thanks. Sorry for breaking CI.
I did run Eclair on it, but for the manually triggered run, it only
generates a warning, and does not trigger a "pipeline {failed,fixed}"
event, because the pipeline hasn't changed state from the last time I
ran it.
I'm not sure if we can do any better for something which is
conditionally manually triggered.
~Andrew
