| Message ID | c318f85f-43e5-e902-3032-89eeddcb4ffa@ramsayjones.plus.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
On Sun, Jul 30, 2017 at 7:27 PM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > > Introduce a $(CHECKER_FLAGS) variable to allow adding flags, using > target specific variable assignments, to specific $(CHECKER) command > invocations. In particular, in a new pre-process.cs target, include > '-Wno-vla' in the flags while checking pre-process.c. > > Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Thanks, I want to apply it. But I have some very minor feed backs. > +pre-process.sc: CHECKER_FLAGS += -Wno-vla > + You actually don't need to introduce CHECKER_FLAGS. You can just do: pre-process.sc: CFLAGS += -Wno-vla The target specific CFLAGS will only impact pre-process.sc. I would use CHECKER_FLAGS is for some thing impact all the checker target. > > %.sc: %.c sparse > - $(QUIET_CHECK) $(CHECKER) -c $(ALL_CFLAGS) $< > + $(QUIET_CHECK) $(CHECKER) $(CHECKER_FLAGS) -c $(ALL_CFLAGS) $< If you use target specific CFLAGS, there is no need to use CHECKER_FLAGS. 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 Sun, Jul 30, 2017 at 8:05 PM, Christopher Li <sparse@chrisli.org> wrote: > On Sun, Jul 30, 2017 at 7:27 PM, Ramsay Jones > > You actually don't need to introduce CHECKER_FLAGS. > You can just do: Ah, I see what you mean here. You want to distinguish CFLAGS for gcc, CHECKER_FLAGS for sparse specific flags. It is different than what I have in mind, I think that is fine too. I will apply. 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 01:12, Christopher Li wrote: > On Sun, Jul 30, 2017 at 8:05 PM, Christopher Li <sparse@chrisli.org> wrote: >> On Sun, Jul 30, 2017 at 7:27 PM, Ramsay Jones >> >> You actually don't need to introduce CHECKER_FLAGS. >> You can just do: > > 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. In this case, for example, -W[no]-vla is also a gcc flag, viz: $ make CFLAGS=-Wno-vla pre-process.sc Makefile:69: Your system does not have libxml, disabling c2xml Makefile:82: Your system does not have libgtk2, disabling test-inspect Makefile:102: Your system does not have llvm, disabling sparse-llvm CHECK pre-process.c $ $ rm pre-process.o $ make CFLAGS=-Wno-vla pre-process.o Makefile:69: Your system does not have libxml, disabling c2xml Makefile:82: Your system does not have libgtk2, disabling test-inspect Makefile:102: Your system does not have llvm, disabling sparse-llvm CC pre-process.o $ $ rm pre-process.o $ make CFLAGS=-Wvla pre-process.o Makefile:69: Your system does not have libxml, disabling c2xml Makefile:82: Your system does not have libgtk2, disabling test-inspect Makefile:102: Your system does not have llvm, disabling sparse-llvm CC pre-process.o pre-process.c: In function ‘expand’: pre-process.c:712:9: warning: ISO C90 forbids variable length array ‘args’ [-Wvla] struct arg args[nargs]; ^ pre-process.c: In function ‘dump_macro’: pre-process.c:2019:9: warning: ISO C90 forbids variable length array ‘args’ [-Wvla] struct token *args[nargs]; ^ $ So, yes, I initially didn't have the $(CHECKER_FLAGS) and simply had the following addition: $ git diff diff --git a/Makefile b/Makefile index 64146db..8e4b0ae 100644 --- a/Makefile +++ b/Makefile @@ -198,6 +198,8 @@ endif c2xml.o c2xml.sc: CFLAGS += $(LIBXML_CFLAGS) +pre-process.sc: CFLAGS += -Wno-vla + %.o: %.c $(LIB_H) $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) $< $ ... which works just as well. [Note that gcc and sparse have a different default for -Wvla]. Also, note that we are using cgcc for checker, so non-gcc flags should never get to gcc anyway (or we have a bug). So, if you prefer not to introduce another variable, I would be equally fine with that. However, bear in mind that you will have to remember to add -Wno-vla manually to CFLAGS if you invoke make with CFLAGS on the command line. 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
diff --git a/Makefile b/Makefile index 64146db..c20ea2c 100644 --- a/Makefile +++ b/Makefile @@ -19,6 +19,7 @@ LD = gcc AR = ar PKG_CONFIG = pkg-config CHECKER = ./cgcc -no-compile +CHECKER_FLAGS = ALL_CFLAGS = $(CFLAGS) $(BASIC_CFLAGS) # @@ -198,11 +199,13 @@ endif c2xml.o c2xml.sc: CFLAGS += $(LIBXML_CFLAGS) +pre-process.sc: CHECKER_FLAGS += -Wno-vla + %.o: %.c $(LIB_H) $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) $< %.sc: %.c sparse - $(QUIET_CHECK) $(CHECKER) -c $(ALL_CFLAGS) $< + $(QUIET_CHECK) $(CHECKER) $(CHECKER_FLAGS) -c $(ALL_CFLAGS) $< ALL_OBJS := $(LIB_OBJS) $(foreach p,$(PROGRAMS),$(p).o $($(p)_EXTRA_DEPS)) selfcheck: $(ALL_OBJS:.o=.sc)
Introduce a $(CHECKER_FLAGS) variable to allow adding flags, using target specific variable assignments, to specific $(CHECKER) command invocations. In particular, in a new pre-process.cs target, include '-Wno-vla' in the flags while checking pre-process.c. Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> --- Hi Chris, This is what I had in mind for the selfcheck of pre-process.c. With this patch, selfcheck is clean for me on Linux, but not on cygwin (I will look at fixing that later). Thanks! ATB, Ramsay Jones Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)