Re: [XEN PATCH v2 3/3] xen: address violations of MISRA C:2012 Rule 13.1

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.11.2023 18:29, Simone Ballarin wrote:
> Rule 13.1: Initializer lists shall not contain persistent side effects
> 
> The assignment operation in:
> 
> .irq = rc = uart->irq,
> 
> is a persistent side effect in a struct initializer list.
> 
> This patch avoids rc assignment and directly uses uart->irq
> in the following if statement.
> 
> No functional changes.
> 
> Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@xxxxxxxxxxx>
> Signed-off-by: Simone Ballarin <simone.ballarin@xxxxxxxxxxx>

Who's the author of this patch? (Either the order of the SoB is wrong, or
there's a From: tag missing.)

> ---
> Changes in v2:
> - avoid assignment of rc;
> - drop changes in vcpu_yield(void).
> ---
>  xen/drivers/char/ns16550.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

This warrants a more specific subject prefix. Also there's only a single
violation being dealt with here.

> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -445,11 +445,13 @@ static void __init cf_check ns16550_init_postirq(struct 
> serial_port *port)
>              struct msi_info msi = {
>                  .sbdf = PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
>                                   uart->ps_bdf[2]),
> -                .irq = rc = uart->irq,
> +                .irq = uart->irq,
>                  .entry_nr = 1
>              };
>  
> -            if ( rc > 0 )
> +            rc = 0;
> +
> +            if ( uart->irq > 0 )
>              {
>                  struct msi_desc *msi_desc = NULL;

The fact that there's no functional change here isn't really obvious.
Imo you want to prove that to a reasonable degree in the description.

Jan