Xen devel - Oct 2013 - [PATCH] common/initcall: Extern linker symbols with correct types.

Coverity IDs 1054956, 1054957

Coverity pointed out that we applying array operations based on an expression
which yielded singleton pointers.  The problem is actually that the externs
were typed incorrectly.

Correct the extern declaration to prevent straying into undefined behaviour,
and relying on the lenience of GCC to work.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/common/kernel.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index b8707d9..e785edb 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -196,19 +196,19 @@ void add_taint(unsigned flag)
     tainted |= flag;
 }
 
-extern initcall_t __initcall_start, __presmp_initcall_end, __initcall_end;
+extern initcall_t __initcall_start[], __presmp_initcall_end[],
__initcall_end[];
 
 void __init do_presmp_initcalls(void)
 {
     initcall_t *call;
-    for ( call = &__initcall_start; call < &__presmp_initcall_end;
call++ )
+    for ( call = __initcall_start; call < __presmp_initcall_end; call++ )
         (*call)();
 }
 
 void __init do_initcalls(void)
 {
     initcall_t *call;
-    for ( call = &__presmp_initcall_end; call < &__initcall_end;
call++ )
+    for ( call = __presmp_initcall_end; call < __initcall_end; call++ )
         (*call)();
 }
 
-- 
1.7.10.4

>>> On 17.10.13 at 19:03, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> Coverity IDs 1054956, 1054957
> 
> Coverity pointed out that we applying array operations based on an 
> expression
> which yielded singleton pointers.  The problem is actually that the externs
> were typed incorrectly.
> 
> Correct the extern declaration to prevent straying into undefined
behaviour,
> and relying on the lenience of GCC to work.
Fine by me, but if you already touch this I would have really liked
to see you also make the declarations const-correct.

Jan
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> ---
>  xen/common/kernel.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index b8707d9..e785edb 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -196,19 +196,19 @@ void add_taint(unsigned flag)
>      tainted |= flag;
>  }
>  
> -extern initcall_t __initcall_start, __presmp_initcall_end, __initcall_end;
> +extern initcall_t __initcall_start[], __presmp_initcall_end[], 
> __initcall_end[];
>  
>  void __init do_presmp_initcalls(void)
>  {
>      initcall_t *call;
> -    for ( call = &__initcall_start; call <
&__presmp_initcall_end; call++ )
> +    for ( call = __initcall_start; call < __presmp_initcall_end; call++
)
>          (*call)();
>  }
>  
>  void __init do_initcalls(void)
>  {
>      initcall_t *call;
> -    for ( call = &__presmp_initcall_end; call <
&__initcall_end; call++ )
> +    for ( call = __presmp_initcall_end; call < __initcall_end; call++ )
>          (*call)();
>  }
>  
> -- 
> 1.7.10.4

On 18/10/13 08:51, Jan Beulich wrote:
>>>> On 17.10.13 at 19:03, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
>> Coverity IDs 1054956, 1054957
>>
>> Coverity pointed out that we applying array operations based on an 
>> expression
>> which yielded singleton pointers.  The problem is actually that the
externs
>> were typed incorrectly.
>>
>> Correct the extern declaration to prevent straying into undefined
behaviour,
>> and relying on the lenience of GCC to work.
> Fine by me, but if you already touch this I would have really liked
> to see you also make the declarations const-correct.
>
> Jan
Very good point - I shall submit v2 in due course.

~Andrew
>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <JBeulich@suse.com>
>> ---
>>  xen/common/kernel.c |    6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
>> index b8707d9..e785edb 100644
>> --- a/xen/common/kernel.c
>> +++ b/xen/common/kernel.c
>> @@ -196,19 +196,19 @@ void add_taint(unsigned flag)
>>      tainted |= flag;
>>  }
>>  
>> -extern initcall_t __initcall_start, __presmp_initcall_end,
__initcall_end;
>> +extern initcall_t __initcall_start[], __presmp_initcall_end[], 
>> __initcall_end[];
>>  
>>  void __init do_presmp_initcalls(void)
>>  {
>>      initcall_t *call;
>> -    for ( call = &__initcall_start; call <
&__presmp_initcall_end; call++ )
>> +    for ( call = __initcall_start; call < __presmp_initcall_end;
call++ )
>>          (*call)();
>>  }
>>  
>>  void __init do_initcalls(void)
>>  {
>>      initcall_t *call;
>> -    for ( call = &__presmp_initcall_end; call <
&__initcall_end; call++ )
>> +    for ( call = __presmp_initcall_end; call < __initcall_end;
call++ )
>>          (*call)();
>>  }
>>  
>> -- 
>> 1.7.10.4
>
>

Coverity IDs 1054956, 1054957

Coverity pointed out that we applying array operations based on an expression
which yielded singleton pointers.  The problem is actually that the externs
were typed incorrectly.

Correct the extern declaration to prevent straying into undefined behaviour,
and relying on the lenience of GCC to work.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>

---

Changes in v2: apply const correctness as well.
---
 xen/common/kernel.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index b8707d9..4ca50c4 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -196,19 +196,20 @@ void add_taint(unsigned flag)
     tainted |= flag;
 }
 
-extern initcall_t __initcall_start, __presmp_initcall_end, __initcall_end;
+extern const initcall_t __initcall_start[], __presmp_initcall_end[],
+    __initcall_end[];
 
 void __init do_presmp_initcalls(void)
 {
-    initcall_t *call;
-    for ( call = &__initcall_start; call < &__presmp_initcall_end;
call++ )
+    const initcall_t *call;
+    for ( call = __initcall_start; call < __presmp_initcall_end; call++ )
         (*call)();
 }
 
 void __init do_initcalls(void)
 {
-    initcall_t *call;
-    for ( call = &__presmp_initcall_end; call < &__initcall_end;
call++ )
+    const initcall_t *call;
+    for ( call = __presmp_initcall_end; call < __initcall_end; call++ )
         (*call)();
 }
 
-- 
1.7.10.4

On 18/10/13 15:06, Andrew Cooper wrote:
> Coverity IDs 1054956, 1054957
>
> Coverity pointed out that we applying array operations based on an
expression
> which yielded singleton pointers.  The problem is actually that the externs
> were typed incorrectly.
>
> Correct the extern declaration to prevent straying into undefined
behaviour,
> and relying on the lenience of GCC to work.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
Ping?
>
> ---
>
> Changes in v2: apply const correctness as well.
> ---
>  xen/common/kernel.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index b8707d9..4ca50c4 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -196,19 +196,20 @@ void add_taint(unsigned flag)
>      tainted |= flag;
>  }
>  
> -extern initcall_t __initcall_start, __presmp_initcall_end, __initcall_end;
> +extern const initcall_t __initcall_start[], __presmp_initcall_end[],
> +    __initcall_end[];
>  
>  void __init do_presmp_initcalls(void)
>  {
> -    initcall_t *call;
> -    for ( call = &__initcall_start; call <
&__presmp_initcall_end; call++ )
> +    const initcall_t *call;
> +    for ( call = __initcall_start; call < __presmp_initcall_end; call++
)
>          (*call)();
>  }
>  
>  void __init do_initcalls(void)
>  {
> -    initcall_t *call;
> -    for ( call = &__presmp_initcall_end; call <
&__initcall_end; call++ )
> +    const initcall_t *call;
> +    for ( call = __presmp_initcall_end; call < __initcall_end; call++ )
>          (*call)();
>  }
>

On 22/10/2013 17:34, "Andrew Cooper" <andrew.cooper3@citrix.com>
wrote:
> On 18/10/13 15:06, Andrew Cooper wrote:
>> Coverity IDs 1054956, 1054957
>> 
>> Coverity pointed out that we applying array operations based on an
expression
>> which yielded singleton pointers.  The problem is actually that the
externs
>> were typed incorrectly.
>> 
>> Correct the extern declaration to prevent straying into undefined
behaviour,
>> and relying on the lenience of GCC to work.
>> 
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <JBeulich@suse.com>
> 
> Ping?
Acked-by: Keir Fraser <keir@xen.org>
>> 
>> ---
>> 
>> Changes in v2: apply const correctness as well.
>> ---
>>  xen/common/kernel.c |   11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>> 
>> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
>> index b8707d9..4ca50c4 100644
>> --- a/xen/common/kernel.c
>> +++ b/xen/common/kernel.c
>> @@ -196,19 +196,20 @@ void add_taint(unsigned flag)
>>      tainted |= flag;
>>  }
>>  
>> -extern initcall_t __initcall_start, __presmp_initcall_end,
__initcall_end;
>> +extern const initcall_t __initcall_start[], __presmp_initcall_end[],
>> +    __initcall_end[];
>>  
>>  void __init do_presmp_initcalls(void)
>>  {
>> -    initcall_t *call;
>> -    for ( call = &__initcall_start; call <
&__presmp_initcall_end; call++ )
>> +    const initcall_t *call;
>> +    for ( call = __initcall_start; call < __presmp_initcall_end;
call++ )
>>          (*call)();
>>  }
>>  
>>  void __init do_initcalls(void)
>>  {
>> -    initcall_t *call;
>> -    for ( call = &__presmp_initcall_end; call <
&__initcall_end; call++ )
>> +    const initcall_t *call;
>> +    for ( call = __presmp_initcall_end; call < __initcall_end;
call++ )
>>          (*call)();
>>  }
>>  
>