Re: [kvm-unit-tests PATCH 3/3] 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>
Subject: Re: [kvm-unit-tests PATCH 3/3] Makefile: use "-Werror" in cc-option
Date: Tue, 15 Oct 2019 00:57:49 -0700	[thread overview]
Message-ID: <CAGG=3QUCRO3YjTFyTt6x2K9Cxi0oz=F1H3H41YYU0n-zc=_Xbw@mail.gmail.com> (raw)
In-Reply-To: <67d3c72f-1537-7765-4b8c-d631859a3204@redhat.com>

On Tue, Oct 15, 2019 at 12:29 AM Thomas Huth <thuth@redhat.com> wrote:
>
> On 10/10/2019 20.35, 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 | 19 +++++++++++--------
> >  1 file changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 32414dc..3ec0458 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -46,30 +46,33 @@ 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, "")
> >  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, "")
> >  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 += $(call cc-option, -Wno-frame-address, "")
>
> I think the old code used ":=" on purpose, so that the test is only done
> once. With your new code, the test is now done each time COMMON_CFLAGS
> gets evaluated, i.e. for each compiled object.
>
> Could you please rewrite this test to use the ":=" detour for all lines
> that call cc-option?
>
Does it rerun the "cc-option" call if the COMMON_CFLAGS is initially
defined with ":="? I ask because the kernel calls its version of
cc-option in the way I have it above, but the original definition of
the variable uses ":=".

-bw