| Message ID | 20221220085100.22848-6-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cppcheck rule 20.7 fixes | expand |
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
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,
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)); \
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(-)