Portable mktemp calls in the tests

Martin Schwenke martin at meltin.net
Fri Dec 22 04:55:17 UTC 2017


On Fri, 22 Dec 2017 05:43:31 +0100, "Timur I. Bakeyev"
<timur at freebsd.org> wrote:

> Thanks for the positive feedback!
> 
> On Fri, Dec 22, 2017 at 5:05 AM, Martin Schwenke <martin at meltin.net> wrote:
> 
> > Given that I don't know how to write texinfo documentation, I've opened
> > a bug (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29807) against the
> > GNU coreutils package to try to get this documented.  I would encourage
> > you to do the same for FreeBSD's mktemp.

> Can you suggest what would be an adequate explanation/example for this use
> case?

Actually, I've re-read the documentation (for both GNU coreutils and
FreeBSD) and they both mention what happens if no arguments are
passed.  You just need to work out the implications involving
TMPDIR.

So, no documentation problem apart of lack of readability.

> All that said, even though the documentation of mktemp is poor, I think
> > this is an improvement and would be happy to give it a positive
> > review.
> >
> > However, can you please remove the following change (and those related
> > to it) from the mktemp commit?
> >
> > -cat >$tmpeditor <<-'EOF'
> > +cat >${tmpeditor}.sh <<-'EOF'
> >
> > If you really meant to do that then can you please make that change
> > in a separate commit?  :-)
> >  
> 
> That's the direct followup of the:
> 
> -tmpeditor=$(mktemp --suffix .sh -p $STpath/bin samba-tool-editor-XXXXXXXX)
> +tmpeditor=$(TMPDIR=$STpath/bin mktemp samba-tool-editor-XXXXXXXX)
> 
> As '--suffix .sh' is also non-portable extension I've implemented similar
> functionality
> by adding .sh extension to the resulting file. I see, it may have security
> implications
> as we don't verify that resulting file doesn't exist.
> 
> I can remove whole block, as seems, self-test doesn't really use this code
> route.
> 
> Or, do you have better suggestion for this code part?

Oh, sorry!  I missed the --suffix part.  Instead of changing all of the
lines that use this variable I would suggest simply adding an extra
line (after the changed line) that says:

  tmpeditor="${tmpeditor}.sh"

:-)

Note there's another problem!  Where a template is used (and,
therefore, non-option arguments are present) TMPDIR is *not* used (at
least in GNU coreutils).  Therefore, the above will create the file in
the current directory!  The obvious solution is to do something like:

  tmpeditor=$(mktemp "$STpath/bin/samba-tool-editor-XXXXXXXX")
  tmpeditor="${tempeditor}.sh"

This is also true for this change:

    export CTDB_PUBLIC_ADDRESSES=$(TMPDIR="$EVENTSCRIPTS_TESTS_VAR_DIR" \
				        mktemp \
				       "public-addresses-XXXXXXXX")
It will need to be:

    export CTDB_PUBLIC_ADDRESSES=$(mktemp \
		   "${EVENTSCRIPTS_TESTS_VAR_DIR}/public-addresses-XXXXXXXX")

There might other places that behave unexpectedly because a template is
used.  :-(

peace & happiness,
martin



More information about the samba-technical mailing list