On Sat, Feb 16, 2019 at 09:21:39PM +0000, Ramsay Jones wrote: > On 16/02/2019 15:06, Luc Van Oostenryck wrote: > > These variables are only used for selfcheck and as such they > > shouldn't be configurable from the command line or the environment. > > > > So, turn 'CHECKER_FLAGS' into 'selfcheck-flags' and get rid of > > 'CHECKER'. > > > > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx> > > --- > > Heh, I nearly sent you an email last night, which looked like: > > diff --git a/Makefile b/Makefile > index bd2b089..252e030 100644 > --- a/Makefile > +++ b/Makefile > @@ -11,8 +11,7 @@ CFLAGS += -Wall -Wwrite-strings > LD = $(CC) > AR = ar > PKG_CONFIG = pkg-config > -CHECKER = CHECK=./sparse ./cgcc -no-compile > -CHECKER_FLAGS = -Wno-vla > +CHECKER_FLAGS ?= -Wno-vla > > DESTDIR= > PREFIX ?= $(HOME) > @@ -208,7 +207,7 @@ cflags += $($(*)-cflags) $(CPPFLAGS) $(CFLAGS) > > %.sc: %.c sparse > @echo " CHECK $<" > - $(Q) $(CHECKER) $(CHECKER_FLAGS) $(cflags) -c $< > + $(Q) CHECK=./sparse ./cgcc -no-compile $(CHECKER_FLAGS) $(cflags) -c $< > > selfcheck: $(OBJS:.o=.sc) > > ... but I changed my mind just before hitting send! :-D > > As you can see, I also dropped CHECKER, by in-lining the macro > into the target, but kept CHECKER_FLAGS (even allowing it to be > set from the environment). > > I decided that, although I would use it locally (probably from > the local.mk file), it was possible that Uwe (or other packagers) > may need to tweak the value for some exotic platform I have no > access to. (and packagers seem to have a penchant for setting > things from the environment). This presupposes that getting a > clean 'selfcheck' is important to packagers, of course. I don't mind/care keeping CHECKER_FLAGS configurable but is it really needed/useful? > BTW, was the intention to remove the current use of VLAs in the > sparse pre-processor, so that we can get rid of '-Wno-vla' in the > selfcheck target? Not really, IIRC the flag was added because it was decided that the code was OK so. I somehow agree with this. It's clear that for the kernel it's not desirable to use VLAs (for security & quality of the generated code) but here for userspace it's not a real problem. That said, I wouldn't mind to have the 2 VLAs gone away. Best regards, -- Luc
