On Thu, 20 Jun 2024, Federico Serafini wrote: > On 19/06/24 13:17, Julien Grall wrote: > > Hi Federico, > > > > On 19/06/2024 10:29, Federico Serafini wrote: > > > MISRA C Rule 16.4 states that every `switch' statement shall have a > > > `default' label" and a statement or a comment prior to the > > > terminating break statement. > > > > > > This patch addresses some violations of the rule related to the > > > "notifier pattern": a frequently-used pattern whereby only a few values > > > are handled by the switch statement and nothing should be done for > > > others (nothing to do in the default case). > > > > > > Note that for function mwait_idle_cpu_init() in > > > xen/arch/x86/cpu/mwait-idle.c the /* Notifier pattern. */ comment is > > > not added: differently from the other functions covered in this patch, > > > the default label has a return statement that does not violates Rule 16.4. > > > > > > No functional change. > > > > > > Signed-off-by: Federico Serafini <federico.seraf...@bugseng.com> > > > --- > > > Changes in v2: > > > as Jan pointed out, in the v1 some patterns were not explicitly identified > > > (https://lore.kernel.org/xen-devel/cad05a5c-e2d8-4e5d-af05-30ae6f959...@bugseng.com/). > > > > > > This version adds the /* Notifier pattern. */ to all the pattern present > > > in > > > the Xen codebase except for mwait_idle_cpu_init(). > > > --- > > > xen/arch/arm/cpuerrata.c | 1 + > > > xen/arch/arm/gic-v3-lpi.c | 4 ++++ > > > xen/arch/arm/gic.c | 1 + > > > xen/arch/arm/irq.c | 4 ++++ > > > xen/arch/arm/mmu/p2m.c | 1 + > > > xen/arch/arm/percpu.c | 1 + > > > xen/arch/arm/smpboot.c | 1 + > > > xen/arch/arm/time.c | 1 + > > > xen/arch/arm/vgic-v3-its.c | 2 ++ > > > xen/arch/x86/acpi/cpu_idle.c | 4 ++++ > > > xen/arch/x86/cpu/mcheck/mce.c | 4 ++++ > > > xen/arch/x86/cpu/mcheck/mce_intel.c | 4 ++++ > > > xen/arch/x86/genapic/x2apic.c | 3 +++ > > > xen/arch/x86/hvm/hvm.c | 1 + > > > xen/arch/x86/nmi.c | 1 + > > > xen/arch/x86/percpu.c | 3 +++ > > > xen/arch/x86/psr.c | 3 +++ > > > xen/arch/x86/smpboot.c | 3 +++ > > > xen/common/kexec.c | 1 + > > > xen/common/rcupdate.c | 1 + > > > xen/common/sched/core.c | 1 + > > > xen/common/sched/cpupool.c | 1 + > > > xen/common/spinlock.c | 1 + > > > xen/common/tasklet.c | 1 + > > > xen/common/timer.c | 1 + > > > xen/drivers/cpufreq/cpufreq.c | 1 + > > > xen/drivers/cpufreq/cpufreq_misc_governors.c | 3 +++ > > > xen/drivers/passthrough/x86/hvm.c | 3 +++ > > > xen/drivers/passthrough/x86/iommu.c | 3 +++ > > > 29 files changed, 59 insertions(+) > > > > > > diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c > > > index 2b7101ea25..69c30aecd8 100644 > > > --- a/xen/arch/arm/cpuerrata.c > > > +++ b/xen/arch/arm/cpuerrata.c > > > @@ -730,6 +730,7 @@ static int cpu_errata_callback(struct notifier_block > > > *nfb, > > > rc = enable_nonboot_cpu_caps(arm_errata); > > > break; > > > default: > > > + /* Notifier pattern. */ > > Without looking at the commit message (which may not be trivial when > > committed), it is not clear to me what this is supposed to mean. Will there > > be a longer explanation in the MISRA doc? Should this be a SAF-* comment? > > > > > break; > > > } > > > diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c > > > index eb0a5535e4..4c2bd35403 100644 > > > --- a/xen/arch/arm/gic-v3-lpi.c > > > +++ b/xen/arch/arm/gic-v3-lpi.c > > > @@ -389,6 +389,10 @@ static int cpu_callback(struct notifier_block *nfb, > > > unsigned long action, > > > printk(XENLOG_ERR "Unable to allocate the pendtable for > > > CPU%lu\n", > > > cpu); > > > break; > > > + > > > + default: > > > + /* Notifier pattern. */ > > > + break; > > > > Skimming through v1, it was pointed out that gic-v3-lpi may miss some cases. > > > > Let me start with that I understand this patch is technically not changing > > anything. However, it gives us an opportunity to check the notifier pattern. > > > > Has anyone done any proper investigation? If so, what was the outcome? If > > not, have we identified someone to do it? > > > > The same question will apply for place where you add "default". > > Yes, I also think this could be an opportunity to check the pattern > but no one has yet been identified to do this.
I don't think I understand Julien's question and/or your answer. Is the question whether someone has done an analysis to make sure this patch covers all notifier patters in the xen codebase? If so, I expect that you have done an analysis simply by basing this patch on the 16.4 violations reported by ECLAIR?
