On 02.12.2023 04:22, Stefano Stabellini wrote: > On Mon, 27 Nov 2023, Jan Beulich wrote: >> 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. > > Agreed. Only reading this chunk, wouldn't it be better to do: > > }; > > rc = uart->irq; > > if ( rc > 0 ) > > at least it would be obvious?
Indeed. Jan
