* [PATCH v2] x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const"
@ 2017-12-28 12:25 Andy Shevchenko
2017-12-28 12:34 ` Ingo Molnar
2018-01-08 20:34 ` [tip:x86/urgent] " tip-bot for Andy Shevchenko
0 siblings, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2017-12-28 12:25 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, x86, Andi Kleen,
linux-kernel, Darren Hart, platform-driver-x86
Cc: Andy Shevchenko, Bhumika Goyal, julia.lawall
The annoying static analyzer follow up patches sometimes make a pain
rather than fixing issues.
The one done by commit 276c87054751
("x86/platform/intel-mid: Make 'bt_sfi_data' const")
convert the struct to be const while it is used as a temporary container
for important data that is used to fill 'parent' and 'name' fields in
struct platform_device_info.
That's why revert the commit which had been apparently done w/o reading
the code.
Note, compiler doesn't notice this due to explicit cast, which would be
addressed separately.
Cc: Bhumika Goyal <bhumirks@gmail.com>
Cc: julia.lawall@lip6.fr
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: low the tone of accusation that this made a regression
arch/x86/platform/intel-mid/device_libs/platform_bt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_bt.c b/arch/x86/platform/intel-mid/device_libs/platform_bt.c
index dc036e511f48..5a0483e7bf66 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_bt.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_bt.c
@@ -60,7 +60,7 @@ static int __init tng_bt_sfi_setup(struct bt_sfi_data *ddata)
return 0;
}
-static const struct bt_sfi_data tng_bt_sfi_data __initdata = {
+static struct bt_sfi_data tng_bt_sfi_data __initdata = {
.setup = tng_bt_sfi_setup,
};
--
2.15.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const"
2017-12-28 12:25 [PATCH v2] x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const" Andy Shevchenko
@ 2017-12-28 12:34 ` Ingo Molnar
2018-01-05 17:18 ` Andy Shevchenko
2018-01-08 20:34 ` [tip:x86/urgent] " tip-bot for Andy Shevchenko
1 sibling, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2017-12-28 12:34 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, x86, Andi Kleen,
linux-kernel, Darren Hart, platform-driver-x86, Bhumika Goyal,
julia.lawall
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> v2: low the tone of accusation that this made a regression
BTW., don't worry about that aspect too much: after a long debugging session it's
pretty natural to be upset at whoever introduced a regression.
( In fact a number of times I too got upset at the moron who wrote a particular
piece of buggy code, only for 'git annotate' to remind me that the moron was me. )
I personally just ignore the emotional attributes, and I usually edit changelogs
accordingly as well so the temporary state of mind of finding a regression doesn't
trickle upstream.
Plus in this particular case if we can help type propagation for driver data to
become a bit cleaner then the kernel project has gained a bit through all this
pain.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const"
2017-12-28 12:34 ` Ingo Molnar
@ 2018-01-05 17:18 ` Andy Shevchenko
2018-01-05 17:41 ` Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2018-01-05 17:18 UTC (permalink / raw)
To: Ingo Molnar, Fengguang Wu
Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, x86, Andi Kleen,
linux-kernel, Darren Hart, platform-driver-x86, Bhumika Goyal,
julia.lawall
On Thu, 2017-12-28 at 13:34 +0100, Ingo Molnar wrote:
> * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > v2: low the tone of accusation that this made a regression
>
> BTW., don't worry about that aspect too much: after a long debugging
> session it's
> pretty natural to be upset at whoever introduced a regression.
It appears that regression has been introduced by a new dependency to
the hci_bcm.c.
In any case, can we apply this one to 4.15 cycle to make others prevent
do an actual regressions further:
commit 03838ae1e8f692dd2bdbd49820ed668d4b7bfbc2
Author: Andi Kleen <ak@linux.intel.com>
Date: Fri Jan 5 13:26:44 2018 +1100
arch/x86/platform/intel-mid/device_libs/platform_bt.c: fix const
confusion
>
> ( In fact a number of times I too got upset at the moron who wrote a
> particular
> piece of buggy code, only for 'git annotate' to remind me that the
> moron was me. )
>
> I personally just ignore the emotional attributes, and I usually edit
> changelogs
> accordingly as well so the temporary state of mind of finding a
> regression doesn't
> trickle upstream.
>
> Plus in this particular case if we can help type propagation for
> driver data to
> become a bit cleaner then the kernel project has gained a bit through
> all this
> pain.
I has been thinking if 0day can complain about these:
1) castings in new code
2) applying const to older *working* code
Fengguang, what do you think?
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const"
2018-01-05 17:18 ` Andy Shevchenko
@ 2018-01-05 17:41 ` Julia Lawall
0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2018-01-05 17:41 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Ingo Molnar, Fengguang Wu, Thomas Gleixner, Ingo Molnar,
H . Peter Anvin, x86, Andi Kleen, linux-kernel, Darren Hart,
platform-driver-x86, Bhumika Goyal
On Fri, 5 Jan 2018, Andy Shevchenko wrote:
> On Thu, 2017-12-28 at 13:34 +0100, Ingo Molnar wrote:
> > * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >
> > > v2: low the tone of accusation that this made a regression
> >
> > BTW., don't worry about that aspect too much: after a long debugging
> > session it's
> > pretty natural to be upset at whoever introduced a regression.
>
> It appears that regression has been introduced by a new dependency to
> the hci_bcm.c.
>
> In any case, can we apply this one to 4.15 cycle to make others prevent
> do an actual regressions further:
>
> commit 03838ae1e8f692dd2bdbd49820ed668d4b7bfbc2
> Author: Andi Kleen <ak@linux.intel.com>
> Date: Fri Jan 5 13:26:44 2018 +1100
>
> arch/x86/platform/intel-mid/device_libs/platform_bt.c: fix const
> confusion
>
> >
> > ( In fact a number of times I too got upset at the moron who wrote a
> > particular
> > piece of buggy code, only for 'git annotate' to remind me that the
> > moron was me. )
> >
> > I personally just ignore the emotional attributes, and I usually edit
> > changelogs
> > accordingly as well so the temporary state of mind of finding a
> > regression doesn't
> > trickle upstream.
> >
> > Plus in this particular case if we can help type propagation for
> > driver data to
> > become a bit cleaner then the kernel project has gained a bit through
> > all this
> > pain.
>
> I has been thinking if 0day can complain about these:
> 1) castings in new code
> 2) applying const to older *working* code
>
> Fengguang, what do you think?
As suggested previously, I think it would be better if code that retrieves
const structures from nonconst fields would store the result in a const
variable. Then the compiler would pick up the verification from there. A
Coccinelle script can detect cases where this property does not hold, at
least within a single file. This would have protected the given code and
still allowed constant structures to be made const.
julia
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:x86/urgent] x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const"
2017-12-28 12:25 [PATCH v2] x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const" Andy Shevchenko
2017-12-28 12:34 ` Ingo Molnar
@ 2018-01-08 20:34 ` tip-bot for Andy Shevchenko
2018-01-09 10:35 ` Andy Shevchenko
1 sibling, 1 reply; 6+ messages in thread
From: tip-bot for Andy Shevchenko @ 2018-01-08 20:34 UTC (permalink / raw)
To: linux-tip-commits
Cc: torvalds, dvhart, bhumirks, peterz, tglx, hpa, andriy.shevchenko,
mingo, linux-kernel
Commit-ID: 9d0513d82f1a8fe17b41f113ac5922fa57dbaf5c
Gitweb: https://git.kernel.org/tip/9d0513d82f1a8fe17b41f113ac5922fa57dbaf5c
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
AuthorDate: Thu, 28 Dec 2017 14:25:23 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 8 Jan 2018 20:01:44 +0100
x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const"
So one of the constification patches unearthed a type casting fragility
of the underlying code:
276c87054751 ("x86/platform/intel-mid: Make 'bt_sfi_data' const")
converted the struct to be const while it is also used as a temporary
container for important data that is used to fill 'parent' and 'name'
fields in struct platform_device_info.
The compiler doesn't notice this due to an explicit type cast that loses
the const - which fragility will be fixed separately.
This type cast turned a seemingly trivial const propagation patch into a
hard to debug data corruptor and crasher bug.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Bhumika Goyal <bhumirks@gmail.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: julia.lawall@lip6.fr
Cc: platform-driver-x86@vger.kernel.org
Link: http://lkml.kernel.org/r/20171228122523.21802-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/platform/intel-mid/device_libs/platform_bt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_bt.c b/arch/x86/platform/intel-mid/device_libs/platform_bt.c
index dc036e5..5a0483e 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_bt.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_bt.c
@@ -60,7 +60,7 @@ static int __init tng_bt_sfi_setup(struct bt_sfi_data *ddata)
return 0;
}
-static const struct bt_sfi_data tng_bt_sfi_data __initdata = {
+static struct bt_sfi_data tng_bt_sfi_data __initdata = {
.setup = tng_bt_sfi_setup,
};
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [tip:x86/urgent] x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const"
2018-01-08 20:34 ` [tip:x86/urgent] " tip-bot for Andy Shevchenko
@ 2018-01-09 10:35 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2018-01-09 10:35 UTC (permalink / raw)
To: torvalds, dvhart, bhumirks, peterz, tglx, hpa, mingo,
linux-kernel, linux-tip-commits
On Mon, 2018-01-08 at 12:34 -0800, tip-bot for Andy Shevchenko wrote:
> Committer: Ingo Molnar <mingo@kernel.org>
Thank you!
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-09 10:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-28 12:25 [PATCH v2] x86/platform/intel-mid: Revert "Make 'bt_sfi_data' const" Andy Shevchenko
2017-12-28 12:34 ` Ingo Molnar
2018-01-05 17:18 ` Andy Shevchenko
2018-01-05 17:41 ` Julia Lawall
2018-01-08 20:34 ` [tip:x86/urgent] " tip-bot for Andy Shevchenko
2018-01-09 10:35 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.