public inbox for linux-kbuild@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default
@ 2013-08-15 18:37 Sergei Trofimovich
  2013-08-15 21:18 ` Andrew Morton
  2013-08-23 12:18 ` Geert Uytterhoeven
  0 siblings, 2 replies; 5+ messages in thread
From: Sergei Trofimovich @ 2013-08-15 18:37 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sergei Trofimovich, Michal Marek, linux-kbuild, Andrew Morton

The common error type found in forward-ported/backported patches is missing headers.
One recent example (files and function names are mangled):

    void foo(){}
    EXPORT_SYMBOL(foo);

gave only warning

    foo.c:12345678:5: warning: function declaration isn't a prototype [-Wstrict-prototypes]
    void foo(){}
         ^

    foo.c:12345679:5: warning: data definition has no type or storage class [enabled by default]
    EXPORT_SYMBOL(foo);
    foo.c:12345679:5: warning: type defaults to 'int' in declaration of 'EXORT_SYMBOL' [-Werror=implicit-int]

Now it's a fata error. Tested on x86_64 allyesconfig.

Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
Cc: Michal Marek <mmarek@suse.cz>
Cc: linux-kbuild@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

Change since v1:
 - use 'cc-option' to respect old gccs
 - fix typos. Thanks to Oleg Verych

diff --git a/Makefile b/Makefile
index 6e48848..53f4776 100644
--- a/Makefile
+++ b/Makefile
@@ -374,6 +374,11 @@ KBUILD_CFLAGS   := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
 		   -Werror-implicit-function-declaration \
 		   -Wno-format-security \
 		   -fno-delete-null-pointer-checks
+
+# not universally available, but nice to have ones
+KBUILD_CFLAGS   +=  $(call cc-option,-Werror=implicit-int) \
+		    $(call cc-option,-Werror=strict-prototypes)
+
 KBUILD_AFLAGS_KERNEL :=
 KBUILD_CFLAGS_KERNEL :=
 KBUILD_AFLAGS   := -D__ASSEMBLY__
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default
  2013-08-15 18:37 [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default Sergei Trofimovich
@ 2013-08-15 21:18 ` Andrew Morton
  2013-08-23 12:18 ` Geert Uytterhoeven
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2013-08-15 21:18 UTC (permalink / raw)
  To: Sergei Trofimovich; +Cc: linux-kernel, Michal Marek, linux-kbuild

On Thu, 15 Aug 2013 21:37:31 +0300 Sergei Trofimovich <slyfox@gentoo.org> wrote:

> The common error type found in forward-ported/backported patches is missing headers.
> One recent example (files and function names are mangled):
> 
>     void foo(){}
>     EXPORT_SYMBOL(foo);
> 
> gave only warning
> 
>     foo.c:12345678:5: warning: function declaration isn't a prototype [-Wstrict-prototypes]
>     void foo(){}
>          ^
> 
>     foo.c:12345679:5: warning: data definition has no type or storage class [enabled by default]
>     EXPORT_SYMBOL(foo);
>     foo.c:12345679:5: warning: type defaults to 'int' in declaration of 'EXORT_SYMBOL' [-Werror=implicit-int]
> 
> Now it's a fata error. Tested on x86_64 allyesconfig.

Yes, let's try that.

Partly because the build still generates far too many warnings..

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default
  2013-08-15 18:37 [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default Sergei Trofimovich
  2013-08-15 21:18 ` Andrew Morton
@ 2013-08-23 12:18 ` Geert Uytterhoeven
  2013-08-23 17:37   ` Sergei Trofimovich
  2013-08-23 17:55   ` [PATCH v3] " Sergei Trofimovich
  1 sibling, 2 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2013-08-23 12:18 UTC (permalink / raw)
  To: Sergei Trofimovich
  Cc: linux-kernel@vger.kernel.org, Michal Marek, linux-kbuild,
	Andrew Morton

On Thu, Aug 15, 2013 at 8:37 PM, Sergei Trofimovich <slyfox@gentoo.org> wrote:
> diff --git a/Makefile b/Makefile
> index 6e48848..53f4776 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -374,6 +374,11 @@ KBUILD_CFLAGS   := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
>                    -Werror-implicit-function-declaration \
>                    -Wno-format-security \
>                    -fno-delete-null-pointer-checks
> +
> +# not universally available, but nice to have ones
> +KBUILD_CFLAGS   +=  $(call cc-option,-Werror=implicit-int) \
> +                   $(call cc-option,-Werror=strict-prototypes)
> +

This should be _below_ the line

    include $(srctree)/arch/$(SRCARCH)/Makefile

, together with the other users of cc-option, else it detects the features of
the host compiler instead of the cross-compiler when cross-compiling:

cc1: error: unrecognized command line option "-Werror=implicit-int"
cc1: error: unrecognized command line option "-Werror=strict-prototypes"

See also commit a1f42beb8e287482d1a802731d4fb7e2bdc2c703
("Makefile: fix up CROSS_COMPILE and READABLE_ASM interaction.").

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default
  2013-08-23 12:18 ` Geert Uytterhoeven
@ 2013-08-23 17:37   ` Sergei Trofimovich
  2013-08-23 17:55   ` [PATCH v3] " Sergei Trofimovich
  1 sibling, 0 replies; 5+ messages in thread
From: Sergei Trofimovich @ 2013-08-23 17:37 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-kernel@vger.kernel.org, Michal Marek, linux-kbuild,
	Andrew Morton

[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]

On Fri, 23 Aug 2013 14:18:02 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> On Thu, Aug 15, 2013 at 8:37 PM, Sergei Trofimovich <slyfox@gentoo.org> wrote:
> > diff --git a/Makefile b/Makefile
> > index 6e48848..53f4776 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -374,6 +374,11 @@ KBUILD_CFLAGS   := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> >                    -Werror-implicit-function-declaration \
> >                    -Wno-format-security \
> >                    -fno-delete-null-pointer-checks
> > +
> > +# not universally available, but nice to have ones
> > +KBUILD_CFLAGS   +=  $(call cc-option,-Werror=implicit-int) \
> > +                   $(call cc-option,-Werror=strict-prototypes)
> > +
> 
> This should be _below_ the line
> 
>     include $(srctree)/arch/$(SRCARCH)/Makefile
> 
> , together with the other users of cc-option, else it detects the features of
> the host compiler instead of the cross-compiler when cross-compiling:
> 
> cc1: error: unrecognized command line option "-Werror=implicit-int"
> cc1: error: unrecognized command line option "-Werror=strict-prototypes"

Oh, it was not obvious.

Does it mean the code right before is buggy as well?

>    ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
>    KBUILD_CFLAGS<->+= -Os $(call cc-disable-warning,maybe-uninitialized,)
    ^^^
>    else
>    KBUILD_CFLAGS<->+= -O2
>    endif
    
    include $(srctree)/arch/$(SRCARCH)/Makefile

Will respin fixed patch in a while.

Thanks!

-- 

  Sergei

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default
  2013-08-23 12:18 ` Geert Uytterhoeven
  2013-08-23 17:37   ` Sergei Trofimovich
@ 2013-08-23 17:55   ` Sergei Trofimovich
  1 sibling, 0 replies; 5+ messages in thread
From: Sergei Trofimovich @ 2013-08-23 17:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sergei Trofimovich, Michal Marek, linux-kbuild, Andrew Morton,
	Geert Uytterhoeven

The common error fount in forward-ported/backported patches is missing headers.
One recent example (files and function names are mangled):

    void foo(){}
    EXPORT_SYMBOL(foo);

gave only warning

    foo.c:12345678:5: warning: function declaration isn't a prototype [-Wstrict-prototypes]
    void foo(){}
         ^

    foo.c:12345679:5: warning: data definition has no type or storage class [enabled by default]
    EXPORT_SYMBOL(foo);
    foo.c:12345679:5: warning: type defaults to 'int' in declaration of 'EXORT_SYMBOL' [-Werror=implicit-int]

Now it's a fatal error. Tested on x86_64 allyesconfig.

Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
Cc: Michal Marek <mmarek@suse.cz>
Cc: linux-kbuild@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
---
 Makefile | 6 +++++++
 1 file changed, 6 insertions(+)

Change since v2:
 - moved CFLAGS checks lower to catch $(CROSS) case
   as suggested by Geert
 - added comments as other warning options do

Change since v1:
 - use 'cc-option' to respect old gccs
 - fix typos. Thanks to Oleg Verych

diff --git a/Makefile b/Makefile
index a5a55f4..f5f36e1 100644
--- a/Makefile
+++ b/Makefile
@@ -659,6 +660,12 @@ KBUILD_CFLAGS	+= $(call cc-option,-fno-strict-overflow)
 # conserve stack if available
 KBUILD_CFLAGS   += $(call cc-option,-fconserve-stack)
 
+# disallog errors like 'EXPORT_GPL(foo);' with missing header
+KBUILD_CFLAGS   += $(call cc-option,-Werror=implicit-int)
+
+# require functions to have argumens in prototypes, not empty 'int foo()'
+KBUILD_CFLAGS   += $(call cc-option,-Werror=strict-prototypes)
+
 # use the deterministic mode of AR if available
 KBUILD_ARFLAGS := $(call ar-option,D)
 
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-08-23 17:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-15 18:37 [PATCH v2] Makefile: enable -Werror=implicit-int and -Werror=strict-prototypes by default Sergei Trofimovich
2013-08-15 21:18 ` Andrew Morton
2013-08-23 12:18 ` Geert Uytterhoeven
2013-08-23 17:37   ` Sergei Trofimovich
2013-08-23 17:55   ` [PATCH v3] " Sergei Trofimovich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox