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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.10.2023 16:18, Simone Ballarin wrote:
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -1504,6 +1504,8 @@ long vcpu_yield(void)
>  {
>      struct vcpu * v=current;
>      spinlock_t *lock;
> +    domid_t domain_id;
> +    int vcpu_id;

While I realize that the field this variable is initialized from is
plain "int", I still think that being wrong means the new variables
here and below want to be "unsigned int".

> @@ -1515,7 +1517,9 @@ long vcpu_yield(void)
>  
>      SCHED_STAT_CRANK(vcpu_yield);
>  
> -    TRACE_2D(TRC_SCHED_YIELD, current->domain->domain_id, current->vcpu_id);
> +    domain_id = current->domain->domain_id;
> +    vcpu_id = current->vcpu_id;
> +    TRACE_2D(TRC_SCHED_YIELD, domain_id, vcpu_id);

If you touch this, I think you ought to also switch to using "v" in
place of "current" (making the supposed side effect go away aiui).

Yet then (for the further changes to this file) - what persistent
side effect does reading "current" have? Clarification of that is
part of the justification for this change, and hence ought to be
spelled out in the description.

> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -445,10 +445,12 @@ 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
>              };
>  
> +            rc = uart->irq;
> +
>              if ( rc > 0 )

If this needs transforming, I think we'd better switch it to

            rc = 0;

            if ( uart->irq > 0 )

thus matching what we have elsewhere in the function.

Jan