From 1f9f514e7c2ba41b0954d0141f99652f6a53a107 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 2 Aug 2017 01:53:46 -0700 Subject: [PATCH] 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 96c5639a096..0264c9fa1d8 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 bfa1d63d833..dd8cb28c425 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 cffaf954b3b..4de6fc85ec1 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 db99f53299c..22446b25d16 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -37,6 +37,11 @@ along with GNU Emacs. If not, see . */ #include "sysselect.h" #include "blockinput.h" +#ifdef HAVE_LINUX_FS_H +# include +# include +#endif + #if defined DARWIN_OS || defined __FreeBSD__ # include #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. */ -- 2.39.2