| Message ID | 1498761773-11164-3-git-send-email-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 29, 2017 at 08:42:52PM +0200, Thomas Huth wrote: > Using -Wextra together with -Werror is troublesome - various versions > of GCC produce suspicious or even wrong warnings with -Wextra which > then become fatal errors with -Werror. For example, the current state > of the kvm-unit-tests does not compile anymore with GCC 4.8.1 for > s390x due to an inadequate -Wmissing-field-initializers warning. > That's annoying for users who just would like to compile the > kvm-unit-tests and cumbersome for the developers who have to work > around these problems in the source code. So let's replace -Wextra > by a saner lists of warning flags that are normally enabled by -Wextra. > Since they apparently can be used for building x86, too, the flags > are now also applied to the global CFLAGS instead of specifying them > for the single targets only. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v2: > - Now that we've got COMMON_CFLAGS, the remaining flags can be > added to the global CFLAGS, too > - Removed -Wsign-compare > > Makefile | 10 +++++++--- > arm/Makefile.common | 1 - > powerpc/Makefile.common | 1 - > s390x/Makefile | 1 - > 4 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/Makefile b/Makefile > index 3ef6ea7..f12b2df 100644 > --- a/Makefile > +++ b/Makefile > @@ -50,9 +50,11 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile > cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ > > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) > > -COMMON_CFLAGS += -g > -COMMON_CFLAGS += $(autodepend-flags) -Wall -Wwrite-strings -Werror > -frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer > +COMMON_CFLAGS += -g $(autodepend-flags) > +COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized > +COMMON_CFLAGS += -Wtype-limits -Wignored-qualifiers -Wunused-but-set-parameter > +COMMON_CFLAGS += -Werror > + rame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer Some finger fumbling here. > fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "") > fnostack_protector := $(call cc-option, -fno-stack-protector, "") > fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "") > @@ -67,6 +69,8 @@ COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,) > COMMON_CFLAGS += $(fno_pic) $(no_pie) > > CFLAGS += $(COMMON_CFLAGS) > +CFLAGS += -Wmissing-parameter-type -Wold-style-declaration -Woverride-init > + > CXXFLAGS += $(COMMON_CFLAGS) > > autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d > diff --git a/arm/Makefile.common b/arm/Makefile.common > index 03b497b..7e5f527 100644 > --- a/arm/Makefile.common > +++ b/arm/Makefile.common > @@ -24,7 +24,6 @@ phys_base = $(LOADADDR) > > CFLAGS += -std=gnu99 > CFLAGS += -ffreestanding > -CFLAGS += -Wextra > CFLAGS += -O2 > CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib > > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common > index db5ba62..c4df2e9 100644 > --- a/powerpc/Makefile.common > +++ b/powerpc/Makefile.common > @@ -19,7 +19,6 @@ all: directories $(TEST_DIR)/boot_rom.bin $(tests-all) > > CFLAGS += -std=gnu99 > CFLAGS += -ffreestanding > -CFLAGS += -Wextra > CFLAGS += -O2 > CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib > CFLAGS += -Wa,-mregnames > diff --git a/s390x/Makefile b/s390x/Makefile > index 470cbba..bc099da 100644 > --- a/s390x/Makefile > +++ b/s390x/Makefile > @@ -7,7 +7,6 @@ test_cases: $(tests) > > CFLAGS += -std=gnu99 > CFLAGS += -ffreestanding > -CFLAGS += -Wextra > CFLAGS += -I $(SRCDIR)/lib > CFLAGS += -O2 > CFLAGS += -march=z900 > -- > 1.8.3.1 >
On Thu, Jun 29, 2017 at 08:42:52PM +0200, Thomas Huth wrote: > Using -Wextra together with -Werror is troublesome - various versions > of GCC produce suspicious or even wrong warnings with -Wextra which > then become fatal errors with -Werror. For example, the current state > of the kvm-unit-tests does not compile anymore with GCC 4.8.1 for > s390x due to an inadequate -Wmissing-field-initializers warning. > That's annoying for users who just would like to compile the > kvm-unit-tests and cumbersome for the developers who have to work > around these problems in the source code. So let's replace -Wextra > by a saner lists of warning flags that are normally enabled by -Wextra. > Since they apparently can be used for building x86, too, the flags > are now also applied to the global CFLAGS instead of specifying them > for the single targets only. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v2: > - Now that we've got COMMON_CFLAGS, the remaining flags can be > added to the global CFLAGS, too > - Removed -Wsign-compare I was just doing some reading about warning types. It looks like -Wsign-compare comes with -Wall anyway. drew
On 30.06.2017 10:07, Andrew Jones wrote: > On Thu, Jun 29, 2017 at 08:42:52PM +0200, Thomas Huth wrote: >> Using -Wextra together with -Werror is troublesome - various versions >> of GCC produce suspicious or even wrong warnings with -Wextra which >> then become fatal errors with -Werror. For example, the current state >> of the kvm-unit-tests does not compile anymore with GCC 4.8.1 for >> s390x due to an inadequate -Wmissing-field-initializers warning. >> That's annoying for users who just would like to compile the >> kvm-unit-tests and cumbersome for the developers who have to work >> around these problems in the source code. So let's replace -Wextra >> by a saner lists of warning flags that are normally enabled by -Wextra. >> Since they apparently can be used for building x86, too, the flags >> are now also applied to the global CFLAGS instead of specifying them >> for the single targets only. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> v2: >> - Now that we've got COMMON_CFLAGS, the remaining flags can be >> added to the global CFLAGS, too >> - Removed -Wsign-compare >> >> Makefile | 10 +++++++--- >> arm/Makefile.common | 1 - >> powerpc/Makefile.common | 1 - >> s390x/Makefile | 1 - >> 4 files changed, 7 insertions(+), 6 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index 3ef6ea7..f12b2df 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -50,9 +50,11 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile >> cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ >> > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) >> >> -COMMON_CFLAGS += -g >> -COMMON_CFLAGS += $(autodepend-flags) -Wall -Wwrite-strings -Werror >> -frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer >> +COMMON_CFLAGS += -g $(autodepend-flags) >> +COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized >> +COMMON_CFLAGS += -Wtype-limits -Wignored-qualifiers -Wunused-but-set-parameter >> +COMMON_CFLAGS += -Werror >> + rame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer > > Some finger fumbling here. Ooops, ... very well spotted ... I'll send a v3 Thomas
On 30.06.2017 10:19, Andrew Jones wrote: > On Thu, Jun 29, 2017 at 08:42:52PM +0200, Thomas Huth wrote: >> Using -Wextra together with -Werror is troublesome - various versions >> of GCC produce suspicious or even wrong warnings with -Wextra which >> then become fatal errors with -Werror. For example, the current state >> of the kvm-unit-tests does not compile anymore with GCC 4.8.1 for >> s390x due to an inadequate -Wmissing-field-initializers warning. >> That's annoying for users who just would like to compile the >> kvm-unit-tests and cumbersome for the developers who have to work >> around these problems in the source code. So let's replace -Wextra >> by a saner lists of warning flags that are normally enabled by -Wextra. >> Since they apparently can be used for building x86, too, the flags >> are now also applied to the global CFLAGS instead of specifying them >> for the single targets only. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> v2: >> - Now that we've got COMMON_CFLAGS, the remaining flags can be >> added to the global CFLAGS, too >> - Removed -Wsign-compare > > I was just doing some reading about warning types. It looks like > -Wsign-compare comes with -Wall anyway. According to https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html it's only enabled by -Wall when you're compiling a C++ file, for plain C it is only enabled by -Wextra. Thomas
On 29/06/2017 20:42, Thomas Huth wrote: > + rame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer Fixed the first character of this line. Paolo > fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "") > fnostack_protector := $(call cc-option, -fno-stack-protector, "")
diff --git a/Makefile b/Makefile index 3ef6ea7..f12b2df 100644 --- a/Makefile +++ b/Makefile @@ -50,9 +50,11 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) -COMMON_CFLAGS += -g -COMMON_CFLAGS += $(autodepend-flags) -Wall -Wwrite-strings -Werror -frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer +COMMON_CFLAGS += -g $(autodepend-flags) +COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized +COMMON_CFLAGS += -Wtype-limits -Wignored-qualifiers -Wunused-but-set-parameter +COMMON_CFLAGS += -Werror + rame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "") fnostack_protector := $(call cc-option, -fno-stack-protector, "") fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "") @@ -67,6 +69,8 @@ COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,) COMMON_CFLAGS += $(fno_pic) $(no_pie) CFLAGS += $(COMMON_CFLAGS) +CFLAGS += -Wmissing-parameter-type -Wold-style-declaration -Woverride-init + CXXFLAGS += $(COMMON_CFLAGS) autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d diff --git a/arm/Makefile.common b/arm/Makefile.common index 03b497b..7e5f527 100644 --- a/arm/Makefile.common +++ b/arm/Makefile.common @@ -24,7 +24,6 @@ phys_base = $(LOADADDR) CFLAGS += -std=gnu99 CFLAGS += -ffreestanding -CFLAGS += -Wextra CFLAGS += -O2 CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common index db5ba62..c4df2e9 100644 --- a/powerpc/Makefile.common +++ b/powerpc/Makefile.common @@ -19,7 +19,6 @@ all: directories $(TEST_DIR)/boot_rom.bin $(tests-all) CFLAGS += -std=gnu99 CFLAGS += -ffreestanding -CFLAGS += -Wextra CFLAGS += -O2 CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib CFLAGS += -Wa,-mregnames diff --git a/s390x/Makefile b/s390x/Makefile index 470cbba..bc099da 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -7,7 +7,6 @@ test_cases: $(tests) CFLAGS += -std=gnu99 CFLAGS += -ffreestanding -CFLAGS += -Wextra CFLAGS += -I $(SRCDIR)/lib CFLAGS += -O2 CFLAGS += -march=z900
Using -Wextra together with -Werror is troublesome - various versions of GCC produce suspicious or even wrong warnings with -Wextra which then become fatal errors with -Werror. For example, the current state of the kvm-unit-tests does not compile anymore with GCC 4.8.1 for s390x due to an inadequate -Wmissing-field-initializers warning. That's annoying for users who just would like to compile the kvm-unit-tests and cumbersome for the developers who have to work around these problems in the source code. So let's replace -Wextra by a saner lists of warning flags that are normally enabled by -Wextra. Since they apparently can be used for building x86, too, the flags are now also applied to the global CFLAGS instead of specifying them for the single targets only. Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: - Now that we've got COMMON_CFLAGS, the remaining flags can be added to the global CFLAGS, too - Removed -Wsign-compare Makefile | 10 +++++++--- arm/Makefile.common | 1 - powerpc/Makefile.common | 1 - s390x/Makefile | 1 - 4 files changed, 7 insertions(+), 6 deletions(-)