[XEN PATCH 3/8] xen: address MISRA C:2012 Rule 8.4

Nicola Vetrini posted 8 patches 2 years, 6 months ago
[XEN PATCH 3/8] xen: address MISRA C:2012 Rule 8.4
Posted by Nicola Vetrini 2 years, 6 months ago
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;
 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
Re: [XEN PATCH 3/8] xen: address MISRA C:2012 Rule 8.4
Posted by Luca Fancellu 2 years, 6 months ago

> 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
> 
> 
Re: [XEN PATCH 3/8] xen: address MISRA C:2012 Rule 8.4
Posted by Nicola Vetrini 2 years, 6 months ago
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.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH 3/8] xen: address MISRA C:2012 Rule 8.4
Posted by Jan Beulich 2 years, 6 months ago
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
Re: [XEN PATCH 3/8] xen: address MISRA C:2012 Rule 8.4
Posted by Luca Fancellu 2 years, 6 months ago

> 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
Re: [XEN PATCH 3/8] xen: address MISRA C:2012 Rule 8.4
Posted by Stefano Stabellini 2 years, 6 months ago
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>