Re: [PATCH v3 2/3] xen: x86: irq: use do-while loop in create_irq()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.04.2025 03:17, Volodymyr Babchuk wrote:
> Changes in v3:
>  - Correct code style ("do {")
>  - Add comment describing why we need do { } while loop.
>    I prefer to leave do {} while because Nicola Vetrini
>    said that this approach might help with MISRA Rule 9.1
>    without needing an explicit initializer.

Just to mention it here as well - I still prefer the v1 form of the fix. Plus,
for my taste, ...

> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -264,15 +264,24 @@ void __init clear_irq_vector(int irq)
>  
>  int create_irq(nodeid_t node, bool grant_access)
>  {
> -    int irq, ret;
> +    int ret;
> +    int irq = nr_irqs_gsi;
>      struct irq_desc *desc;
>  
> -    for (irq = nr_irqs_gsi; irq < nr_irqs; irq++)
> -    {
> +    if ( irq >= nr_irqs )
> +        return -ENOSPC;
> +
> +    /*
> +     * do { } while loop is used here to convince gcc14 that 'desc' is
> +     * really assigned. Otherwise with -Og or -fcondition-coverage it
> +     * may throw an false error stating that 'desc' may be used before
> +     * initialization.
> +     */
> +    do {
>          desc = irq_to_desc(irq);
>          if (cmpxchg(&desc->arch.used, IRQ_UNUSED, IRQ_RESERVED) == 
> IRQ_UNUSED)
>             break;
> -    }
> +    } while ( ++irq < nr_irqs );

... the comment is now to verbose. See what I suggested as a comment for the
v1 change, as a very rough example.

Furthermore the question towards reporting the issue upstream still wasn't
answered. There really would want to be a reference to the bug report in the
description (or even the code comment) here.

Jan