[Emacs-diffs] master 1f9f514 2/2: When renaming a file, ask only if EEXIST or ENOSYS

[Paul Eggert]

branch: master
commit 1f9f514e7c2ba41b0954d0141f99652f6a53a107
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>

    When renaming a file, ask only if EEXIST or ENOSYS
    
    * src/fileio.c (Frename_file): Avoid calling Ffile_directory_p
    more than once on FILE.  Use renameat_noreplace, so that we can
    ask the user (and unlink and retry) only if this fails with errno
    == EEXIST or ENOSYS.  This avoids the need to ask the user for
    permission to do an operation that will fail anyway.  Simplify
    computation of ok_if_already_exists for subsidiary functions.
    * src/filelock.c (rename_lock_file): Prefer renameat_noreplace
    if it works, as this avoids the need to link and unlink.
    * src/lisp.h (renameat_noreplace): New decl.
    * src/sysdep.c [HAVE_LINUX_FS_H]: Include linux/fs.h and sys/syscall.h.
    (renameat_noreplace): New function.
---
 src/fileio.c   | 83 ++++++++++++++++++++++++++++++++--------------------------
 src/filelock.c |  3 +++
 src/lisp.h     |  6 +++--
 src/sysdep.c   | 20 ++++++++++++++
 4 files changed, 73 insertions(+), 39 deletions(-)

diff --git a/src/fileio.c b/src/fileio.c
index 96c5639..0264c9f 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -2311,6 +2311,7 @@ This is what happens in interactive use with M-x.  */)
 {
   Lisp_Object handler;
   Lisp_Object encoded_file, encoded_newname, symlink_target;
+  int dirp = -1;
 
   symlink_target = encoded_file = encoded_newname = Qnil;
   CHECK_STRING (file);
@@ -2324,8 +2325,8 @@ This is what happens in interactive use with M-x.  */)
       && (NILP (Ffile_name_case_insensitive_p (file))
          || NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname)))))
     {
-      Lisp_Object fname = (NILP (Ffile_directory_p (file))
-                          ? file : Fdirectory_file_name (file));
+      dirp = !NILP (Ffile_directory_p (file));
+      Lisp_Object fname = dirp ? Fdirectory_file_name (file) : file;
       newname = Fexpand_file_name (Ffile_name_nondirectory (fname), newname);
     }
   else
@@ -2343,47 +2344,55 @@ This is what happens in interactive use with M-x.  */)
   encoded_file = ENCODE_FILE (file);
   encoded_newname = ENCODE_FILE (newname);
 
-  /* If the filesystem is case-insensitive and the file names are
-     identical but for the case, don't ask for confirmation: they
-     simply want to change the letter-case of the file name.  */
-  if ((!(file_name_case_insensitive_p (SSDATA (encoded_file)))
-       || NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname))))
-      && ((NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists))))
-    barf_or_query_if_file_exists (newname, false, "rename to it",
-                                 INTEGERP (ok_if_already_exists), false);
-  if (rename (SSDATA (encoded_file), SSDATA (encoded_newname)) < 0)
+  if (renameat_noreplace (AT_FDCWD, SSDATA (encoded_file),
+                         AT_FDCWD, SSDATA (encoded_newname))
+      == 0)
+    return Qnil;
+  int rename_errno = errno;
+
+  if (rename_errno == EEXIST || rename_errno == ENOSYS)
     {
-      int rename_errno = errno;
-      if (rename_errno == EXDEV)
-       {
-          ptrdiff_t count;
-          symlink_target = Ffile_symlink_p (file);
-          if (! NILP (symlink_target))
-            Fmake_symbolic_link (symlink_target, newname,
-                                 NILP (ok_if_already_exists) ? Qnil : Qt);
-         else if (!NILP (Ffile_directory_p (file)))
-           call4 (Qcopy_directory, file, newname, Qt, Qnil);
-         else
-           /* We have already prompted if it was an integer, so don't
-              have copy-file prompt again.  */
-           Fcopy_file (file, newname,
-                       NILP (ok_if_already_exists) ? Qnil : Qt,
-                       Qt, Qt, Qt);
+      /* If the filesystem is case-insensitive and the file names are
+        identical but for the case, don't ask for confirmation: they
+        simply want to change the letter-case of the file name.  */
+      if ((NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists))
+         && (! file_name_case_insensitive_p (SSDATA (encoded_file))
+             || NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname)))))
+       barf_or_query_if_file_exists (newname, rename_errno == EEXIST,
+                                     "rename to it",
+                                     INTEGERP (ok_if_already_exists), false);
+      if (rename (SSDATA (encoded_file), SSDATA (encoded_newname)) == 0)
+       return Qnil;
+      rename_errno = errno;
+      /* Don't prompt again.  */
+      ok_if_already_exists = Qt;
+    }
+  else if (!NILP (ok_if_already_exists))
+    ok_if_already_exists = Qt;
 
-         count = SPECPDL_INDEX ();
-         specbind (Qdelete_by_moving_to_trash, Qnil);
+  if (rename_errno != EXDEV)
+    report_file_errno ("Renaming", list2 (file, newname), rename_errno);
 
-         if (!NILP (Ffile_directory_p (file)) && NILP (symlink_target))
-           call2 (Qdelete_directory, file, Qt);
-         else
-           Fdelete_file (file, Qnil);
-         unbind_to (count, Qnil);
-       }
+  symlink_target = Ffile_symlink_p (file);
+  if (!NILP (symlink_target))
+    Fmake_symbolic_link (symlink_target, newname, ok_if_already_exists);
+  else
+    {
+      if (dirp < 0)
+       dirp = !NILP (Ffile_directory_p (file));
+      if (dirp)
+       call4 (Qcopy_directory, file, newname, Qt, Qnil);
       else
-       report_file_errno ("Renaming", list2 (file, newname), rename_errno);
+       Fcopy_file (file, newname, ok_if_already_exists, Qt, Qt, Qt);
     }
 
-  return Qnil;
+  ptrdiff_t count = SPECPDL_INDEX ();
+  specbind (Qdelete_by_moving_to_trash, Qnil);
+  if (dirp && NILP (symlink_target))
+    call2 (Qdelete_directory, file, Qt);
+  else
+    Fdelete_file (file, Qnil);
+  return unbind_to (count, Qnil);
 }
 
 DEFUN ("add-name-to-file", Fadd_name_to_file, Sadd_name_to_file, 2, 3,
diff --git a/src/filelock.c b/src/filelock.c
index bfa1d63..dd8cb28 100644
--- a/src/filelock.c
+++ b/src/filelock.c
@@ -339,6 +339,9 @@ rename_lock_file (char const *old, char const *new, bool 
force)
     {
       struct stat st;
 
+      int r = renameat_noreplace (AT_FDCWD, old, AT_FDCWD, new);
+      if (! (r < 0 && errno == ENOSYS))
+       return r;
       if (link (old, new) == 0)
        return unlink (old) == 0 || errno == ENOENT ? 0 : -1;
       if (errno != ENOSYS && errno != LINKS_MIGHT_NOT_WORK)
diff --git a/src/lisp.h b/src/lisp.h
index cffaf95..4de6fc8 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4298,13 +4298,15 @@ extern ptrdiff_t emacs_write (int, void const *, 
ptrdiff_t);
 extern ptrdiff_t emacs_write_sig (int, void const *, ptrdiff_t);
 extern ptrdiff_t emacs_write_quit (int, void const *, ptrdiff_t);
 extern void emacs_perror (char const *);
+extern int renameat_noreplace (int, char const *, int, char const *);
+extern int str_collate (Lisp_Object, Lisp_Object, Lisp_Object, Lisp_Object);
 
-extern void unlock_all_files (void);
+/* Defined in filelock.c.  */
 extern void lock_file (Lisp_Object);
 extern void unlock_file (Lisp_Object);
+extern void unlock_all_files (void);
 extern void unlock_buffer (struct buffer *);
 extern void syms_of_filelock (void);
-extern int str_collate (Lisp_Object, Lisp_Object, Lisp_Object, Lisp_Object);
 
 /* Defined in sound.c.  */
 extern void syms_of_sound (void);
diff --git a/src/sysdep.c b/src/sysdep.c
index db99f53..22446b2 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -37,6 +37,11 @@ along with GNU Emacs.  If not, see 
<http://www.gnu.org/licenses/>.  */
 #include "sysselect.h"
 #include "blockinput.h"
 
+#ifdef HAVE_LINUX_FS_H
+# include <linux/fs.h>
+# include <sys/syscall.h>
+#endif
+
 #if defined DARWIN_OS || defined __FreeBSD__
 # include <sys/sysctl.h>
 #endif
@@ -2678,6 +2683,21 @@ set_file_times (int fd, const char *filename,
   timespec[1] = mtime;
   return fdutimens (fd, filename, timespec);
 }
+
+/* Rename directory SRCFD's entry SRC to directory DSTFD's entry DST.
+   This is like renameat except that it fails if DST already exists,
+   or if this operation is not supported atomically.  Return 0 if
+   successful, -1 (setting errno) otherwise.  */
+int
+renameat_noreplace (int srcfd, char const *src, int dstfd, char const *dst)
+{
+#ifdef SYS_renameat2
+  return syscall (SYS_renameat2, srcfd, src, dstfd, dst, RENAME_NOREPLACE);
+#else
+  errno = ENOSYS;
+  return -1;
+#endif
+}
 
 /* Like strsignal, except async-signal-safe, and this function typically
    returns a string in the C locale rather than the current locale.  */