* [PATCH] common/initcall: Extern linker symbols with correct types.
@ 2013-10-17 17:03 Andrew Cooper
2013-10-18 7:51 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2013-10-17 17:03 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] common/initcall: Extern linker symbols with correct types.
2013-10-17 17:03 [PATCH] common/initcall: Extern linker symbols with correct types Andrew Cooper
@ 2013-10-18 7:51 ` Jan Beulich
2013-10-18 9:33 ` Andrew Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2013-10-18 7:51 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Keir Fraser
>>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] common/initcall: Extern linker symbols with correct types.
2013-10-18 7:51 ` Jan Beulich
@ 2013-10-18 9:33 ` Andrew Cooper
2013-10-18 14:06 ` [Patch v2] " Andrew Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2013-10-18 9:33 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser
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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Patch v2] common/initcall: Extern linker symbols with correct types.
2013-10-18 9:33 ` Andrew Cooper
@ 2013-10-18 14:06 ` Andrew Cooper
2013-10-22 16:34 ` Andrew Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2013-10-18 14:06 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch v2] common/initcall: Extern linker symbols with correct types.
2013-10-18 14:06 ` [Patch v2] " Andrew Cooper
@ 2013-10-22 16:34 ` Andrew Cooper
2013-10-22 16:56 ` Keir Fraser
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2013-10-22 16:34 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Keir Fraser, Jan Beulich, Xen-devel
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)();
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch v2] common/initcall: Extern linker symbols with correct types.
2013-10-22 16:34 ` Andrew Cooper
@ 2013-10-22 16:56 ` Keir Fraser
0 siblings, 0 replies; 6+ messages in thread
From: Keir Fraser @ 2013-10-22 16:56 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Jan Beulich, Xen-devel
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)();
>> }
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-22 16:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-17 17:03 [PATCH] common/initcall: Extern linker symbols with correct types Andrew Cooper
2013-10-18 7:51 ` Jan Beulich
2013-10-18 9:33 ` Andrew Cooper
2013-10-18 14:06 ` [Patch v2] " Andrew Cooper
2013-10-22 16:34 ` Andrew Cooper
2013-10-22 16:56 ` Keir Fraser
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).