Re: [PATCH v3 1/3] x86/APIC: include full string with error_interrupt() error messages

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.03.2023 20:45, Elliott Mitchell wrote:
> Rather than adding ", " with each printf(), simply include them in the
> string initially.  This allows converting to strlcat() or other methods
> which strictly concatenate, rather than formatting.
> 
> Signed-off-by: Elliott Mitchell <ehem+xen@xxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Nevertheless I wonder ...

> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -1401,14 +1401,14 @@ static void cf_check spurious_interrupt(struct 
> cpu_user_regs *regs)
>  static void cf_check error_interrupt(struct cpu_user_regs *regs)
>  {
>      static const char *const esr_fields[] = {
> -        "Send CS error",
> -        "Receive CS error",
> -        "Send accept error",
> -        "Receive accept error",
> -        "Redirectable IPI",
> -        "Send illegal vector",
> -        "Received illegal vector",
> -        "Illegal register address",
> +        ", Send CS error",
> +        ", Receive CS error",
> +        ", Send accept error",
> +        ", Receive accept error",
> +        ", Redirectable IPI",
> +        ", Send illegal vector",
> +        ", Received illegal vector",
> +        ", Illegal register address",
>      };
>      unsigned int v, v1;
>      int i;
> @@ -1423,7 +1423,7 @@ static void cf_check error_interrupt(struct 
> cpu_user_regs *regs)
>              smp_processor_id(), v , v1);
>      for ( i = 7; i >= 0; --i )
>          if ( v1 & (1 << i) )
> -            printk(", %s", esr_fields[i]);
> +            printk("%s", esr_fields[i]);

... whether the extra level of indirection (by using %s) is then still
necessary: There are no % characters in any of the individual strings.
Then again iirc this goes away anyway in the next patch ...

Jan