On 2024-06-01 15:08, Andrew Cooper wrote:
On 01/06/2024 1:58 pm, Nicola Vetrini wrote:
On 2024-06-01 14:47, Andrew Cooper wrote:
On 01/06/2024 11:16 am, Nicola Vetrini wrote:
ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure ahead of
rearrangements")
introduced new violations on previously clean rules 20.9 and 20.12.
The first is introduced because CONFIG_CC_IS_CLANG in
xen/self-tests.h is not
defined in the configuration under analysis. Using "defined()"
instead avoids
relying on the preprocessor's behaviour upon encountering an
undedfined identifier
and addresses the violation.
The violation of Rule 20.12 is due to "val" being used both as an
ordinary argument
in macro RUNTIME_CHECK, and as a stringification operator.
No functional change.
Fixes: ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure
ahead of rearrangements")
Signed-off-by: Nicola Vetrini <nicola.vetr...@bugseng.com>
Thankyou for this patch. I'd seen that I'd broken something.
(Entirely
my fault - I've done a lot of testing in Gitlab for the series, but
never manually ran the Eclair jobs. I'll try to remember better next
time.)
One question though.
https://gitlab.com/xen-project/xen/-/jobs/6994213979 says:
Failure: 1 regressions found for clean guidelines
service MC3R1.R20.9: (required) All identifiers used in the
controlling expression of `#if' or `#elif' preprocessing directives
shall be #define'd before evaluation:
violation: 1
While there is a report for 20.12, it's not clean yet (so the first
sentence wants adjusting), and RUNTIME_CHECK doesn't show up newly in
it.
So, while I agree that RUNTIME_CHECK() should be included in the
20.12
exclusions, why is current Gitlab Run not reporting it?
~Andrew
While Rule 20.12 wasn't clean on x86, but it was on arm, so in the
arm64 run it is reported as such
https://gitlab.com/xen-project/xen/-/jobs/6994213980
with the other patches in the series the rule should be clean on both
architectures, so this imbalance should disappear rather shortly.
Thanks - I'd just found the ARM report and was replying - I'll rebase
onto this thread.
I still don't understand why the violation doesn't show up on the x86
build. Specifically, why it's missing from here:
https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/xen/ECLAIR_normal/staging/X86_64/6994213979/prev/PROJECT.ecd;/by_service/MC3R1.R20.12.html
Note the "prev" here in the URL: this means you're looking at the
analysis run before your series was merged (on 03147e6837ff045db)
this is the latest run for x86 on staging:
https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/xen/ECLAIR_normal/staging/X86_64/6994213979/PROJECT.ecd;/by_service/MC3R1.R20.12.html
From the ARM report, one is xen/lib/generic-ffsl.c:61 and checking with
the x86 build log, we are building generic-ffsl.c too (which is
expected.)
~Andrew
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)