The final return statement is unreachable and hence disliked by Misra
C:2012 (rule 2.1). Convert those case-specific (main) return statements
which already use "rc", or in one case when it can be used without
further adding of code, to break.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This is an alternative proposal to
https://lists.xen.org/archives/html/xen-devel/2023-12/msg01537.html.
Yet another option would be to simply pull the default case out of the
switch() statement.
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4724,7 +4724,7 @@ long arch_memory_op(unsigned long cmd, X
spin_unlock(&d->arch.e820_lock);
rcu_unlock_domain(d);
- return rc;
+ break;
}
case XENMEM_memory_map:
@@ -4818,7 +4818,7 @@ long arch_memory_op(unsigned long cmd, X
if ( __copy_to_guest(arg, &ctxt.map, 1) )
return -EFAULT;
- return 0;
+ break;
}
case XENMEM_machphys_mapping:
@@ -4880,7 +4880,7 @@ long arch_memory_op(unsigned long cmd, X
}
rcu_unlock_domain(d);
- return rc;
+ break;
}
#endif
@@ -4888,7 +4888,7 @@ long arch_memory_op(unsigned long cmd, X
return subarch_memory_op(cmd, arg);
}
- return 0;
+ return rc;
}
int cf_check mmio_ro_emulated_write(
On Tue, 19 Dec 2023, Jan Beulich wrote: > The final return statement is unreachable and hence disliked by Misra > C:2012 (rule 2.1). Convert those case-specific (main) return statements > which already use "rc", or in one case when it can be used without > further adding of code, to break. > > No functional change intended. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > This is an alternative proposal to > https://lists.xen.org/archives/html/xen-devel/2023-12/msg01537.html. > Yet another option would be to simply pull the default case out of the > switch() statement. > > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -4724,7 +4724,7 @@ long arch_memory_op(unsigned long cmd, X > spin_unlock(&d->arch.e820_lock); > > rcu_unlock_domain(d); > - return rc; > + break; > } > > case XENMEM_memory_map: > @@ -4818,7 +4818,7 @@ long arch_memory_op(unsigned long cmd, X > if ( __copy_to_guest(arg, &ctxt.map, 1) ) > return -EFAULT; > > - return 0; > + break; > } There are also two other return 0; under case XENMEM_memory_map and XENMEM_machphys_mapping. I would be consistent and either leave this return 0 alone, or change all the return 0. Either way, this patch is correct, so: Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > case XENMEM_machphys_mapping: > @@ -4880,7 +4880,7 @@ long arch_memory_op(unsigned long cmd, X > } > > rcu_unlock_domain(d); > - return rc; > + break; > } > #endif > > @@ -4888,7 +4888,7 @@ long arch_memory_op(unsigned long cmd, X > return subarch_memory_op(cmd, arg); > } > > - return 0; > + return rc; > } > > int cf_check mmio_ro_emulated_write( >
On 20.12.2023 01:22, Stefano Stabellini wrote: > On Tue, 19 Dec 2023, Jan Beulich wrote: >> The final return statement is unreachable and hence disliked by Misra >> C:2012 (rule 2.1). Convert those case-specific (main) return statements >> which already use "rc", or in one case when it can be used without >> further adding of code, to break. >> >> No functional change intended. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> This is an alternative proposal to >> https://lists.xen.org/archives/html/xen-devel/2023-12/msg01537.html. >> Yet another option would be to simply pull the default case out of the >> switch() statement. >> >> --- a/xen/arch/x86/mm.c >> +++ b/xen/arch/x86/mm.c >> @@ -4724,7 +4724,7 @@ long arch_memory_op(unsigned long cmd, X >> spin_unlock(&d->arch.e820_lock); >> >> rcu_unlock_domain(d); >> - return rc; >> + break; >> } >> >> case XENMEM_memory_map: >> @@ -4818,7 +4818,7 @@ long arch_memory_op(unsigned long cmd, X >> if ( __copy_to_guest(arg, &ctxt.map, 1) ) >> return -EFAULT; >> >> - return 0; >> + break; >> } > > There are also two other return 0; under case XENMEM_memory_map and > XENMEM_machphys_mapping. I would be consistent and either leave this > return 0 alone, or change all the return 0. Yes, that would have been another possible pattern to follow. Due to the multiple possible approaches I had specifically outlined (in the description) the pattern I decided to follow. > Either way, this patch is correct, so: > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Thanks. Jan
© 2016 - 2024 Red Hat, Inc.