| Message ID | c010bbb270ff3b546d4790487cf973413ca2af26.1691575243.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: address MISRA C:2012 Rule 8.4 | expand |
> On 9 Aug 2023, at 12:02, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: > > The variable 'saved_cmdline' can be defined static, > as its only uses are within the same file. This in turn avoids > violating Rule 8.4 because no declaration is present. > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > --- > xen/common/kernel.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/common/kernel.c b/xen/common/kernel.c > index fb919f3d9c..52aa287627 100644 > --- a/xen/common/kernel.c > +++ b/xen/common/kernel.c > @@ -28,7 +28,7 @@ CHECK_feature_info; > > enum system_state system_state = SYS_STATE_early_boot; > > -xen_commandline_t saved_cmdline; > +static xen_commandline_t saved_cmdline; I see this line was touched by fa97833ae18e4a42c0e5ba4e781173457b5d3397, have you checked that making it static was not affecting anything else? > static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE; > > static int assign_integer_param(const struct kernel_param *param, uint64_t val) > -- > 2.34.1 > >
On 09.08.2023 15:50, Luca Fancellu wrote: >> On 9 Aug 2023, at 12:02, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: >> >> The variable 'saved_cmdline' can be defined static, >> as its only uses are within the same file. This in turn avoids >> violating Rule 8.4 because no declaration is present. >> >> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >> --- >> xen/common/kernel.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/xen/common/kernel.c b/xen/common/kernel.c >> index fb919f3d9c..52aa287627 100644 >> --- a/xen/common/kernel.c >> +++ b/xen/common/kernel.c >> @@ -28,7 +28,7 @@ CHECK_feature_info; >> >> enum system_state system_state = SYS_STATE_early_boot; >> >> -xen_commandline_t saved_cmdline; >> +static xen_commandline_t saved_cmdline; > > I see this line was touched by fa97833ae18e4a42c0e5ba4e781173457b5d3397, > have you checked that making it static was not affecting anything else? The code requiring the symbol to be non-static went away in e6ee01ad24b6 ("xen/version: Drop compat/kernel.c"). That's where the symbol would have wanted to become static. But that was very easy to overlook. Jan
On 09/08/2023 15:50, Luca Fancellu wrote: >> On 9 Aug 2023, at 12:02, Nicola Vetrini <nicola.vetrini@bugseng.com> >> wrote: >> >> The variable 'saved_cmdline' can be defined static, >> as its only uses are within the same file. This in turn avoids >> violating Rule 8.4 because no declaration is present. >> >> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >> --- >> xen/common/kernel.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/xen/common/kernel.c b/xen/common/kernel.c >> index fb919f3d9c..52aa287627 100644 >> --- a/xen/common/kernel.c >> +++ b/xen/common/kernel.c >> @@ -28,7 +28,7 @@ CHECK_feature_info; >> >> enum system_state system_state = SYS_STATE_early_boot; >> >> -xen_commandline_t saved_cmdline; >> +static xen_commandline_t saved_cmdline; > > I see this line was touched by > fa97833ae18e4a42c0e5ba4e781173457b5d3397, > have you checked that making it static was not affecting anything else? > > Though Jan already replied on this, the commit(s) were tested by patchew and our pipeline. This is normally our process, apart from MISRA checks.
> On 9 Aug 2023, at 15:06, Jan Beulich <jbeulich@suse.com> wrote: > > On 09.08.2023 15:50, Luca Fancellu wrote: >>> On 9 Aug 2023, at 12:02, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: >>> >>> The variable 'saved_cmdline' can be defined static, >>> as its only uses are within the same file. This in turn avoids >>> violating Rule 8.4 because no declaration is present. >>> >>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >>> --- >>> xen/common/kernel.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/xen/common/kernel.c b/xen/common/kernel.c >>> index fb919f3d9c..52aa287627 100644 >>> --- a/xen/common/kernel.c >>> +++ b/xen/common/kernel.c >>> @@ -28,7 +28,7 @@ CHECK_feature_info; >>> >>> enum system_state system_state = SYS_STATE_early_boot; >>> >>> -xen_commandline_t saved_cmdline; >>> +static xen_commandline_t saved_cmdline; >> >> I see this line was touched by fa97833ae18e4a42c0e5ba4e781173457b5d3397, >> have you checked that making it static was not affecting anything else? > > The code requiring the symbol to be non-static went away in e6ee01ad24b6 > ("xen/version: Drop compat/kernel.c"). That's where the symbol would have > wanted to become static. But that was very easy to overlook. Yes you are right Jan, Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> Possibly with the Fixes tag Cheers, Luca
On Wed, 9 Aug 2023, Luca Fancellu wrote: > > On 9 Aug 2023, at 15:06, Jan Beulich <jbeulich@suse.com> wrote: > > > > On 09.08.2023 15:50, Luca Fancellu wrote: > >>> On 9 Aug 2023, at 12:02, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: > >>> > >>> The variable 'saved_cmdline' can be defined static, > >>> as its only uses are within the same file. This in turn avoids > >>> violating Rule 8.4 because no declaration is present. > >>> > >>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > >>> --- > >>> xen/common/kernel.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/xen/common/kernel.c b/xen/common/kernel.c > >>> index fb919f3d9c..52aa287627 100644 > >>> --- a/xen/common/kernel.c > >>> +++ b/xen/common/kernel.c > >>> @@ -28,7 +28,7 @@ CHECK_feature_info; > >>> > >>> enum system_state system_state = SYS_STATE_early_boot; > >>> > >>> -xen_commandline_t saved_cmdline; > >>> +static xen_commandline_t saved_cmdline; > >> > >> I see this line was touched by fa97833ae18e4a42c0e5ba4e781173457b5d3397, > >> have you checked that making it static was not affecting anything else? > > > > The code requiring the symbol to be non-static went away in e6ee01ad24b6 > > ("xen/version: Drop compat/kernel.c"). That's where the symbol would have > > wanted to become static. But that was very easy to overlook. > > Yes you are right Jan, > > Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> > > Possibly with the Fixes tag Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
diff --git a/xen/common/kernel.c b/xen/common/kernel.c index fb919f3d9c..52aa287627 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -28,7 +28,7 @@ CHECK_feature_info; enum system_state system_state = SYS_STATE_early_boot; -xen_commandline_t saved_cmdline; +static xen_commandline_t saved_cmdline; static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE; static int assign_integer_param(const struct kernel_param *param, uint64_t val)
The variable 'saved_cmdline' can be defined static, as its only uses are within the same file. This in turn avoids violating Rule 8.4 because no declaration is present. Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- xen/common/kernel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)