Portable mktemp calls in the tests

Rowland Penny rpenny at samba.org
Tue Jan 2 09:56:48 UTC 2018


On Tue, 2 Jan 2018 12:54:18 +1100
Martin Schwenke via samba-technical <samba-technical at lists.samba.org>
wrote:

> Hi Timur,
> 
> On Fri, 22 Dec 2017 06:46:24 +0100, "Timur I. Bakeyev"
> <timur at freebsd.org> wrote:
> 
> > On Fri, Dec 22, 2017 at 5:55 AM, Martin Schwenke
> > <martin at meltin.net> wrote:
> > 
> > > 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:
> > >
> > > 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"
> > >  
> > 
> > Sounds smarter way of doing it :)
> > 
> > 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"
> > >  
> > 
> > I've tested it, but seems to forgot to properly add it everywhere...
> > 
> > Seems, construction like:
> > 
> > TMPDIR=/var/tmp mktemp -t blah-xxxxx
> > 
> > Note '-t' flag ^
> > 
> > Works both for FreeBSD and Linux(at least, Debian) as intended.
> > Creation of directories
> > with extra '-d' also works.
> > 
> > So, let me re-send updated patch.
> 
> I've looked at both the GNU and FreeBSD sources for mktemp:
> 
>   https://svnweb.freebsd.org/base/release/11.1.0/usr.bin/mktemp/mktemp.c?view=markup
> 
>   https://git.savannah.gnu.org/cgit/coreutils.git/tree/src/mktemp.c?h=v8.29&id=27b2b19aa8d8b30b8cb4198b2f4b54568e10a35e
> 
> and I have also run some tests with both versions.
> 
> It is possible to have portability without using the deprecated -t
> option.
> 
> * When a template is being used then put the directory inline:
> 
>     mktemp /var/tmp/blah-xxxxx
> 
> * When a template is not being used then use TMPDIR:
> 
>     TMPDIR=/var/tmp mktemp
> 
> Both of these appear to work with and without the -d option.
> 
> I really don't think we should be using -t, since it deprecated in the
> GNU version.
> 
> I am wondering if your testing is having problems due to the typo
> here:
> 
> >   tmpeditor=$(mktemp "$STpath/bin/samba-tool-editor-XXXXXXXX")
> >   tmpeditor="${tempeditor}.sh"
> 
> or if the "tempeditor" typo is only in your email.  :-)
> 
> peace & happiness,
> martin
> 

This is all my fault, I wrote the script in question.
I initially used a hardcoded '/tmp/editor.sh', but was advised that
this wasn't a good idea, I changed it to what is there now (without
really thinking it through or considering portability)

With hindsight and thought, it would have been better to use:

tmpeditor=$(mktemp /tmp/samba-tool-editor-XXXXXXXX)

or

tmpeditor=$(mktemp /var/tmp/samba-tool-editor-XXXXXXXX)

Adding the '.sh' suffix is a bit of a red herring, 'scriptname' will
work just as well as 'scriptname.sh'

Rowland



More information about the samba-technical mailing list