Re: [PATCH 09/12] x86/shadow: Rework write_atomic() call in shadow_write_entries()

[Roberto Bagnara <roberto.bagnara@xxxxxxxxxxx>]

On 20/02/26 22:46, Andrew Cooper wrote:
> Eclair complains of a side effect in a sizeof() expression (R13.6).
I disagree with comments of the form "Eclair complains" used whereas
the right thing to say is, e.g.:

  sizeof() expressions with "potential side effects" violate
  MISRA C:2012 + AMD2 Rule 13.6, for which each function call
  is a potential side effect

or

  Take out potential side effects from sizeof() as mandated by
  MISRA C:2012 + AMD2 Rule 13.6.

Note that in recent versions of MISRA C that rule is no longer
mandatory.  More generally, note also that, IMHO, switching to
a more modern version of MISRA C would simplify compliance.

BTW: the correct spelling is "ECLAIR", all capitals.

> write_atomic() only evaluates each parameter once, but rewrite the expression
> to less resemble entries in an obfuscation contest.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: consulting@xxxxxxxxxxx <consulting@xxxxxxxxxxx>
> CC: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> ---
>   xen/arch/x86/mm/shadow/set.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/mm/shadow/set.c b/xen/arch/x86/mm/shadow/set.c
> index 8b670b6bb555..96ba2811077e 100644
> --- a/xen/arch/x86/mm/shadow/set.c
> +++ b/xen/arch/x86/mm/shadow/set.c
> @@ -62,8 +62,8 @@ shadow_write_entries(void *d, const void *s, unsigned int 
> entries, mfn_t mfn)
>   
>       ASSERT(IS_ALIGNED((unsigned long)dst, sizeof(*dst)));
>   
> -    for ( ; i < entries; i++ )
> -        write_atomic(&dst++->l1, src++->l1);
> +    for ( ; i < entries; i++, dst++, src++ )
> +        write_atomic(&dst->l1, src->l1);
>   
>       unmap_domain_page(map);
>   }