[COMMIT] Import make-temp-name (the functionality of mkstemp(3)) from GNU

Aidan Kehoe kehoea at parhasard.net
Thu Nov 15 06:41:55 EST 2007


 Ar an cúigiú lá déag de mí na Samhain, scríobh Stephen J. Turnbull: 

 > QUERY
 > 
 > AFAICS your implementation is basically buggy.  Some suggestions for
 > alternative implementation below.
 > 
 > Aidan Kehoe writes:
 >  > 
 >  > Yes, the CODING-SYSTEM-OR-MUSTBENEW argument is ugly. No, I don't see a
 >  > better approach to this which would be compatible with GNU, given that our
 >  > CODING-SYSTEM argument was already in 21.4. 
 > 
 > Where is this argument used with MUSTBENEW semantics? 

http://google.com/codesearch?hl=de&lr=&q=file%3A%5C.el+%27excl%5B%5Ea-z%5D&btnG=Suche

apel/poe.el
gnus/mm-util.el
gnus/nnmaildir.el
ediff-util.el

And in make-temp-file, the function I just merged.

 > Wouldn't it be better to keep the CODING-SYSTEM name and simply document
 > the additional semantics? I don't think we should encourage use of this
 > misfeature in application code.

There’s no other way, beyond using write-region-internal--not portable to
GNU--to access this functionality--avoiding the race condition between
checking for a file’s existence and creating it. It’s ugly, but it is a
positive feature.

>  > 	* code-files.el (write-region):
 >  > 	Provide a new arg, CODING-SYSTEM-OR-MUSTBENEW, for compatibility
 >  > 	both with GNU (where it has the MUSTBENEW meaning) and earlier
 >  > 	XEmacs code (where it has the CODING-SYSTEM meaning). 
 > 
 > For the sake of review, please be accurate.  This argument is not new,
 > it's just been given additional semantics.

OK. 

 >  > 	* fileio.c (Fwrite_region_internal):
 >  > 	Take a new arg, MUSTBENEW, to error if the file to be written
 >  > 	already exists.
 > 
 > *gag* It's just wrong to do this at that level. Isn't there some way to
 > avoid this? That function is already way overcomplicated.

I’m not sure you’re clear on the point of the MUSTBENEW argument. To make
the check-for-an-existing-file-if-it-doesn’t-exist-create-it operation
atomic¹--which it needs to be to avoid security issues for temporary
files--it needs to be done in the OS kernel. Which means a subr is needed to
expose it to Lisp. Which means it needs to be done at this level.

 > As long as we're here ...
 > 
 >  >  Kludgy feature:
 > 
 > Wouldn't it be less work to mark the *clean* features of this monster?
 > :-(  How about something like [...]

Yeah, that’s a much nicer docstring. 

 > BTW, do you insist on 'excl (eg, for gagmacs compatibility)? 

Yes. 

 > Really I'd like to see t instead of 'excl, and restrict to 'ask for the
 > query confirmation.

I agree that would be a better interface. It’s unhelpfully incompatible,
though.

 >  > Index: src/fileio.c
 >  > ===================================================================
 > 
 >  > +    if (!NILP (mustbenew) && !EQ (mustbenew, Qexcl))
 >  > +      barf_or_query_if_file_exists (filename, "overwrite", 1, NULL);
 > 
 > Isn't it cleaner to do
 > 
 >     if (!NILP (mustbenew))
 >       barf_or_query_if_file_exists (filename, "overwrite",
 >                                     EQ (mustbenew, Qexcl) ? 0 : 1, NULL);

That would be a little friendlier for most people most of the time, yes. I
copied the logic from GNU, though, so I didn’t write it like that.

 >  > @@ -3433,12 +3448,14 @@
 >  >    desc = -1;
 >  >    if (!NILP (append))
 >  >      {
 >  > -      desc = qxe_open (XSTRING_DATA (fn), O_WRONLY | OPEN_BINARY, 0);
 >  > +      desc = qxe_open (XSTRING_DATA (fn), O_WRONLY | OPEN_BINARY
 >  > +                       | (EQ (mustbenew, Qexcl) ? O_EXCL : 0), 0);
 > 
 > Note that on Mac OS X, at least, O_EXCL is documented to have no
 > effect unless O_CREAT is also specified.  Even if this does work on
 > some systems, the user will get a less specific `file-error' rather
 > than `file-already-exists'.

FreeBSD: ‘If O_EXCL is set with O_CREAT and the file already exists, open()
returns an error. This may be used to implement a simple exclusive access
locking mechanism. If O_EXCL is set and the last component of the pathname
is a symbolic link, open() will fail even if the symbolic link points to a
non-existent name.’

I read that to mean that O_EXCL does have function when O_CREAT isn’t set,
but I admittedly haven’t written any code to test that understanding.


¹ In the sense that it cannot be interrupted by other processes and resumed.

-- 
¿Dónde estará ahora mi sobrino Yoghurtu Nghé, que tuvo que huir
precipitadamente de la aldea por culpo de la escasez de rinocerontes?



More information about the XEmacs-Patches mailing list