[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH v2 1/2] mktemp: avoid removed mktemp()
From:       Rich Felker <dalias () aerifal ! cx>
Date:       2014-04-15 1:09:01
Message-ID: 20140415010901.GW26358 () brightrain ! aerifal ! cx
[Download RAW message or body]

On Mon, Apr 14, 2014 at 02:29:21PM +0200, Denys Vlasenko wrote:
> On Mon, Apr 14, 2014 at 12:20 PM, Bernhard Reutner-Fischer
> <rep.dot.nop@gmail.com> wrote:
> >> This is a change in behavior - now we would
> >> actually create, and then immediately delete the directory
> >> if run as "mktemp -du".
> >>
> >> I have mixed feelings about that.
> >> Do you think it's ok?
> >
> > I think it's ok, yes. The race-window is about the same as
> > before, the creat()/unlink() do make it a bit slower but that
> > is IMO ok.
> 
> There was no race at all before for "mktemp -n" case:
> 
>         if (opts & OPT_u) {
>                 chp = mktemp(chp);
>                 if (chp[0] == '\0')
>                         goto error;
>         } else ...
>         puts(chp);
>         return EXIT_SUCCESS;
> 
> No race here.

The 'race' is in the caller, but it's not always a race. If the caller
properly uses O_EXCL or similar and retries on fail, there's no race.
Note that there are legitimate uses of mktemp -u, such as along with
mkfifo or for choosing a name for a socket or shared memory object or
similar.

> > An alternative would be to export a uClibc-specific
> > __gen_tempname() function
> 
> How about exporting a function named mktemp()?
>  ;)
> POSIX removing it from its text doesn't mean that
> a conforming libc must *not* provide it.

Agreed. I don't see any value in removing these functions. If support
for libcs that lack them is desired, a proper implementation should be
provided in busybox rather than poor hacks that involve creating and
deleting the temp file, and which could fail due to odd permission
setups, etc.

Rich
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]