Hi Julien, > On 24 Mar 2022, at 13:00, Julien Grall <jul...@xen.org> wrote: > > Hi Bertrand, > > On 24/03/2022 11:04, Bertrand Marquis wrote: >> cppcheck can be used to check Xen code quality. > > Is there anything we should be concerned of in the initial report?
Nothing major no. Michal will soon push a serie with a first set of fixes, mostly related to unused variables. > >> To create a report do "make cppcheck" on a built tree adding any options >> you added during the process you used to build xen (like CROSS_COMPILE >> or XEN_TARGET_ARCH). This will generate an xml report xen-cppcheck.xml. >> To create a html report do "make cppcheck-html" in the same way and a >> full report to be seen in a browser will be generated in >> cppcheck-htmlreport/index.html. >> For better results it is recommended to build your own cppcheck from the >> latest sources that you can find at [1]. >> Development and result analysis has been done with cppcheck 2.7. > > We want to write down the minimum version of cppcheck we are going to > support. My suggestion would be to use the version you picked for your > development. Agree, 2.7 (current stable) should be the one as we had some unstable behaviours with other versions. Should I put that in the main readme ? > >> The Makefile rule is searching for all C files which have been compiled >> (ie which have a generated .o file) and is running cppcheck on all of >> them using the current configuration of xen so only the code actually >> compiled is checked. >> A new tool is introduced to merge all cppcheck reports into one global >> report including all findings and removing duplicates. >> Some extra variables can be used to customize the report: >> - CPPCHECK can be used to give the full path to the cppcheck binary to >> use (default is to use the one from the standard path). >> - CPPCHECK_HTMLREPORT can be used to give the full path to >> cppcheck-htmlreport (default is to use the one from the standard path). >> This has been tested on several arm configurations (x86 should work but >> has not been tested). >> [1] https://cppcheck.sourceforge.io/ >> Signed-off-by: Bertrand Marquis <bertrand.marq...@arm.com> >> Signed-off-by: Michal Orzel <michal.or...@arm.com> >> --- >> .gitignore | 3 ++ >> xen/Makefile | 75 +++++++++++++++++++++++++++- >> xen/arch/arm/include/asm/processor.h | 4 +- >> xen/include/xen/config.h | 4 ++ >> xen/include/xen/kconfig.h | 5 ++ >> xen/tools/merge_cppcheck_reports.py | 73 +++++++++++++++++++++++++++ > > In the long term, I think it would be useful if we generate the report > regularly. We might be able to hook the script that generate > https://xenbits.xen.org/docs/. Agree > >> diff --git a/xen/arch/arm/include/asm/processor.h >> b/xen/arch/arm/include/asm/processor.h >> index 852b5f3c24..0b4ba73760 100644 >> --- a/xen/arch/arm/include/asm/processor.h >> +++ b/xen/arch/arm/include/asm/processor.h >> @@ -219,9 +219,11 @@ >> SCTLR_Axx_ELx_A | SCTLR_Axx_ELx_C |\ >> SCTLR_Axx_ELx_WXN | SCTLR_Axx_ELx_EE) >> -#if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffUL >> +#ifndef CPPCHECK >> +#if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffULL >> #error "Inconsistent SCTLR_EL2 set/clear bits" >> #endif >> +#endif > > Why is it necessary? The ULL was an attempt to fix but did not work. I will remove that in the next version. Here cppcheck preprocessor is wrongly generating the error for a reason that I could not understand, hence the ifdef. Cheers Bertrand > > Cheers, > > -- > Julien Grall
