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...
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?
_______________________________________________
XEmacs-Patches mailing list
XEmacs-Patches(a)xemacs.org
http://calypso.tux.org/cgi-bin/mailman/listinfo/xemacs-patches