[PATCH] build: clean up $CFLAGS handling in the makefile

[PATCH] build: clean up $CFLAGS handling in the makefile

[Jeff Layton]

From: Jeff Layton <jlayton@redhat.com>

The fedora packaging tools want to provide a base set of $CFLAGS when
building packages, but that fails when building sparse.

There are a couple of build targets in the makefile that add options to
$CFLAGS.  When we do a build though, we pass $ALL_CFLAGS to the
compiler, and that ends up without those extra options, if CFLAGS= was
passed to the make command.

This patch changes the code to append the options to $ALL_CFLAGS instead
of $CFLAGS. At the same time, passing a CFLAGS argument to make ends up
with the initial CFLAGS assignment being clobbered such that we lose the
options that are assigned to it internally. Fold the internal usage of
CFLAGS into BASIC_CFLAGS. They both just end up as part of ALL_CFLAGS
anyway, so this should be functionally equivalent.

Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

I sent this last week in a reply to a different thread. Re-posting it
here since it got no replies.

diff --git a/Makefile b/Makefile
index d0341764158e..335dcfff54ce 100644
--- a/Makefile
+++ b/Makefile
@@ -12,8 +12,8 @@ OS = linux
 
 
 CC = gcc
-CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g
-CFLAGS += -Wall -Wwrite-strings
+BASIC_CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g
+BASIC_CFLAGS += -Wall -Wwrite-strings
 LDFLAGS += -g
 LD = gcc
 AR = ar
@@ -21,7 +21,7 @@ PKG_CONFIG = pkg-config
 CHECKER = ./cgcc -no-compile
 CHECKER_FLAGS =
 
-ALL_CFLAGS = $(CFLAGS) $(BASIC_CFLAGS)
+ALL_CFLAGS = $(BASIC_CFLAGS) $(CFLAGS)
 #
 # For debugging, put this in local.mk:
 #
@@ -44,7 +44,7 @@ LLVM_CONFIG:=llvm-config
 HAVE_LLVM:=$(shell $(LLVM_CONFIG) --version >/dev/null 2>&1 && echo 'yes')
 
 GCC_BASE := $(shell $(CC) --print-file-name=)
-BASIC_CFLAGS = -DGCC_BASE=\"$(GCC_BASE)\"
+BASIC_CFLAGS += -DGCC_BASE=\"$(GCC_BASE)\"
 
 MULTIARCH_TRIPLET := $(shell $(CC) -print-multiarch 2>/dev/null)
 BASIC_CFLAGS += -DMULTIARCH_TRIPLET=\"$(MULTIARCH_TRIPLET)\"
@@ -83,7 +83,7 @@ PROGRAMS += test-inspect
 INST_PROGRAMS += test-inspect
 test-inspect_EXTRA_DEPS := ast-model.o ast-view.o ast-inspect.o
 test-inspect_OBJS := test-inspect.o $(test-inspect_EXTRA_DEPS)
-$(test-inspect_OBJS) $(test-inspect_OBJS:.o=.sc): CFLAGS += $(GTK_CFLAGS)
+$(test-inspect_OBJS) $(test-inspect_OBJS:.o=.sc): ALL_CFLAGS += $(GTK_CFLAGS)
 test-inspect_EXTRA_OBJS := $(GTK_LIBS)
 else
 $(warning Your system does not have gtk3/gtk2, disabling test-inspect)
@@ -208,7 +208,7 @@ ifneq ($(DEP_FILES),)
 include $(DEP_FILES)
 endif
 
-c2xml.o c2xml.sc: CFLAGS += $(LIBXML_CFLAGS)
+c2xml.o c2xml.sc: ALL_CFLAGS += $(LIBXML_CFLAGS)
 
 pre-process.sc: CHECKER_FLAGS += -Wno-vla
 
-- 
2.13.6

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Luc Van Oostenryck]

On Wed, Oct 25, 2017 at 06:02:22AM -0400, Jeff Layton wrote:
> From: Jeff Layton <jlayton@redhat.com>
> 
> The fedora packaging tools want to provide a base set of $CFLAGS when
> building packages, but that fails when building sparse.
> 
> There are a couple of build targets in the makefile that add options to
> $CFLAGS.  When we do a build though, we pass $ALL_CFLAGS to the
> compiler, and that ends up without those extra options, if CFLAGS= was
> passed to the make command.
> 
> This patch changes the code to append the options to $ALL_CFLAGS instead
> of $CFLAGS. At the same time, passing a CFLAGS argument to make ends up
> with the initial CFLAGS assignment being clobbered such that we lose the
> options that are assigned to it internally. Fold the internal usage of
> CFLAGS into BASIC_CFLAGS. They both just end up as part of ALL_CFLAGS
> anyway, so this should be functionally equivalent.
> 
> Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  Makefile | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> I sent this last week in a reply to a different thread. Re-posting it
> here since it got no replies.
> 

Hi,

I'm fine with this patch but I would prefer to go one step further
and get rid of BASIC_CFLAGS/ALL_CFLAGS while at the same time
being more explicit. I have in mind something like:

From 258efc92cff090bba5ea14a05819514779d7992a Mon Sep 17 00:00:00 2001
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Date: Wed, 25 Oct 2017 14:38:01 +0200
Subject: [PATCH] build: allow to override CFLAGS via the command line

Some distros (fedora) and some developers want to specifiy their
own CFLAGS. OTOH most of CFLAGS's content is automatically
contructed or must not be changed.

This patch, allow to extend CFLAGS via the command line, like:
	make CFLAGS=-some-extra-flags

The patch also get rid of the confusing CFLAGS/BASIC_CFLAGS/ALL_CFLAGS

With this change, target specific CFLAGS can be given either via
	target.o: CFLAGS += -some-specific-flags
or via
	target_CFLAGS = -some-specific-flags

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 Makefile | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index d03417641..f53d66d70 100644
--- a/Makefile
+++ b/Makefile
@@ -11,6 +11,11 @@ endif
 OS = linux
 
 
+# This save any CFLAGS given in the command line
+# and let us use 'CFLAGS' here under
+CLI_CFLAGS := $(CFLAGS)
+override undefine CFLAGS
+
 CC = gcc
 CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g
 CFLAGS += -Wall -Wwrite-strings
@@ -21,7 +26,6 @@ PKG_CONFIG = pkg-config
 CHECKER = ./cgcc -no-compile
 CHECKER_FLAGS =
 
-ALL_CFLAGS = $(CFLAGS) $(BASIC_CFLAGS)
 #
 # For debugging, put this in local.mk:
 #
@@ -44,13 +48,13 @@ LLVM_CONFIG:=llvm-config
 HAVE_LLVM:=$(shell $(LLVM_CONFIG) --version >/dev/null 2>&1 && echo 'yes')
 
 GCC_BASE := $(shell $(CC) --print-file-name=)
-BASIC_CFLAGS = -DGCC_BASE=\"$(GCC_BASE)\"
+CFLAGS += -DGCC_BASE=\"$(GCC_BASE)\"
 
 MULTIARCH_TRIPLET := $(shell $(CC) -print-multiarch 2>/dev/null)
-BASIC_CFLAGS += -DMULTIARCH_TRIPLET=\"$(MULTIARCH_TRIPLET)\"
+CFLAGS += -DMULTIARCH_TRIPLET=\"$(MULTIARCH_TRIPLET)\"
 
 ifeq ($(HAVE_GCC_DEP),yes)
-BASIC_CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
+CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
 endif
 
 DESTDIR=
@@ -71,7 +75,7 @@ ifeq ($(HAVE_LIBXML),yes)
 PROGRAMS+=c2xml
 INST_PROGRAMS+=c2xml
 c2xml_EXTRA_OBJS = `$(PKG_CONFIG) --libs libxml-2.0`
-LIBXML_CFLAGS := $(shell $(PKG_CONFIG) --cflags libxml-2.0)
+c2xml_CFLAGS := $(shell $(PKG_CONFIG) --cflags libxml-2.0)
 else
 $(warning Your system does not have libxml, disabling c2xml)
 endif
@@ -101,7 +105,7 @@ LLVM_LIBS := $(shell $(LLVM_CONFIG) --libs)
 LLVM_LIBS += $(shell $(LLVM_CONFIG) --system-libs 2>/dev/null)
 PROGRAMS += $(LLVM_PROGS)
 INST_PROGRAMS += sparse-llvm sparsec
-sparse-llvm.o: BASIC_CFLAGS += $(LLVM_CFLAGS)
+sparse-llvm_CFLAGS := $(LLVM_CFLAGS)
 sparse-llvm_EXTRA_OBJS := $(LLVM_LIBS) $(LLVM_LDFLAGS)
 else
 $(warning LLVM 3.0 or later required. Your system has version $(LLVM_VERSION) installed.)
@@ -127,7 +131,7 @@ LIB_OBJS= target.o parse.o tokenize.o pre-process.o symbol.o lib.o scope.o \
 LIB_FILE= libsparse.a
 SLIB_FILE= libsparse.so
 
-# If you add $(SLIB_FILE) to this, you also need to add -fpic to BASIC_CFLAGS above.
+# If you add $(SLIB_FILE) to this, you also need to add -fpic to CFLAGS above.
 # Doing so incurs a noticeable performance hit, and Sparse does not have a
 # stable shared library interface, so this does not occur by default.  If you
 # really want a shared library, you may want to build Sparse twice: once
@@ -208,15 +212,15 @@ ifneq ($(DEP_FILES),)
 include $(DEP_FILES)
 endif
 
-c2xml.o c2xml.sc: CFLAGS += $(LIBXML_CFLAGS)

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Christopher Li]

Hi Luc and Jeff,

That get me curious how other package using CFLAGS in their make file
generated by automake and cmake.

I have look at gcc, clang and a few others. My observation is that,
it is up to the package to provide what flags to be override.
There is no fix way to assign and override CFLAGS.

For example, This is one of the line I found in gcc, very typical:

$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS)
$(CPPFLAGS) $(libz_a_CFLAGS) $(CFLAGS)  -c

The  CFLAGS variable does not contain every thing.
Also explicit overriding the CFLAGS in Makefile is rare. There are
some example doing that. But there is a lot of Makefile don't
explicit override CFLAGS at all.

I also found case that CFLAGS was detect and assigned by ./configure
as part of @CFLAGS@ template. For those case, it is very easy
to mistakenly over written CFLAGS set by ./configure.

For cmake. I have seen some package using C_FLAGS and clang
is using CXX.Flags.

Over all I think having ALL_CFLAGS is fine.

BASIC_CFLAGS might be a bad name.
Our current usage case, we can divide that into three groups.
I still want some name space for CFLAGS instead of adding
every thing to CFLAGS.

ALL_CFLAGS = $(CFLAGS) $(PKG_CFLAGS) $(COMMON_CFLAGS)

COMMON_CFLAGS is the CFLAGS shared by all objects.
It serve as the base line.

PKG_CFLAGS is the CFLAGS introduce by  external packages. like LLVM,
libxml, gtk etc.

CFLAGS just for others to assign from the command line.

That should handle all the CFLAGS we have in sparse right now.

Chris



On Wed, Oct 25, 2017 at 7:20 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>> From: Jeff Layton <jlayton@redhat.com>
>>
>> The fedora packaging tools want to provide a base set of $CFLAGS when
>> building packages, but that fails when building sparse.
>>
>> There are a couple of build targets in the makefile that add options to
>> $CFLAGS.  When we do a build though, we pass $ALL_CFLAGS to the
>> compiler, and that ends up without those extra options, if CFLAGS= was
>> passed to the make command.
>>
>> This patch changes the code to append the options to $ALL_CFLAGS instead
>> of $CFLAGS. At the same time, passing a CFLAGS argument to make ends up
>> with the initial CFLAGS assignment being clobbered such that we lose the
>> options that are assigned to it internally. Fold the internal usage of
>> CFLAGS into BASIC_CFLAGS. They both just end up as part of ALL_CFLAGS
>> anyway, so this should be functionally equivalent.
>>
>> Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
>> Signed-off-by: Jeff Layton <jlayton@redhat.com>
>> ---
>>  Makefile | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> I sent this last week in a reply to a different thread. Re-posting it
>> here since it got no replies.
>>
>
> Hi,
>
> I'm fine with this patch but I would prefer to go one step further
> and get rid of BASIC_CFLAGS/ALL_CFLAGS while at the same time
> being more explicit. I have in mind something like:
>
> From 258efc92cff090bba5ea14a05819514779d7992a Mon Sep 17 00:00:00 2001
> From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> Date: Wed, 25 Oct 2017 14:38:01 +0200
> Subject: [PATCH] build: allow to override CFLAGS via the command line
>
> Some distros (fedora) and some developers want to specifiy their
> own CFLAGS. OTOH most of CFLAGS's content is automatically
> contructed or must not be changed.
>
> This patch, allow to extend CFLAGS via the command line, like:
>         make CFLAGS=-some-extra-flags
>
> The patch also get rid of the confusing CFLAGS/BASIC_CFLAGS/ALL_CFLAGS
>
> With this change, target specific CFLAGS can be given either via
>         target.o: CFLAGS += -some-specific-flags
> or via
>         target_CFLAGS = -some-specific-flags
>
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Jeff Layton]

I don't have a particular attachment to the patch I proposed. It just
seemed like the simplest change that fixes the problem I'm having,
without changing anything in the common case where no CFLAGS are passed
to make. Luc's patch would be fine with me too.

It's not clear to me what Chris is suggesting below. It sounds sort of
like you're OK with the patch I originally suggested, but want different
variable names?

Perhaps instead of describing it, you could roll up an alternative patch
that shows what you would prefer to see here?

-- Jeff

On Thu, 2017-10-26 at 03:08 -0700, Christopher Li wrote:
> Hi Luc and Jeff,
> 
> That get me curious how other package using CFLAGS in their make file
> generated by automake and cmake.
> 
> I have look at gcc, clang and a few others. My observation is that,
> it is up to the package to provide what flags to be override.
> There is no fix way to assign and override CFLAGS.
> 
> For example, This is one of the line I found in gcc, very typical:
> 
> $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS)
> $(CPPFLAGS) $(libz_a_CFLAGS) $(CFLAGS)  -c
> 
> The  CFLAGS variable does not contain every thing.
> Also explicit overriding the CFLAGS in Makefile is rare. There are
> some example doing that. But there is a lot of Makefile don't
> explicit override CFLAGS at all.
> 
> I also found case that CFLAGS was detect and assigned by ./configure
> as part of @CFLAGS@ template. For those case, it is very easy
> to mistakenly over written CFLAGS set by ./configure.
> 
> For cmake. I have seen some package using C_FLAGS and clang
> is using CXX.Flags.
> 
> Over all I think having ALL_CFLAGS is fine.
> 
> BASIC_CFLAGS might be a bad name.
> Our current usage case, we can divide that into three groups.
> I still want some name space for CFLAGS instead of adding
> every thing to CFLAGS.
> 
> ALL_CFLAGS = $(CFLAGS) $(PKG_CFLAGS) $(COMMON_CFLAGS)
> 
> COMMON_CFLAGS is the CFLAGS shared by all objects.
> It serve as the base line.
> 
> PKG_CFLAGS is the CFLAGS introduce by  external packages. like LLVM,
> libxml, gtk etc.
> 
> CFLAGS just for others to assign from the command line.
> 
> That should handle all the CFLAGS we have in sparse right now.
> 
> Chris
> 
> 
> 
> On Wed, Oct 25, 2017 at 7:20 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > > From: Jeff Layton <jlayton@redhat.com>
> > > 
> > > The fedora packaging tools want to provide a base set of $CFLAGS when
> > > building packages, but that fails when building sparse.
> > > 
> > > There are a couple of build targets in the makefile that add options to
> > > $CFLAGS.  When we do a build though, we pass $ALL_CFLAGS to the
> > > compiler, and that ends up without those extra options, if CFLAGS= was
> > > passed to the make command.
> > > 
> > > This patch changes the code to append the options to $ALL_CFLAGS instead
> > > of $CFLAGS. At the same time, passing a CFLAGS argument to make ends up
> > > with the initial CFLAGS assignment being clobbered such that we lose the
> > > options that are assigned to it internally. Fold the internal usage of
> > > CFLAGS into BASIC_CFLAGS. They both just end up as part of ALL_CFLAGS
> > > anyway, so this should be functionally equivalent.
> > > 
> > > Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> > > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > > ---
> > >  Makefile | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > I sent this last week in a reply to a different thread. Re-posting it
> > > here since it got no replies.
> > > 
> > 
> > Hi,
> > 
> > I'm fine with this patch but I would prefer to go one step further
> > and get rid of BASIC_CFLAGS/ALL_CFLAGS while at the same time
> > being more explicit. I have in mind something like:
> > 
> > From 258efc92cff090bba5ea14a05819514779d7992a Mon Sep 17 00:00:00 2001
> > From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> > Date: Wed, 25 Oct 2017 14:38:01 +0200
> > Subject: [PATCH] build: allow to override CFLAGS via the command line
> > 
> > Some distros (fedora) and some developers want to specifiy their
> > own CFLAGS. OTOH most of CFLAGS's content is automatically
> > contructed or must not be changed.
> > 
> > This patch, allow to extend CFLAGS via the command line, like:
> >         make CFLAGS=-some-extra-flags
> > 
> > The patch also get rid of the confusing CFLAGS/BASIC_CFLAGS/ALL_CFLAGS
> > 
> > With this change, target specific CFLAGS can be given either via
> >         target.o: CFLAGS += -some-specific-flags
> > or via
> >         target_CFLAGS = -some-specific-flags
> > 
> > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Christopher Li]

On Thu, Oct 26, 2017 at 5:11 AM, Jeff Layton <jlayton@kernel.org> wrote:
>
> It's not clear to me what Chris is suggesting below. It sounds sort of
> like you're OK with the patch I originally suggested, but want different
> variable names?
>
> Perhaps instead of describing it, you could roll up an alternative patch
> that shows what you would prefer to see here?

Sure. I am traveling today so I am not able to send out the patch today.

I will do that.

Chris

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Christopher Li]

On Thu, Oct 26, 2017 at 8:11 PM, Jeff Layton <jlayton@kernel.org> wrote:
>
> Perhaps instead of describing it, you could roll up an alternative patch
> that shows what you would prefer to see here?

Here it is the patch.

I push it on https://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git/log/?h=cflags

Chris

From 9fc170ec5e5814ae858922496dea668bb80dbf34 Mon Sep 17 00:00:00 2001
From: Christopher Li <sparse@chrisli.org>
Date: Sun, 29 Oct 2017 19:16:44 +0800
Subject: [PATCH] Makefile: provide CFLAGS for command line override.

Avoid assign to CFLAGS in Makefile.
Rename BASIC_CFLAGS to COMMON_CFLAGS.
Use PKG_CFLAGS to store external package related cflags.

Signed-off-by: Christopher Li <sparse@chrisli.org>
---
 Makefile | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 76902b7..ef47358 100644
--- a/Makefile
+++ b/Makefile
@@ -12,14 +12,14 @@ OS = linux


 CC = gcc
-CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g
-CFLAGS += -Wall -Wwrite-strings
+COMMON_CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g
+COMMON_CFLAGS += -Wall -Wwrite-strings
 LDFLAGS += -g
 LD = gcc
 AR = ar
 PKG_CONFIG = pkg-config

-ALL_CFLAGS = $(CFLAGS) $(BASIC_CFLAGS)
+ALL_CFLAGS = $(CFLAGS) $(COMMON_CFLAGS) $(PKG_CFLAGS)
 #
 # For debugging, put this in local.mk:
 #
@@ -35,13 +35,13 @@ LLVM_CONFIG:=llvm-config
 HAVE_LLVM:=$(shell $(LLVM_CONFIG) --version >/dev/null 2>&1 && echo 'yes')

 GCC_BASE = $(shell $(CC) --print-file-name=)
-BASIC_CFLAGS = -DGCC_BASE=\"$(GCC_BASE)\"
+COMMON_CFLAGS += -DGCC_BASE=\"$(GCC_BASE)\"

 MULTIARCH_TRIPLET = $(shell $(CC) -print-multiarch 2>/dev/null)
-BASIC_CFLAGS += -DMULTIARCH_TRIPLET=\"$(MULTIARCH_TRIPLET)\"
+COMMON_CFLAGS += -DMULTIARCH_TRIPLET=\"$(MULTIARCH_TRIPLET)\"

 ifeq ($(HAVE_GCC_DEP),yes)
-BASIC_CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
+COMMON_CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
 endif

 DESTDIR=
@@ -72,7 +72,7 @@ GTK2_LIBS := $(shell $(PKG_CONFIG) --libs gtk+-2.0)
 PROGRAMS += test-inspect
 INST_PROGRAMS += test-inspect
 test-inspect_EXTRA_DEPS := ast-model.o ast-view.o ast-inspect.o
-test-inspect.o $(test-inspect_EXTRA_DEPS): BASIC_CFLAGS += $(GTK2_CFLAGS)
+test-inspect.o $(test-inspect_EXTRA_DEPS): PKG_CFLAGS += $(GTK2_CFLAGS)
 test-inspect_EXTRA_OBJS := $(GTK2_LIBS)
 else
 $(warning Your system does not have libgtk2, disabling test-inspect)
@@ -89,7 +89,7 @@ LLVM_LIBS := $(shell $(LLVM_CONFIG) --libs)
 LLVM_LIBS += $(shell $(LLVM_CONFIG) --system-libs 2>/dev/null)
 PROGRAMS += $(LLVM_PROGS)
 INST_PROGRAMS += sparse-llvm sparsec
-sparse-llvm.o: BASIC_CFLAGS += $(LLVM_CFLAGS)
+sparse-llvm.o: PKG_CFLAGS += $(LLVM_CFLAGS)
 sparse-llvm_EXTRA_OBJS := $(LLVM_LIBS) $(LLVM_LDFLAGS)
 else
 $(warning LLVM 3.0 or later required. Your system has version
$(LLVM_VERSION) installed.)
-- 
2.13.6

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Luc Van Oostenryck]

On Sun, Oct 29, 2017 at 07:32:39PM +0800, Christopher Li wrote:
> On Thu, Oct 26, 2017 at 8:11 PM, Jeff Layton <jlayton@kernel.org> wrote:
> >
> > Perhaps instead of describing it, you could roll up an alternative patch
> > that shows what you would prefer to see here?
> 
> Here it is the patch.
> 
> I push it on https://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git/log/?h=cflags
> 
> Chris
> 
> From 9fc170ec5e5814ae858922496dea668bb80dbf34 Mon Sep 17 00:00:00 2001
> From: Christopher Li <sparse@chrisli.org>
> Date: Sun, 29 Oct 2017 19:16:44 +0800
> Subject: [PATCH] Makefile: provide CFLAGS for command line override.
> 
> Avoid assign to CFLAGS in Makefile.
> Rename BASIC_CFLAGS to COMMON_CFLAGS.
> Use PKG_CFLAGS to store external package related cflags.

The goal of the initial patch, as stated by Jeff, was:
"The fedora packaging tools want to *override* $CFLAGS when building sparse"

This version doesn't allow to override CFLAGS.
See what happens with 'make CFLAGS=-O3' for example.

-- Luc

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Christopher Li]

On Mon, Oct 30, 2017 at 1:28 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> The goal of the initial patch, as stated by Jeff, was:
> "The fedora packaging tools want to *override* $CFLAGS when building sparse"
>
> This version doesn't allow to override CFLAGS.
> See what happens with 'make CFLAGS=-O3' for example.

The original patch proposed by Jeff does not allow usage of
"make CFLAGS=-O3" either.  I take a look at your patch, it behave
the same way in this regard. Do you mean I should change the
commit title to *append* CFLAGS instead of override?

May be Jeff can clarify the intention?

BTW, I am actually leaning toward just expose some thing like
CLI_CFLAGS to be override by package builders. I am not that
convince override CFLAGS is established stander practices from
the project that I have look at.

On the other hand, it is trivial to avoid using CFLAGS in sparse
Makefile. If Jeff really want CFLAGS instead of CLI_CFLAGS,
it is not a big a deal.

Chris

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Josh Triplett]

On Mon, Oct 30, 2017 at 05:53:49AM +0800, Christopher Li wrote:
> On Mon, Oct 30, 2017 at 1:28 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > The goal of the initial patch, as stated by Jeff, was:
> > "The fedora packaging tools want to *override* $CFLAGS when building sparse"
> >
> > This version doesn't allow to override CFLAGS.
> > See what happens with 'make CFLAGS=-O3' for example.
> 
> The original patch proposed by Jeff does not allow usage of
> "make CFLAGS=-O3" either.  I take a look at your patch, it behave
> the same way in this regard. Do you mean I should change the
> commit title to *append* CFLAGS instead of override?
> 
> May be Jeff can clarify the intention?
> 
> BTW, I am actually leaning toward just expose some thing like
> CLI_CFLAGS to be override by package builders. I am not that
> convince override CFLAGS is established stander practices from
> the project that I have look at.

`make CFLAGS='-arbitrary -flags'` is quite common, and people generally
expect it to work, though it isn't *shocking* when it breaks. Though the
exact boundary for what should get overridden by that and what should
remain seems a bit fuzzy. Roughly speaking, if the application normally
wants to build with "-O2 -ffooish-bar -Wall -Wanother -I/some/path" ,
then `make CFLAGS='-Os -arbitrary -flags'` should build with "-Os
-arbitrary -flags -Wall -Wanother -Isome-path`. I think.

> On the other hand, it is trivial to avoid using CFLAGS in sparse
> Makefile. If Jeff really want CFLAGS instead of CLI_CFLAGS,
> it is not a big a deal.

I think it's worth not putting anything into CFLAGS that would break the
build if missing.

For instance, git's Makefile specifically says "# CFLAGS and LDFLAGS are
for the users to override from the command line.", and then just has
"CFLAGS = -g -O2 -Wall". It then has additional variables like
ALL_CFLAGS that include both CFLAGS and other things.

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Christopher Li]

On Mon, Oct 30, 2017 at 6:11 AM, Josh Triplett <josh@joshtriplett.org> wrote:

> `make CFLAGS='-arbitrary -flags'` is quite common, and people generally
> expect it to work, though it isn't *shocking* when it breaks. Though the
> exact boundary for what should get overridden by that and what should
> remain seems a bit fuzzy. Roughly speaking, if the application normally
> wants to build with "-O2 -ffooish-bar -Wall -Wanother -I/some/path" ,
> then `make CFLAGS='-Os -arbitrary -flags'` should build with "-Os
> -arbitrary -flags -Wall -Wanother -Isome-path`. I think.

Thanks for the feed back. Yes, there is some project do that.
Those project also document it.
On the other hand, some cmake project don't use CFLAGS
at all as far as I can tell.


So I think as long as the project provide clear document for what
it expect to be override from command line, the package builder
use it consistently. That is fine.

> I think it's worth not putting anything into CFLAGS that would break the
> build if missing.
>
> For instance, git's Makefile specifically says "# CFLAGS and LDFLAGS are
> for the users to override from the command line.", and then just has
> "CFLAGS = -g -O2 -Wall". It then has additional variables like

We can do that for sparse if some one want to use it his way.
On the other hand, I think it is up to the package to decide what
to be allow to override.

The package builder should not take it for granted that very package
should work in this way. Some project don't use CFLAGS at all.

Provide the interface as needed, then document it. That seems to
be the right thing to do.

Chris

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Luc Van Oostenryck]

On Mon, Oct 30, 2017 at 05:53:49AM +0800, Christopher Li wrote:
> On Mon, Oct 30, 2017 at 1:28 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > The goal of the initial patch, as stated by Jeff, was:
> > "The fedora packaging tools want to *override* $CFLAGS when building sparse"
> >
> > This version doesn't allow to override CFLAGS.
> > See what happens with 'make CFLAGS=-O3' for example.
> 
> The original patch proposed by Jeff does not allow usage of
> "make CFLAGS=-O3" either.  I take a look at your patch, it behave
> the same way in this regard. Do you mean I should change the
> commit title to *append* CFLAGS instead of override?

Since for flags like -O[0123] but also for all flags like -f[no-]xxx
and all sort of others settings in GCC and in a multitude of others
programs it's the last value that matters, appending to CFLAGS *is*
the usual way to *override* previous settings.
Jeff's patch allowed that, mine too but yours not.
 
-- Luc

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Christopher Li]

On Mon, Oct 30, 2017 at 1:40 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Since for flags like -O[0123] but also for all flags like -f[no-]xxx
> and all sort of others settings in GCC and in a multitude of others
> programs it's the last value that matters, appending to CFLAGS *is*
> the usual way to *override* previous settings.
> Jeff's patch allowed that, mine too but yours not.

Ah, I see. Thanks the for catching that. I did not notice that subtle
detail. I just need to reorder the CFLAGS as the last variable then.

Chris

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Jeff Layton]

On Sun, 2017-10-29 at 19:32 +0800, Christopher Li wrote:
> On Thu, Oct 26, 2017 at 8:11 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > Perhaps instead of describing it, you could roll up an alternative patch
> > that shows what you would prefer to see here?
> 
> Here it is the patch.
> 
> I push it on https://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git/log/?h=cflags
> 
> Chris
> 
> From 9fc170ec5e5814ae858922496dea668bb80dbf34 Mon Sep 17 00:00:00 2001
> From: Christopher Li <sparse@chrisli.org>
> Date: Sun, 29 Oct 2017 19:16:44 +0800
> Subject: [PATCH] Makefile: provide CFLAGS for command line override.
> 
> Avoid assign to CFLAGS in Makefile.
> Rename BASIC_CFLAGS to COMMON_CFLAGS.
> Use PKG_CFLAGS to store external package related cflags.
> 
> Signed-off-by: Christopher Li <sparse@chrisli.org>
> ---
>  Makefile | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 76902b7..ef47358 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -12,14 +12,14 @@ OS = linux
> 
> 
>  CC = gcc
> -CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g
> -CFLAGS += -Wall -Wwrite-strings
> +COMMON_CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g
> +COMMON_CFLAGS += -Wall -Wwrite-strings
>  LDFLAGS += -g
>  LD = gcc
>  AR = ar
>  PKG_CONFIG = pkg-config
> 
> -ALL_CFLAGS = $(CFLAGS) $(BASIC_CFLAGS)
> +ALL_CFLAGS = $(CFLAGS) $(COMMON_CFLAGS) $(PKG_CFLAGS)

As Luc and Josh pointed out, I think we want $(CFLAGS) last here. That
allows you to pass in options that can supersede what the makefile puts
in there.

>  #
>  # For debugging, put this in local.mk:
>  #
> @@ -35,13 +35,13 @@ LLVM_CONFIG:=llvm-config
>  HAVE_LLVM:=$(shell $(LLVM_CONFIG) --version >/dev/null 2>&1 && echo 'yes')
> 
>  GCC_BASE = $(shell $(CC) --print-file-name=)
> -BASIC_CFLAGS = -DGCC_BASE=\"$(GCC_BASE)\"
> +COMMON_CFLAGS += -DGCC_BASE=\"$(GCC_BASE)\"
> 
>  MULTIARCH_TRIPLET = $(shell $(CC) -print-multiarch 2>/dev/null)
> -BASIC_CFLAGS += -DMULTIARCH_TRIPLET=\"$(MULTIARCH_TRIPLET)\"
> +COMMON_CFLAGS += -DMULTIARCH_TRIPLET=\"$(MULTIARCH_TRIPLET)\"
> 
>  ifeq ($(HAVE_GCC_DEP),yes)
> -BASIC_CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
> +COMMON_CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
>  endif
> 
>  DESTDIR=
> @@ -72,7 +72,7 @@ GTK2_LIBS := $(shell $(PKG_CONFIG) --libs gtk+-2.0)
>  PROGRAMS += test-inspect
>  INST_PROGRAMS += test-inspect
>  test-inspect_EXTRA_DEPS := ast-model.o ast-view.o ast-inspect.o
> -test-inspect.o $(test-inspect_EXTRA_DEPS): BASIC_CFLAGS += $(GTK2_CFLAGS)
> +test-inspect.o $(test-inspect_EXTRA_DEPS): PKG_CFLAGS += $(GTK2_CFLAGS)
>  test-inspect_EXTRA_OBJS := $(GTK2_LIBS)
>  else
>  $(warning Your system does not have libgtk2, disabling test-inspect)
> @@ -89,7 +89,7 @@ LLVM_LIBS := $(shell $(LLVM_CONFIG) --libs)
>  LLVM_LIBS += $(shell $(LLVM_CONFIG) --system-libs 2>/dev/null)
>  PROGRAMS += $(LLVM_PROGS)
>  INST_PROGRAMS += sparse-llvm sparsec
> -sparse-llvm.o: BASIC_CFLAGS += $(LLVM_CFLAGS)
> +sparse-llvm.o: PKG_CFLAGS += $(LLVM_CFLAGS)
>  sparse-llvm_EXTRA_OBJS := $(LLVM_LIBS) $(LLVM_LDFLAGS)
>  else
>  $(warning LLVM 3.0 or later required. Your system has version
> $(LLVM_VERSION) installed.)

...other than that, I'm fine with this. Assuming you fix that, you can
add:

Acked-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Christopher Li]

On Tue, Oct 31, 2017 at 2:57 PM, Jeff Layton <jlayton@kernel.org> wrote:
> As Luc and Josh pointed out, I think we want $(CFLAGS) last here. That
> allows you to pass in options that can supersede what the makefile puts
> in there.
> ...other than that, I'm fine with this. Assuming you fix that, you can
> add:

I update the patch here and git url:
https://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git/log/?h=cflags-v2

Chris

From 55496234238e127c05807550fb432e1125a85710 Mon Sep 17 00:00:00 2001
From: Christopher Li <sparse@chrisli.org>
Date: Sun, 29 Oct 2017 19:16:44 +0800
Subject: [PATCH] Makefile: provide CFLAGS for command line override.

Avoid assign to CFLAGS in Makefile.
Rename BASIC_CFLAGS to COMMON_CFLAGS.
Use PKG_CFLAGS to store external package related cflags.

Signed-off-by: Christopher Li <sparse@chrisli.org>
Acked-by: Jeff Layton <jlayton@kernel.org>
---
 Makefile | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index d0341764..66cb1ae1 100644
--- a/Makefile
+++ b/Makefile
@@ -12,8 +12,8 @@ OS = linux


 CC = gcc
-CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g
-CFLAGS += -Wall -Wwrite-strings
+COMMON_CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g
+COMMON_CFLAGS += -Wall -Wwrite-strings
 LDFLAGS += -g
 LD = gcc
 AR = ar
@@ -21,7 +21,7 @@ PKG_CONFIG = pkg-config
 CHECKER = ./cgcc -no-compile
 CHECKER_FLAGS =

-ALL_CFLAGS = $(CFLAGS) $(BASIC_CFLAGS)
+ALL_CFLAGS = $(COMMON_CFLAGS) $(PKG_CFLAGS) $(CFLAGS)
 #
 # For debugging, put this in local.mk:
 #
@@ -44,13 +44,13 @@ LLVM_CONFIG:=llvm-config
 HAVE_LLVM:=$(shell $(LLVM_CONFIG) --version >/dev/null 2>&1 && echo 'yes')

 GCC_BASE := $(shell $(CC) --print-file-name=)
-BASIC_CFLAGS = -DGCC_BASE=\"$(GCC_BASE)\"
+COMMON_CFLAGS += -DGCC_BASE=\"$(GCC_BASE)\"

 MULTIARCH_TRIPLET := $(shell $(CC) -print-multiarch 2>/dev/null)
-BASIC_CFLAGS += -DMULTIARCH_TRIPLET=\"$(MULTIARCH_TRIPLET)\"
+COMMON_CFLAGS += -DMULTIARCH_TRIPLET=\"$(MULTIARCH_TRIPLET)\"

 ifeq ($(HAVE_GCC_DEP),yes)
-BASIC_CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
+COMMON_CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
 endif

 DESTDIR=
@@ -83,7 +83,7 @@ PROGRAMS += test-inspect
 INST_PROGRAMS += test-inspect
 test-inspect_EXTRA_DEPS := ast-model.o ast-view.o ast-inspect.o
 test-inspect_OBJS := test-inspect.o $(test-inspect_EXTRA_DEPS)
-$(test-inspect_OBJS) $(test-inspect_OBJS:.o=.sc): CFLAGS += $(GTK_CFLAGS)
+$(test-inspect_OBJS) $(test-inspect_OBJS:.o=.sc): PKG_CFLAGS += $(GTK_CFLAGS)
 test-inspect_EXTRA_OBJS := $(GTK_LIBS)
 else
 $(warning Your system does not have gtk3/gtk2, disabling test-inspect)
@@ -101,7 +101,7 @@ LLVM_LIBS := $(shell $(LLVM_CONFIG) --libs)
 LLVM_LIBS += $(shell $(LLVM_CONFIG) --system-libs 2>/dev/null)
 PROGRAMS += $(LLVM_PROGS)
 INST_PROGRAMS += sparse-llvm sparsec
-sparse-llvm.o: BASIC_CFLAGS += $(LLVM_CFLAGS)
+sparse-llvm.o: PKG_CFLAGS += $(LLVM_CFLAGS)
 sparse-llvm_EXTRA_OBJS := $(LLVM_LIBS) $(LLVM_LDFLAGS)
 else
 $(warning LLVM 3.0 or later required. Your system has version
$(LLVM_VERSION) installed.)
-- 
2.13.6

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Luc Van Oostenryck]

On Tue, Oct 31, 2017 at 08:56:47PM -0400, Christopher Li wrote:
> On Tue, Oct 31, 2017 at 2:57 PM, Jeff Layton <jlayton@kernel.org> wrote:
> > As Luc and Josh pointed out, I think we want $(CFLAGS) last here. That
> > allows you to pass in options that can supersede what the makefile puts
> > in there.
> > ...other than that, I'm fine with this. Assuming you fix that, you can
> > add:
> 
> I update the patch here and git url:
> https://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git/log/?h=cflags-v2

This is still not correct. It still doesn't allow to compile sparse
via 'make CFLAGS=<some extra flags, possibly none>'.

Visibly this hasn't been tested.

-- Luc

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Christopher Li]

On Wed, Nov 1, 2017 at 10:17 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:

> This is still not correct. It still doesn't allow to compile sparse
> via 'make CFLAGS=<some extra flags, possibly none>'.
>
> Visibly this hasn't been tested.

Sorry about that.

There is one spot I miss after solving conflict of rebase master.

How about this one:

https://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git/log/?h=cflags-v3

Thanks

Chris

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Luc Van Oostenryck]

On Sun, Nov 05, 2017 at 08:45:53AM +0800, Christopher Li wrote:
> On Wed, Nov 1, 2017 at 10:17 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> 
> > This is still not correct. It still doesn't allow to compile sparse
> > via 'make CFLAGS=<some extra flags, possibly none>'.
> >
> > Visibly this hasn't been tested.
> 
> Sorry about that.
> 
> There is one spot I miss after solving conflict of rebase master.

You rejected the patch, wrote your own version and you had to rebase it
and you had a conflict?
 
> How about this one:
> 
> https://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git/log/?h=cflags-v3

Much better!
Now you can give it a try at 'make CFLAGS=-O3 selfcheck'.


-- Luc Van Oostenryck 

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Christopher Li]

On Mon, Nov 6, 2017 at 12:57 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Much better!
> Now you can give it a try at 'make CFLAGS=-O3 selfcheck'.
>
The selfcheck on sparse-llvm.sc is independent of the CFLAGS changes.
BTW, I never saw the brakage on FC 26. because "llvm-config --cflags" does not
add new search path.

Here is the V4:

https://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git/commit/?h=cflags-v4

Chris

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Luc Van Oostenryck]

On Fri, Nov 10, 2017 at 05:10:48AM +0800, Christopher Li wrote:
> On Mon, Nov 6, 2017 at 12:57 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > Much better!
> > Now you can give it a try at 'make CFLAGS=-O3 selfcheck'.
> >
> The selfcheck on sparse-llvm.sc is independent of the CFLAGS changes.

On the contrary, it's very much at the core of the way the
different CFLAGS variable are used.

Everywhere you will use something like:
   <sometarget>.o: <some CFLAGS variant> += <something>
you will also need to add <sometarget>.sc
It's the way you solve the problem and it may seems to you 
independent of the CFLAGS changes and yet in my version of
the CFLAGS changes this bug was solved automatically by how
the CFLAGS were used (I discovered the bug while writting the
patch; you know this "ohoh, this can't possibly be correct").

But well, more than enough time have already spend on these CFLAGS.

-- Luc

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Christopher Li]

On Fri, Nov 10, 2017 at 5:26 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>> The selfcheck on sparse-llvm.sc is independent of the CFLAGS changes.
>
> On the contrary, it's very much at the core of the way the
> different CFLAGS variable are used.

I mean the bug is triggerable even before the CFLAGS patch applied.

>
> Everywhere you will use something like:
>    <sometarget>.o: <some CFLAGS variant> += <something>
> you will also need to add <sometarget>.sc
> It's the way you solve the problem and it may seems to you
> independent of the CFLAGS changes and yet in my version of
> the CFLAGS changes this bug was solved automatically by how
> the CFLAGS were used (I discovered the bug while writting the
> patch; you know this "ohoh, this can't possibly be correct").

I see. You avoid using target specific variables. That might be
a good idea. I was looking for the smallest fix to so that
don't impact too much on the other Makefile changes.

I don't like to have CFLAGS += for every thing though.
It only works if the options always append from the tail.
If it is order sensitive, need to insert in the middle, then
you will need to have sub variable to group from any way,
like your CPPFLAGS.

Chris

Re: [PATCH] build: clean up $CFLAGS handling in the makefile

[Luc Van Oostenryck]

On Thu, Nov 9, 2017 at 11:18 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Fri, Nov 10, 2017 at 5:26 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>>> The selfcheck on sparse-llvm.sc is independent of the CFLAGS changes.
>>
>> On the contrary, it's very much at the core of the way the
>> different CFLAGS variable are used.
>
> I mean the bug is triggerable even before the CFLAGS patch applied.
>
>>
>> Everywhere you will use something like:
>>    <sometarget>.o: <some CFLAGS variant> += <something>
>> you will also need to add <sometarget>.sc
>> It's the way you solve the problem and it may seems to you
>> independent of the CFLAGS changes and yet in my version of
>> the CFLAGS changes this bug was solved automatically by how
>> the CFLAGS were used (I discovered the bug while writting the
>> patch; you know this "ohoh, this can't possibly be correct").
>
> I see. You avoid using target specific variables. That might be
> a good idea. I was looking for the smallest fix to so that
> don't impact too much on the other Makefile changes.
>
> I don't like to have CFLAGS += for every thing though.
> It only works if the options always append from the tail.
> If it is order sensitive, need to insert in the middle, then
> you will need to have sub variable to group from any way,
> like your CPPFLAGS.

In the big makefile cleanup series, I've done slightly
differently but yes, no more target specific vars,
only <target>_CFLAGS and using 'cflags' for
flags private to sparse.
Aesthetically, it's not what I found the most pleasing
but this does well the job (SPARSE_CFLAGS, _CFLAGS,
C_FLAGS,  ... could have been used too).

-- Luc