Re: [kvm-unit-tests PATCH v3 3/6] Makefile: use "-Werror" in cc-option

From: Bill Wendling <morbo@google.com>
To: Thomas Huth <thuth@redhat.com>
Cc: kvm list <kvm@vger.kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Jim Mattson <jmattson@google.com>,
	Sean Christopherson <sean.j.christopherson@intel.com>
Subject: Re: [kvm-unit-tests PATCH v3 3/6] Makefile: use "-Werror" in cc-option
Date: Fri, 8 Nov 2019 12:36:13 -0800	[thread overview]
Message-ID: <CAGG=3QXgCseTiZ-5qeUqwE6L4E8DxNr8nbthacgH-BAQCn6e3g@mail.gmail.com> (raw)
In-Reply-To: <e4c7494f-1ca5-fe6e-7238-08307fa3bccb@redhat.com>

I sent out another set of patches for the -Werror, because I didn't
see the ones I posted on the list and thought they got lost.

On Fri, Nov 8, 2019 at 12:43 AM Thomas Huth <thuth@redhat.com> wrote:
>
> On 30/10/2019 22.04, Bill Wendling wrote:
> > The "cc-option" macro should use "-Werror" to determine if a flag is
> > supported. Otherwise the test may not return a nonzero result. Also
> > conditionalize some of the warning flags which aren't supported by
> > clang.
> >
> > Signed-off-by: Bill Wendling <morbo@google.com>
> > ---
> >   Makefile | 16 ++++++++++------
> >   1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 32414dc..6201c45 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -46,13 +46,13 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
> >   # cc-option
> >   # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
> >
> > -cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
> > +cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> >                 > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
> >
> >   COMMON_CFLAGS += -g $(autodepend-flags)
> > -COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
> > -COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
> > -COMMON_CFLAGS += -Werror
> > +COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
> > +COMMON_CFLAGS += -Wignored-qualifiers -Werror
> > +
> >   frame-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, "")
> > @@ -60,16 +60,20 @@ fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
> >   wno_frame_address := $(call cc-option, -Wno-frame-address, "")
> >   fno_pic := $(call cc-option, -fno-pic, "")
> >   no_pie := $(call cc-option, -no-pie, "")
> > +wclobbered := $(call cc-option, -Wclobbered, "")
> > +wunused_but_set_parameter := $(call cc-option, -Wunused-but-set-parameter, "")
> > +
> >   COMMON_CFLAGS += $(fomit_frame_pointer)
> >   COMMON_CFLAGS += $(fno_stack_protector)
> >   COMMON_CFLAGS += $(fno_stack_protector_all)
> >   COMMON_CFLAGS += $(wno_frame_address)
> >   COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
> >   COMMON_CFLAGS += $(fno_pic) $(no_pie)
> > +COMMON_CFLAGS += $(wclobbered)
> > +COMMON_CFLAGS += $(wunused_but_set_parameter)
>
> Looks good to me up to this point here...
>
> >   CFLAGS += $(COMMON_CFLAGS)
> > -CFLAGS += -Wmissing-parameter-type -Wold-style-declaration -Woverride-init
> > -CFLAGS += -Wmissing-prototypes -Wstrict-prototypes
> > +CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
>
> ... but I think this hunk rather belongs to the next patch instead?
>
>   Thomas
>