RE: [PATCH 2/2] Arm32: tidy the memset() macro

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Jan,

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan
> Beulich
> Sent: 2022年8月19日 15:50
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Julien Grall <julien@xxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>;
> Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> Subject: [PATCH 2/2] Arm32: tidy the memset() macro
> 
> - add parentheses where they were missing (MISRA)
> - make sure to evaluate also v exactly once (MISRA)
> - remove excess parentheses
> - rename local variables to not have leading underscores
> - apply Xen coding style
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> I wonder whether "if ( n_ )" is really helpful: It's extra code in all
> callers passing a non-constant size, just to cover a pretty rare case
> which memset() is required to deal with correctly anyway, and which

What rare case we need to use n_ that can make memset happy? As your
comment, I read the code again, but it seems to work fine without n_.

Cheers,
Wei Chen

> __memzero() also handles quite fine from all I can tell.
> 
> --- a/xen/arch/arm/include/asm/string.h
> +++ b/xen/arch/arm/include/asm/string.h
> @@ -28,17 +28,19 @@
> 
>  void __memzero(void *ptr, size_t n);
> 
> -#define memset(p, v, n)                                                 \
> -        ({                                                              \
> -                void *__p = (p); size_t __n = n;                        \
> -                if ((__n) != 0) {                                       \
> -                        if (__builtin_constant_p((v)) && (v) == 0)      \
> -                                __memzero((__p),(__n));                 \
> -                        else                                            \
> -                                memset((__p),(v),(__n));                \
> -                }                                                       \
> -                (__p);                                                  \
> -        })
> +#define memset(p, v, n)                                 \
> +    ({                                                  \
> +        void *p_ = (p); size_t n_ = (n);                \
> +        typeof(v) v_ = (v);                             \
> +        if ( n_ )                                       \
> +        {                                               \
> +            if ( __builtin_constant_p(v) && !v_ )       \
> +                __memzero(p_, n_);                      \
> +            else                                        \
> +                memset(p_, v_, n_);                     \
> +        }                                               \
> +        p_;                                             \
> +    })
> 
>  #endif
> 
>