Message ID | 6545856e-5cb3-aad0-af87-bea7c0ccafd7@ramsayjones.plus.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
On Mon, Jul 31, 2017 at 11:29 AM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > > >>> Ah, I see what you mean here. You want to distinguish >>> CFLAGS for gcc, CHECKER_FLAGS for sparse specific >>> flags. >> >> Err, ... well, yes and no! :-D >> >> The main idea is to separate the 'additional' flags passed to >> sparse for the $(CHECKER) target - not necessarily for sparse >> specific flags. Yes, my mental model was CHECKER_FLAGS for flags sparse to use. I haven't notice the small detail that -Wvla works for gcc as well. > Hmm, actually the following may be a better patch. what do you > think? Hmm. When I write the BASIC_CFLAGS, I was thinking to use it for the common part of the CFLAGS impact the whole directory or project. Some thing as base line, like architecture related stuff etc, nothing fancy. Each C file might have specific different requirement, like include path etc, Those goes to CFLAGS. The stuff get overwrite more often should have a slightly shorter name for readability. At least that was my mental model. Either one of the three we discuss here is acceptable. If you have to ask me to pick, my order goes to the simplest one line change, then introducing $(CHECKER_CFLAGS), then this one. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31/07/17 16:51, Christopher Li wrote: > On Mon, Jul 31, 2017 at 11:29 AM, Ramsay Jones > <ramsay@ramsayjones.plus.com> wrote: >> >> >>>> Ah, I see what you mean here. You want to distinguish >>>> CFLAGS for gcc, CHECKER_FLAGS for sparse specific >>>> flags. >>> >>> Err, ... well, yes and no! :-D >>> >>> The main idea is to separate the 'additional' flags passed to >>> sparse for the $(CHECKER) target - not necessarily for sparse >>> specific flags. > > Yes, my mental model was CHECKER_FLAGS for flags sparse > to use. I haven't notice the small detail that -Wvla works for gcc > as well. > >> Hmm, actually the following may be a better patch. what do you >> think? > > Hmm. When I write the BASIC_CFLAGS, I was thinking to use it for the > common part of the CFLAGS impact the whole directory or project. Some > thing as base line, like architecture related stuff etc, nothing fancy. I thought of it more as a non-override-able list of cflags, which are required for 'correct'/'desired' behaviour. The CFLAGS are supposed to be user settable from the command line. So, for example, if you wanted to compile c2xml and override the CFLAGS, then you must know to add the output of '$(PKG_CONFIG) --cflags libxml-2.0' manually. > Each C file might have specific different requirement, like include path etc, > Those goes to CFLAGS. The stuff get overwrite more often should have > a slightly shorter name for readability. Hmm, but see above! > At least that was my mental model. Either one of the three we discuss here > is acceptable. If you have to ask me to pick, my order goes to the simplest > one line change, then introducing $(CHECKER_CFLAGS), then this one. I'm OK with any of the three too, but I actually think the last patch is the best. ;-) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 31, 2017 at 12:21 PM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > > > On 31/07/17 16:51, Christopher Li wrote: > I thought of it more as a non-override-able list of cflags, which > are required for 'correct'/'desired' behaviour. The CFLAGS are > supposed to be user settable from the command line. So, for example, > if you wanted to compile c2xml and override the CFLAGS, then you must > know to add the output of '$(PKG_CONFIG) --cflags libxml-2.0' manually. I think CFLAGS are used by makefile implicate rules. So it has pre-existing usage pattern there. Setting CFLAGS on command line is most likely not going to work with most open source project out there, unless you give a very detai and long CFLAGS. > > I'm OK with any of the three too, but I actually think the last > patch is the best. ;-) In that case, will the simplest one line change one a better middle ground then? Obvious there is only one file need to add flags here. Just keep it simple. If more complicate flags setting required later for self checking, we can always introduce the CHECK_CFFLAGS later. Just asking. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Makefile b/Makefile index 64146db..93c7db9 100644 --- a/Makefile +++ b/Makefile @@ -14,6 +14,7 @@ OS = linux CC = gcc CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g CFLAGS += -Wall -Wwrite-strings +BASIC_CFLAGS = LDFLAGS += -g LD = gcc AR = ar @@ -24,7 +25,7 @@ ALL_CFLAGS = $(CFLAGS) $(BASIC_CFLAGS) # # For debugging, put this in local.mk: # -# CFLAGS += -O0 -DDEBUG -g3 -gdwarf-2 +# BASIC_CFLAGS += -O0 -DDEBUG -g3 -gdwarf-2 # HAVE_LIBXML:=$(shell $(PKG_CONFIG) --exists libxml-2.0 2>/dev/null && echo 'yes') @@ -36,7 +37,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)\" @@ -75,7 +76,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 += $(GTK2_CFLAGS) +$(test-inspect_OBJS) $(test-inspect_OBJS:.o=.sc): BASIC_CFLAGS += $(GTK2_CFLAGS) test-inspect_EXTRA_OBJS := $(GTK2_LIBS) else $(warning Your system does not have libgtk2, disabling test-inspect) @@ -196,7 +197,9 @@ ifneq ($(DEP_FILES),) include $(DEP_FILES) endif -c2xml.o c2xml.sc: CFLAGS += $(LIBXML_CFLAGS) +c2xml.o c2xml.sc: BASIC_CFLAGS += $(LIBXML_CFLAGS) + +pre-process.sc: BASIC_CFLAGS += -Wno-vla %.o: %.c $(LIB_H) $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) $<