Cppcheck has found a violation of rule 20.7 for the macro memset
about missing parenthesis for the "n" argument, while the parenthesis
are not mandatory because the argument is never used in an
expression, adding them will not harm code and readability, so fix
the finding adding parenthesis for the argument.

Eclair and coverity does not report this finding.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
 xen/arch/arm/include/asm/string.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/include/asm/string.h b/xen/arch/arm/include/asm/string.h
index b485e4904419..f1c87d215b0b 100644
--- a/xen/arch/arm/include/asm/string.h
+++ b/xen/arch/arm/include/asm/string.h
@@ -30,7 +30,7 @@ void __memzero(void *ptr, size_t n);
 
 #define memset(p, v, n)                                                 \
         ({                                                              \
-                void *__p = (p); size_t __n = n;                        \
+                void *__p = (p); size_t __n = (n);                      \
                 if ((__n) != 0) {                                       \
                         if (__builtin_constant_p((v)) && (v) == 0)      \
                                 __memzero((__p),(__n));                 \
-- 
2.17.1

Hi,

On 20/12/2022 08:50, Luca Fancellu wrote:
> Cppcheck has found a violation of rule 20.7 for the macro memset
> about missing parenthesis for the "n" argument, while the parenthesis
> are not mandatory because the argument is never used in an
> expression, adding them will not harm code and readability, so fix
> the finding adding parenthesis for the argument.

This is something I have argued against in the past (see [1]). So...

> 
> Eclair and coverity does not report this finding.

... if neither Eclair nor Coverity report it then I think this should be 
a bug report against Cppcheck.

Also, typo: s/does not/do not/

> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
>   xen/arch/arm/include/asm/string.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/include/asm/string.h b/xen/arch/arm/include/asm/string.h
> index b485e4904419..f1c87d215b0b 100644
> --- a/xen/arch/arm/include/asm/string.h
> +++ b/xen/arch/arm/include/asm/string.h
> @@ -30,7 +30,7 @@ void __memzero(void *ptr, size_t n);
>   
>   #define memset(p, v, n)                                                 \
>           ({                                                              \
> -                void *__p = (p); size_t __n = n;                        \
> +                void *__p = (p); size_t __n = (n);                      \
>                   if ((__n) != 0) {                                       \
>                           if (__builtin_constant_p((v)) && (v) == 0)      \
>                                   __memzero((__p),(__n));                 \

Cheers,

[1] 20220728134943.1185621-1-burzalodowa@gmail.com

-- 
Julien Grall

On 20.12.2022 10:12, Julien Grall wrote:
> On 20/12/2022 08:50, Luca Fancellu wrote:
>> Cppcheck has found a violation of rule 20.7 for the macro memset
>> about missing parenthesis for the "n" argument, while the parenthesis
>> are not mandatory because the argument is never used in an
>> expression, adding them will not harm code and readability, so fix
>> the finding adding parenthesis for the argument.
> 
> This is something I have argued against in the past (see [1]). So...
> 
>>
>> Eclair and coverity does not report this finding.
> 
> ... if neither Eclair nor Coverity report it then I think this should be 
> a bug report against Cppcheck.

Furthermore in reply to my "Arm32: tidy the memset() macro" you said [1]

"In this case, Linux has removed __memzero() is patch ff5fdafc9e97 "ARM: 
 8745/1: get rid of __memzero()" because the performance difference with 
 memset() was limited. For Xen, I think we should also remove the function."

So either you want to follow that route, or it would rather be my patch
which ought to be considered for merging, not the least because it also
deals with yet another MISRA violation.

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2022-08/msg01185.html

Hi Jan,

On 20/12/2022 09:38, Jan Beulich wrote:
> On 20.12.2022 10:12, Julien Grall wrote:
>> On 20/12/2022 08:50, Luca Fancellu wrote:
>>> Cppcheck has found a violation of rule 20.7 for the macro memset
>>> about missing parenthesis for the "n" argument, while the parenthesis
>>> are not mandatory because the argument is never used in an
>>> expression, adding them will not harm code and readability, so fix
>>> the finding adding parenthesis for the argument.
>>
>> This is something I have argued against in the past (see [1]). So...
>>
>>>
>>> Eclair and coverity does not report this finding.
>>
>> ... if neither Eclair nor Coverity report it then I think this should be
>> a bug report against Cppcheck.
> 
> Furthermore in reply to my "Arm32: tidy the memset() macro" you said [1]
> 
> "In this case, Linux has removed __memzero() is patch ff5fdafc9e97 "ARM:
>   8745/1: get rid of __memzero()" because the performance difference with
>   memset() was limited. For Xen, I think we should also remove the function."
> 
> So either you want to follow that route, or it would rather be my patch
> which ought to be considered for merging, not the least because it also
> deals with yet another MISRA violation.

I forgot that discussion, thanks for the reminder! I would still prefer 
if we port the Linux change to Xen.

Cheers,

-- 
Julien Grall