Re: [PATCH v2 2/2] kbuild: Remove stale asm-generic wrappers

[James Hogan]

On Thu, Jan 21, 2016 at 12:03:42AM +0000, Paul Burton wrote:
> On Tue, Jan 19, 2016 at 01:37:50PM +0000, James Hogan wrote:
> > When a header file is removed from generic-y (often accompanied by the
> > addition of an arch specific header), the generated wrapper file will
> > persist, and in some cases may still take precedence over the new arch
> > header.
> > 
> > For example commit f1fe2d21f4e1 ("MIPS: Add definitions for extended
> > context") removed ucontext.h from generic-y in arch/mips/include/asm/,
> > and added an arch/mips/include/uapi/asm/ucontext.h. The continued use of
> > the wrapper when reusing a dirty build tree resulted in build failures
> > in arch/mips/kernel/signal.c:
> > 
> > arch/mips/kernel/signal.c: In function âsc_to_extcontextâ:
> > arch/mips/kernel/signal.c:142:12: error: âstruct ucontextâ has no member named âuc_extcontextâ
> >   return &uc->uc_extcontext;
> >             ^
> > 
> > Fix by detecting and removing wrapper headers in generated header
> > directories that do not correspond to a filename in generic-y, genhdr-y,
> > or the newly introduced generated-y.
> > 
> > Reported-by: Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx>
> > Reported-by: Hauke Mehrtens <hauke@xxxxxxxxxx>
> > Reported-by: Heinrich Schuchardt <xypron.glpk@xxxxxx>
> > Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx>
> > Cc: Michal Marek <mmarek@xxxxxxxx>
> > Cc: Arnd Bergmann <arnd@xxxxxxxx>
> > Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> > Cc: Paul Burton <paul.burton@xxxxxxxxxx>
> > Cc: Florian Fainelli <f.fainelli@xxxxxxxxx>
> > Cc: linux-kbuild@xxxxxxxxxxxxxxx
> > Cc: linux-arch@xxxxxxxxxxxxxxx
> > Cc: linux-mips@xxxxxxxxxxxxxx
> > ---
> > Changes in v2:
> > - Rewrite a bit, drawing inspiration from Makefile.headersinst.
> > - Exclude genhdr-y and generated-y (thanks to kbuild test robot).
> > ---
> >  scripts/Makefile.asm-generic | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/Makefile.asm-generic b/scripts/Makefile.asm-generic
> > index 045e0098e962..24c29f16f029 100644
> > --- a/scripts/Makefile.asm-generic
> > +++ b/scripts/Makefile.asm-generic
> > @@ -13,11 +13,26 @@ include scripts/Kbuild.include
> >  # Create output directory if not already present
> >  _dummy := $(shell [ -d $(obj) ] || mkdir -p $(obj))
> >  
> > +# Stale wrappers when the corresponding files are removed from generic-y
> > +# need removing.
> > +generated-y   := $(generic-y) $(genhdr-y) $(generated-y)
> > +all-files     := $(patsubst %, $(obj)/%, $(generated-y))
> > +old-headers   := $(wildcard $(obj)/*.h)
> > +unwanted      := $(filter-out $(all-files),$(old-headers))
> 
> Hi James,
> 
> Thanks a bunch for fixing this!

FTR, I noticed yesterday it fixes a similar case when switching v4.3 to
v4.4 too:

arch/mips/kernel/../../../fs/binfmt_elf.c In function âcreate_elf_tablesâ:
./arch/mips/include/asm/elf.h +425 :14: error: âAT_SYSINFO_EHDRâ undeclared (first use in this function)
  NEW_AUX_ENT(AT_SYSINFO_EHDR,     \
              ^

Due to commit ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
adding uapi/asm/auxvec.h and changing generic-y to header-y. Should
ucontext.h be exported via header-y too?

With these patches, it removes the stale file:
  REMOVE  arch/mips/include/generated/uapi/asm/auxvec.h

> 
> Though is it my sleepy self or are all-files & old-headers misnomers?
> That is, isn't all-files actually a list of headers to be kept, and
> old-headers actually the list of all (header) files?

I've followed the naming in Makefile.headersinst. I read all-files as
"all the files we care about" (i.e. its a combination of several sets of
generated files, hence "all") and old-headers as in "existing headers"
(since it won't include files which haven't been generated yet).

all-files could perhaps be renamed new-headers, but that could be
misleading too.

Cheers
James

> 
> Thanks,
>     Paul
> 
> > +
> >  quiet_cmd_wrap = WRAP    $@
> >  cmd_wrap = echo "\#include <asm-generic/$*.h>" >$@
> >  
> > -all: $(patsubst %, $(obj)/%, $(generic-y))
> > +quiet_cmd_remove = REMOVE  $(unwanted)
> > +cmd_remove = rm -f $(unwanted)
> > +
> > +all: $(patsubst %, $(obj)/%, $(generic-y)) FORCE
> > +	$(if $(unwanted),$(call cmd,remove),)
> >  	@:
> >  
> >  $(obj)/%.h:
> >  	$(call cmd,wrap)
> > +
> > +.PHONY: $(PHONY)
> > +PHONY += FORCE
> > +FORCE: ;
> > -- 
> > 2.4.10
> >

Attachment: signature.asc
Description: Digital signature