Re: [PATCH v3 2/3] x86/APIC: modify error_interrupt() to output using single printk()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.03.2023 20:53, Elliott Mitchell wrote:
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -1410,6 +1410,7 @@ static void cf_check error_interrupt(struct 
> cpu_user_regs *regs)
>          ", Received illegal vector",
>          ", Illegal register address",
>      };
> +    const char *entries[ARRAY_SIZE(esr_fields)];
>      unsigned int v, v1;
>      int i;
>  
> @@ -1419,12 +1420,13 @@ static void cf_check error_interrupt(struct 
> cpu_user_regs *regs)
>      v1 = apic_read(APIC_ESR);
>      ack_APIC_irq();
>  
> -    printk(XENLOG_DEBUG "APIC error on CPU%u: %02x(%02x)",
> -            smp_processor_id(), v , v1);
>      for ( i = 7; i >= 0; --i )
> -        if ( v1 & (1 << i) )
> -            printk("%s", esr_fields[i]);
> -    printk("\n");
> +        entries[i] = v1 & (1 << i) ? esr_fields[i] : "";
> +    printk(XENLOG_DEBUG "APIC error on CPU%u: %02x(%02x)"
> +        "%s%s%s%s%s%s%s%s" "\n",
> +        smp_processor_id(), v , v1,
> +        entries[7], entries[6],
> +        entries[5], entries[4], entries[3], entries[2], entries[1], 
> entries[0]);

While pre-existing in both cases, two nits: There's a stray blank before one
of the commas, and indentation is wrong too.

Furthermore there's no reason to split the format string, especially not
ahead of the \n. Plus line wrapping for the 8 entries[] references could
also be done more evenly.

Since these are all cosmetic, I guess I'll do adjustments while committing:
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan