From: Matteo Croce <mcroce@redhat.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: netdev <netdev@vger.kernel.org>, David Ahern <dsahern@kernel.org>
Subject: Re: [PATCH iproute2] testsuite: don't clobber /tmp
Date: Tue, 25 Jun 2019 16:39:49 +0200 [thread overview]
Message-ID: <CAGnkfhzjT1he+77vRC7p_Y7U5L7AksDpkss2TwZcR_xxxGhgSA@mail.gmail.com> (raw)
In-Reply-To: <CAGnkfhyS64WA+947iQFwA9+=yS6Zk856SWBR9Zy7w90xhBmC=Q@mail.gmail.com>
On Thu, Jun 13, 2019 at 7:15 PM Matteo Croce <mcroce@redhat.com> wrote:
>
> On Wed, Jun 12, 2019 at 8:20 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Wed, 12 Jun 2019 19:32:29 +0200
> > Matteo Croce <mcroce@redhat.com> wrote:
> >
> > > On Wed, Jun 12, 2019 at 6:04 PM Matteo Croce <mcroce@redhat.com> wrote:
> > > >
> > > > On Wed, Jun 12, 2019 at 5:55 PM Stephen Hemminger
> > > > <stephen@networkplumber.org> wrote:
> > > > >
> > > > > On Tue, 11 Jun 2019 20:03:26 +0200
> > > > > Matteo Croce <mcroce@redhat.com> wrote:
> > > > >
> > > > > > Even if not running the testsuite, every build will leave
> > > > > > a stale tc_testkenv.* file in the system temp directory.
> > > > > > Conditionally create the temp file only if we're running the testsuite.
> > > > > >
> > > > > > Signed-off-by: Matteo Croce <mcroce@redhat.com>
> > > > > > ---
> > > > > > testsuite/Makefile | 5 ++++-
> > > > > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/testsuite/Makefile b/testsuite/Makefile
> > > > > > index 7f247bbc..5353244b 100644
> > > > > > --- a/testsuite/Makefile
> > > > > > +++ b/testsuite/Makefile
> > > > > > @@ -14,7 +14,9 @@ TESTS_DIR := $(dir $(TESTS))
> > > > > >
> > > > > > IPVERS := $(filter-out iproute2/Makefile,$(wildcard iproute2/*))
> > > > > >
> > > > > > -KENVFN := $(shell mktemp /tmp/tc_testkenv.XXXXXX)
> > > > > > +ifeq ($(MAKECMDGOALS),alltests)
> > > > > > + KENVFN := $(shell mktemp /tmp/tc_testkenv.XXXXXX)
> > > > > > +endif
> > > > > > ifneq (,$(wildcard /proc/config.gz))
> > > > > > KCPATH := /proc/config.gz
> > > > > > else
> > > > > > @@ -94,3 +96,4 @@ endif
> > > > > > rm "$$TMP_ERR" "$$TMP_OUT"; \
> > > > > > sudo dmesg > $(RESULTS_DIR)/$@.$$o.dmesg; \
> > > > > > done
> > > > > > + @$(RM) $(KENVFN)
> > > > >
> > > > > My concern is that there are several targets in this one Makefile.
> > > > >
> > > > > Why not use -u which gives name but does not create the file?
> > > >
> > > > As the manpage says, this is unsafe, as a file with the same name can
> > > > be created in the meantime.
> > > > Another option is to run the mktemp in the target shell, but this will
> > > > require to escape every single end of line to make it a single shell
> > > > command, e.g.:
> > > >
> > > > KENVFN=$$(mktemp /tmp/tc_testkenv.XXXXXX); \
> > > > if [ "$(KCPATH)" = "/proc/config.gz" ]; then \
> > > > gunzip -c $(KCPATH) >$$KENVFN; \
> > > > ...
> > > > done ; \
> > > > $(RM) $$KENVFN
> > > >
> > > > --
> > > > Matteo Croce
> > > > per aspera ad upstream
> > >
> > > Anyway, looking for "tc" instead of "alltests" is probably better, as
> > > it only runs mktemp when at least the tc test is selected, both
> > > manually or via make check from topdir, eg.g
> > >
> > > ifeq ($(MAKECMDGOALS),tc)
> > >
> > > Do you agree?
> >
> > Why use /tmp at all for this config file?
>
> To me any path could work, both /tmp or in the current dir, I have no
> preference.
> The important thing is to remove them wherever they are, as clobbering
> the build dir is bad as messing /tmp.
>
> Anyway, I double checked, and the only target which uses that
> temporary file is 'alltests' so, if the path is ok, I think that the
> condition "ifeq ($(MAKECMDGOALS),alltests)" is the only one which
> fixes the issue and keeps the behaviour unaltered.
> I did some quick tests and it works for me.
>
> Bye,
> --
> Matteo Croce
> per aspera ad upstream
Hi,
any more thoughts about this patch?
--
Matteo Croce
per aspera ad upstream
next prev parent reply other threads:[~2019-06-25 14:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-11 18:03 [PATCH iproute2] testsuite: don't clobber /tmp Matteo Croce
2019-06-12 15:53 ` Stephen Hemminger
2019-06-12 16:04 ` Matteo Croce
2019-06-12 17:32 ` Matteo Croce
2019-06-12 18:19 ` Stephen Hemminger
2019-06-13 17:15 ` Matteo Croce
2019-06-25 14:39 ` Matteo Croce [this message]
2019-10-12 13:32 ` Matteo Croce
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAGnkfhzjT1he+77vRC7p_Y7U5L7AksDpkss2TwZcR_xxxGhgSA@mail.gmail.com \
--to=mcroce@redhat.com \
--cc=dsahern@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).