Re: [PATCH] kbuild: set EXIT trap before creating temporary directory

[Masahiro Yamada]

On Fri, Aug 5, 2022 at 1:13 AM Nicolas Schier <nicolas@xxxxxxxxx> wrote:
>
> On Thu, Jul 28, 2022 at 12:14:33PM +0900, Masahiro Yamada wrote:
> > Swap the order of 'mkdir' and 'trap' just in case the subshell is
> > interrupted between 'mkdir' and 'trap' although the effect might be
> > subtle.
> >
> > Please not this is not a perfect solution to avoid the left-over tmp
>
> not -> note?


Right.
It is a typo.



>
> > directory. There are more cases that miss to remove the tmp directory,
> > for example:
> >
> >  - When interrupted, dash does not invoke the EXIT trap (bash does)
> >
> >  - 'rm' command might be interrupted before removing the directory
> >
> > I am not addressing all the cases since the tmp directory is harmless
> > after all.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> > ---
> >
> >  scripts/Kconfig.include   | 2 +-
> >  scripts/Makefile.compiler | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
>
> Do you want to add the same for filechk macro in scripts/Kbuild.include?



No, I do not think so.


In filechk

   mkdir -p $(dir $@);


this is a directory for the target.
So, the trap does not delete it.



Otherwise, all call-sites must create the directory manually.



$(timeconst-file): kernel/time/timeconst.bc FORCE
        @mkdir -p $(dir $@)
        $(call filechk,gentimeconst)







> Reviewed-by: Nicolas Schier <nicolas@xxxxxxxxx>
>
> Kind regards,
> Nicolas
>
> >
> > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> > index c1f4222d223d..a0ccceb22cf8 100644
> > --- a/scripts/Kconfig.include
> > +++ b/scripts/Kconfig.include
> > @@ -25,7 +25,7 @@ failure = $(if-success,$(1),n,y)
> >
> >  # $(cc-option,<flag>)
> >  # Return y if the compiler supports <flag>, n otherwise
> > -cc-option = $(success,mkdir .tmp_$$; trap "rm -rf .tmp_$$" EXIT; $(CC) -Werror $(CLANG_FLAGS) $(1) -c -x c /dev/null -o .tmp_$$/tmp.o)
> > +cc-option = $(success,trap "rm -rf .tmp_$$" EXIT; mkdir .tmp_$$; $(CC) -Werror $(CLANG_FLAGS) $(1) -c -x c /dev/null -o .tmp_$$/tmp.o)
> >
> >  # $(ld-option,<flag>)
> >  # Return y if the linker supports <flag>, n otherwise
> > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> > index 86ecd2ac874c..94d0d40cddb3 100644
> > --- a/scripts/Makefile.compiler
> > +++ b/scripts/Makefile.compiler
> > @@ -21,8 +21,8 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$
> >  # automatically cleaned up.
> >  try-run = $(shell set -e;            \
> >       TMP=$(TMPOUT)/tmp;              \
> > -     mkdir -p $(TMPOUT);             \
> >       trap "rm -rf $(TMPOUT)" EXIT;   \
> > +     mkdir -p $(TMPOUT);             \
> >       if ($(1)) >/dev/null 2>&1;      \
> >       then echo "$(2)";               \
> >       else echo "$(3)";               \
> > --
> > 2.34.1
>
> --
> epost|xmpp: nicolas@xxxxxxxxx          irc://oftc.net/nsc
> ↳ gpg: 18ed 52db e34f 860e e9fb  c82b 7d97 0932 55a0 ce7f
>      -- frykten for herren er opphav til kunnskap --



-- 
Best Regards
Masahiro Yamada