On 21.10.20 16:53, Andrea Bastoni wrote:
As part of a project I was experimenting with compiler warning options
that partial covers coding standard rules from e.g., MISRA 2012. An
example is Wconversion, and the narrower sign-conversion, sign-compare,
pointer-arch, cast-qual. E.g., Wconversion covers MISRA 2012 10.3 (and
parially 10.4, 10.1).
Thanks a lot, this is valuable work! While standard compliance does not
give you good code per se (*) and - if blindly followed - can even be
counterproductive, some warnings are low-hanging fruits, some report
missing cleanups, and there was at least one bug caught this way. I'll
go through the patches carefully and provide further feedback later.
(*) I once heard the saying that most coding standards exist in order to
reduce the damage unexperienced coders can cause to critical programs.
The sad truth is that such weird combinations are not uncommon in
commercial software projects...
Before enabling those, I've enabled some other warnings to filter out
definition/declaration "issues" that would clutter the compilation
otherwise. Specifically (Patch 0001), I've added: -Wextra -Wundef
-Wnested-externs -Wshadow -Wredundant-decls -Wdeprecated
The warnings were not too many (most of them related to
unused-parameters), but there were some interesting ones e.g.:
hypervisor: provide dedicated declaration for __page_pool and __text_start in
globals.h
Actually, it's only __page_pool, and we should make that global via
paging.h.
hypervisor: introduce uptr_t (depending on the wordsize) and define size_t
accordingly
hypervisor, arm-common: provide an explicit uint INVALID_CPU_ID
The patch series fixes the warnings generated by "-Wextra -Wundef
-Wnested-externs -Wshadow -Wredundant-decls -Wdeprecated", and two small
bugs:
x86: bitops: only x86_64 is supported, avoid picking the wrong default suffix
How did you come across that one? Luckily, we only ever set bit 0...
arm64: smmu: fix double negative
Good catch. I will fold that into the original commit, providing credits.
Up to "0024 x86: bitops: only x86_64 is supported, avoid picking the
wrong default suffix", the series is the same for both master and next.
These two
arm64: smmu: Wsign-compare: make iterator uint
arm64: smmu: fix double negative
are only next related.
I've compile-checked x86, arm, arm64, and tested arm64 (ZCU102), but I don't
have a suitable arm / x86 physical targets and I'm still figuring out
a crash I have on qemu-x86 (can reproduce with plain jailhouse-image
qemu image).
Obviously, I would be interested in learning about how that issue looks
like and how you reproduce it.
I don't know if there's interest in extending the compiler flags with
warnings about code structure, but since the code compiles cleanly with
the patches, I thought about sharing them.
In general, anything that is providing a reasonable chance to reveal
real issues early is welcome. But it's always a balance between
excessive code restructuring to avoid false-positives or still
oversensitive compilers and potentially valuable warnings.
Additional info:
gcc (Debian 10.2.0-13) 10.2.0
JFTR:
Wcast-qual generates 276 warnings on arm64 (but they looks ~ the same,
it could be feasible fixing them) Wconversion + Wpointer-arith are a
worse beast (~ 1400 occurrences), and the "int" return convention in
Linux doesn't make it easy to cleanly fix them.
Those fall under
"W=3 - more obscure warnings, can most likely be ignored"
(linux/scripts/Makefile.extrawarn)
and there are likely good reasons for that.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/7c3c49ea-93d7-2a15-d844-a2004231ebc5%40siemens.com.
