Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1279864 > unrolled thread

[PATCH tip v5 2/5] kbuild: Add option to turn incompatible pointer check into error

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH tip v5 2/5] kbuild: Add option to turn incompatible pointer check into error Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-11-30 14:40 +0100
    Re: [PATCH tip v5 2/5] kbuild: Add option to turn incompatible  pointer check into error Daniel Wagner <daniel.wagner@bmw-carit.de> - 2015-11-30 16:30 +0100
      Re: [PATCH tip v5 2/5] kbuild: Add option to turn incompatible  pointer check into error "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-30 18:40 +0100
        Re: [PATCH tip v5 2/5] kbuild: Add option to turn incompatible  pointer check into error Daniel Wagner <wagi@monom.org> - 2015-11-30 20:00 +0100

#1279864 — [PATCH tip v5 2/5] kbuild: Add option to turn incompatible pointer check into error

With the introduction of the simple wait API we have two very
similar APIs in the kernel. For example wake_up() and swake_up()
is only one character away. Although the compiler will warn
happily the wrong usage it keeps on going an even links the kernel.
Thomas and Peter would rather like to see early missuses reported
as error early on.

In a first attempt we tried to wrap all swait and wait calls
into a macro which has an compile time type assertion. The result
was pretty ugly and wasn't able to catch all wrong usages.
woken_wake_function(), autoremove_wake_function() and wake_bit_function()
are assigned as function pointers. Wrapping them with a macro around is
not possible. Prefixing them with '_' was also not a real option
because there some users in the kernel which do use them as well.
All in all this attempt looked to intrusive and too ugly.

An alternative is to turn the pointer type check into an error which
catches wrong type uses. Obviously not only the swait/wait ones. That
isn't a bad thing either. Though for the beginning let's introduce it
as options in the kernel hacking section.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 Makefile          | 4 ++++
 lib/Kconfig.debug | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/Makefile b/Makefile
index 416660d..8817273 100644
--- a/Makefile
+++ b/Makefile
@@ -625,6 +625,10 @@ KBUILD_CFLAGS += $(call cc-option,-fno-reorder-blocks,) \
                  $(call cc-option,-fno-partial-inlining)
 endif
 
+ifdef CONFIG_ENABLE_ERR_TYPE_CHECK
+KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
+endif
+
 ifneq ($(CONFIG_FRAME_WARN),0)
 KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN})
 endif
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ab76b99..21c0193 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -194,6 +194,13 @@ config ENABLE_MUST_CHECK
 	  suppress the "warning: ignoring return value of 'foo', declared with
 	  attribute warn_unused_result" messages.
 
+config ENABLE_ERR_TYPE_CHECK
+       bool "Turn pointer type check into an error"
+       default n
+       help
+         Instead of just warning that a wrong pointer type is used,
+	 bail with a proper compile error.
+
 config FRAME_WARN
 	int "Warn for stack frames larger than (needs gcc 4.4)"
 	range 0 8192
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1280001 — Re: [PATCH tip v5 2/5] kbuild: Add option to turn incompatible pointer check into error

On 11/30/2015 02:38 PM, Daniel Wagner wrote:
> With the introduction of the simple wait API we have two very
> similar APIs in the kernel. For example wake_up() and swake_up()
> is only one character away. Although the compiler will warn
> happily the wrong usage it keeps on going an even links the kernel.
> Thomas and Peter would rather like to see early missuses reported
> as error early on.
> 
> In a first attempt we tried to wrap all swait and wait calls
> into a macro which has an compile time type assertion. The result
> was pretty ugly and wasn't able to catch all wrong usages.
> woken_wake_function(), autoremove_wake_function() and wake_bit_function()
> are assigned as function pointers. Wrapping them with a macro around is
> not possible. Prefixing them with '_' was also not a real option
> because there some users in the kernel which do use them as well.
> All in all this attempt looked to intrusive and too ugly.
> 
> An alternative is to turn the pointer type check into an error which
> catches wrong type uses. Obviously not only the swait/wait ones. That
> isn't a bad thing either. Though for the beginning let's introduce it
> as options in the kernel hacking section.

The kbuild bot found one problem for allmodconfig. I just send a fix for
it ("regmap: Fix leftover from struct reg_default to struct reg_sequence
change").
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280139 — Re: [PATCH tip v5 2/5] kbuild: Add option to turn incompatible pointer check into error

On Mon, Nov 30, 2015 at 04:26:03PM +0100, Daniel Wagner wrote:
> On 11/30/2015 02:38 PM, Daniel Wagner wrote:
> > With the introduction of the simple wait API we have two very
> > similar APIs in the kernel. For example wake_up() and swake_up()
> > is only one character away. Although the compiler will warn
> > happily the wrong usage it keeps on going an even links the kernel.
> > Thomas and Peter would rather like to see early missuses reported
> > as error early on.
> > 
> > In a first attempt we tried to wrap all swait and wait calls
> > into a macro which has an compile time type assertion. The result
> > was pretty ugly and wasn't able to catch all wrong usages.
> > woken_wake_function(), autoremove_wake_function() and wake_bit_function()
> > are assigned as function pointers. Wrapping them with a macro around is
> > not possible. Prefixing them with '_' was also not a real option
> > because there some users in the kernel which do use them as well.
> > All in all this attempt looked to intrusive and too ugly.
> > 
> > An alternative is to turn the pointer type check into an error which
> > catches wrong type uses. Obviously not only the swait/wait ones. That
> > isn't a bad thing either. Though for the beginning let's introduce it
> > as options in the kernel hacking section.
> 
> The kbuild bot found one problem for allmodconfig. I just send a fix for
> it ("regmap: Fix leftover from struct reg_default to struct reg_sequence
> change").

This will result in an updated series, correct?

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280199 — Re: [PATCH tip v5 2/5] kbuild: Add option to turn incompatible pointer check into error

Hi Paul,

On 11/30/2015 06:38 PM, Paul E. McKenney wrote:
> On Mon, Nov 30, 2015 at 04:26:03PM +0100, Daniel Wagner wrote:
>> On 11/30/2015 02:38 PM, Daniel Wagner wrote:
>>> With the introduction of the simple wait API we have two very
>>> similar APIs in the kernel. For example wake_up() and swake_up()
>>> is only one character away. Although the compiler will warn
>>> happily the wrong usage it keeps on going an even links the kernel.
>>> Thomas and Peter would rather like to see early missuses reported
>>> as error early on.
>>>
>>> In a first attempt we tried to wrap all swait and wait calls
>>> into a macro which has an compile time type assertion. The result
>>> was pretty ugly and wasn't able to catch all wrong usages.
>>> woken_wake_function(), autoremove_wake_function() and wake_bit_function()
>>> are assigned as function pointers. Wrapping them with a macro around is
>>> not possible. Prefixing them with '_' was also not a real option
>>> because there some users in the kernel which do use them as well.
>>> All in all this attempt looked to intrusive and too ugly.
>>>
>>> An alternative is to turn the pointer type check into an error which
>>> catches wrong type uses. Obviously not only the swait/wait ones. That
>>> isn't a bad thing either. Though for the beginning let's introduce it
>>> as options in the kernel hacking section.
>>
>> The kbuild bot found one problem for allmodconfig. I just send a fix for
>> it ("regmap: Fix leftover from struct reg_default to struct reg_sequence
>> change").
>
> This will result in an updated series, correct?

Not necessarily. This patch just points very verbose the incorrect 
pointer usage in regmap_register_path() users with allmodconfig. This 
series itself should be okay (unless I missed something).

cheers,
daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web