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
