diff mbox

[v2,1/4] xen/init: Annotate all command line parameter infrastructure as const

Message ID 1466528345-22235-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper June 21, 2016, 4:59 p.m. UTC
There is no reason for any of it to be modified.  Additionally, link
.init.setup beside the other constant .init data.

While editing this area, correct the types used in the extern
declarations for __setup_start and __setup_end to match the types the
linker actually produces.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>

v2:
 * Extra const
---
 xen/arch/arm/xen.lds.S | 11 ++++++-----
 xen/arch/x86/xen.lds.S | 11 ++++++-----
 xen/common/kernel.c    |  6 +++---
 xen/include/xen/init.h |  7 ++++---
 4 files changed, 19 insertions(+), 16 deletions(-)

Comments

Jan Beulich June 22, 2016, 6:57 a.m. UTC | #1
>>> On 21.06.16 at 18:59, <andrew.cooper3@citrix.com> wrote:
> There is no reason for any of it to be modified.  Additionally, link
> .init.setup beside the other constant .init data.
> 
> While editing this area, correct the types used in the extern
> declarations for __setup_start and __setup_end to match the types the
> linker actually produces.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
Julien Grall June 23, 2016, 11:42 a.m. UTC | #2
Hi Andrew,

On 21/06/16 17:59, Andrew Cooper wrote:
> There is no reason for any of it to be modified.  Additionally, link
> .init.setup beside the other constant .init data.
>
> While editing this area, correct the types used in the extern
> declarations for __setup_start and __setup_end to match the types the
> linker actually produces.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

For the ARM bits:

Acked-by: Julien Grall <julien.grall@arm.com>

Regards,
Jan Beulich June 23, 2016, 4 p.m. UTC | #3
>>> On 21.06.16 at 18:59, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/include/xen/init.h
> +++ b/xen/include/xen/init.h
> @@ -86,10 +86,11 @@ struct kernel_param {
>      void *var;
>  };
>  
> -extern struct kernel_param __setup_start, __setup_end;
> +extern const struct kernel_param __setup_start[], __setup_end[];
>  
> -#define __setup_str static __initdata __attribute__((__aligned__(1))) char
> -#define __kparam static __initsetup \
> +#define __setup_str static const  __initconstrel \
> +    __attribute__((__aligned__(1))) char

This slipped my attention during review, but causes build failure
with some gcc versions now: Why __initconstrel rather than
__initconst?

Jan
Andrew Cooper June 23, 2016, 4:08 p.m. UTC | #4
On 23/06/16 17:00, Jan Beulich wrote:
>>>> On 21.06.16 at 18:59, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/include/xen/init.h
>> +++ b/xen/include/xen/init.h
>> @@ -86,10 +86,11 @@ struct kernel_param {
>>      void *var;
>>  };
>>  
>> -extern struct kernel_param __setup_start, __setup_end;
>> +extern const struct kernel_param __setup_start[], __setup_end[];
>>  
>> -#define __setup_str static __initdata __attribute__((__aligned__(1))) char
>> -#define __kparam static __initsetup \
>> +#define __setup_str static const  __initconstrel \
>> +    __attribute__((__aligned__(1))) char
> This slipped my attention during review, but causes build failure
> with some gcc versions now: Why __initconstrel rather than
> __initconst?

Because this was previously __initconst and you reverted it for breaking
on older GCC.  Message ID <56CB470E02000078000D4DBA@prv-mh.provo.novell.com>

I can't replicate any build failures with any GCC or clang combination
to hand.  Are you able to find some solution which GCC is happy with?

~Andrew
diff mbox

Patch

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 8320381..2ed7dee 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -135,6 +135,12 @@  SECTIONS
        *(.init.rodata)
        *(.init.rodata.rel)
        *(.init.rodata.str*)
+
+       . = ALIGN(POINTER_ALIGN);
+       __setup_start = .;
+       *(.init.setup)
+       __setup_end = .;
+
        *(.init.data)
        *(.init.data.rel)
        *(.init.data.rel.*)
@@ -145,11 +151,6 @@  SECTIONS
        __ctors_end = .;
   } :text
   . = ALIGN(32);
-  .init.setup : {
-       __setup_start = .;
-       *(.init.setup)
-       __setup_end = .;
-  } :text
   .init.proc.info : {
        __proc_info_start = .;
        *(.init.proc.info)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index dcbb8fe..2443b93 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -152,6 +152,12 @@  SECTIONS
        *(.init.rodata)
        *(.init.rodata.rel)
        *(.init.rodata.str*)
+
+       . = ALIGN(POINTER_ALIGN);
+       __setup_start = .;
+       *(.init.setup)
+       __setup_end = .;
+
        *(.init.data)
        *(.init.data.rel)
        *(.init.data.rel.*)
@@ -178,11 +184,6 @@  SECTIONS
        __ctors_end = .;
   } :text
   . = ALIGN(32);
-  .init.setup : {
-       __setup_start = .;
-       *(.init.setup)
-       __setup_end = .;
-  } :text
   .initcall.init : {
        __initcall_start = .;
        *(.initcallpresmp.init)
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index dae7e35..942b042 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -27,7 +27,7 @@  int tainted;
 xen_commandline_t saved_cmdline;
 
 static void __init assign_integer_param(
-    struct kernel_param *param, uint64_t val)
+    const struct kernel_param *param, uint64_t val)
 {
     switch ( param->len )
     {
@@ -52,7 +52,7 @@  void __init cmdline_parse(const char *cmdline)
 {
     char opt[100], *optval, *optkey, *q;
     const char *p = cmdline;
-    struct kernel_param *param;
+    const struct kernel_param *param;
     int bool_assert;
 
     if ( cmdline == NULL )
@@ -96,7 +96,7 @@  void __init cmdline_parse(const char *cmdline)
         if ( !bool_assert )
             optkey += 3;
 
-        for ( param = &__setup_start; param < &__setup_end; param++ )
+        for ( param = __setup_start; param < __setup_end; param++ )
         {
             if ( strcmp(param->name, optkey) )
             {
diff --git a/xen/include/xen/init.h b/xen/include/xen/init.h
index 671ac81..b04bcf9 100644
--- a/xen/include/xen/init.h
+++ b/xen/include/xen/init.h
@@ -86,10 +86,11 @@  struct kernel_param {
     void *var;
 };
 
-extern struct kernel_param __setup_start, __setup_end;
+extern const struct kernel_param __setup_start[], __setup_end[];
 
-#define __setup_str static __initdata __attribute__((__aligned__(1))) char
-#define __kparam static __initsetup \
+#define __setup_str static const  __initconstrel \
+    __attribute__((__aligned__(1))) char
+#define __kparam static const __initsetup \
     __attribute__((__aligned__(sizeof(void *)))) struct kernel_param
 
 #define custom_param(_name, _var) \