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 <[email protected]>
> Signed-off-by: Simone Ballarin <[email protected]>
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