From: Paul Eggert <eggert@cs.ucla.edu>
To: Lars Ingebrigtsen <larsi@gnus.org>
Cc: 34069@debbugs.gnu.org, "Stefan Kangas" <stefan@marxist.se>,
"積丹尼 Dan Jacobson" <jidanni@jidanni.org>
Subject: bug#34069: rename-file on fifos hangs if cross device
Date: Sun, 18 Dec 2022 13:06:58 -0800 [thread overview]
Message-ID: <309f1805-7f3e-9b84-5a8b-5735b812a661@cs.ucla.edu> (raw)
In-Reply-To: <875zur2xyc.fsf@jidanni.org>
[-- Attachment #1: Type: text/plain, Size: 217 bytes --]
In reviewing that patch I discovered that there's a similar bug in
copy-file, and that there's some opportunity to simplify and robustify
the rename-file code. I installed the attached patches on master to do that.
[-- Attachment #2: 0001-Don-t-hang-when-copying-FIFOs.patch --]
[-- Type: text/x-patch, Size: 1111 bytes --]
From ab3cfa4a17663cf479f286149a2289974dd67240 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 18 Dec 2022 11:45:06 -0800
Subject: [PATCH 1/2] =?UTF-8?q?Don=E2=80=99t=20hang=20when=20copying=20FIF?=
=?UTF-8?q?Os?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* src/fileio.c (Fcopy_file): Open the input file with O_NONBLOCK.
This prevents a hang if the input file is a FIFO.
If it’s a regular file O_NONBLOCK has no effect;
otherwise the file is soon rejected anyway.
---
src/fileio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/fileio.c b/src/fileio.c
index 92335b639c..a50f8d67c1 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -2219,7 +2219,7 @@ DEFUN ("copy-file", Fcopy_file, Scopy_file, 2, 6,
report_file_error ("Copying permissions to", newname);
}
#else /* not WINDOWSNT */
- ifd = emacs_open (SSDATA (encoded_file), O_RDONLY, 0);
+ ifd = emacs_open (SSDATA (encoded_file), O_RDONLY | O_NONBLOCK, 0);
if (ifd < 0)
report_file_error ("Opening input file", file);
--
2.38.1
[-- Attachment #3: 0002-Improve-rename-file-fix-bug-34069.patch --]
[-- Type: text/x-patch, Size: 2698 bytes --]
From 79e1bff2694444a27036b08e8fa2a6619b40dc2a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 18 Dec 2022 12:57:57 -0800
Subject: [PATCH 2/2] Improve rename-file fix (bug#34069)
* src/fileio.c (Frename_file): No need for a special case to
rename a fifo, since we already tried and failed to rename it.
Also improve symlink handling, in that if readlink fails report an
error rather than trying to treat the link as a regular file.
---
src/fileio.c | 35 ++++++++++++-----------------------
1 file changed, 12 insertions(+), 23 deletions(-)
diff --git a/src/fileio.c b/src/fileio.c
index a50f8d67c1..789f3d509e 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -134,6 +134,7 @@ #define DRIVE_LETTER(x) c_tolower (x)
is added here. */
static Lisp_Object Vwrite_region_annotation_buffers;
+static Lisp_Object emacs_readlinkat (int, char const *);
static Lisp_Object file_name_directory (Lisp_Object);
static bool a_write (int, Lisp_Object, ptrdiff_t, ptrdiff_t,
Lisp_Object *, struct coding_system *);
@@ -2710,31 +2711,19 @@ DEFUN ("rename-file", Frename_file, Srename_file, 2, 3,
}
if (dirp)
call4 (Qcopy_directory, file, newname, Qt, Qnil);
- else
- {
- Lisp_Object symlink_target
- = (S_ISLNK (file_st.st_mode)
- ? check_emacs_readlinkat (AT_FDCWD, file, SSDATA (encoded_file))
- : Qnil);
- if (!NILP (symlink_target))
- Fmake_symbolic_link (symlink_target, newname, ok_if_already_exists);
- else if (S_ISFIFO (file_st.st_mode))
- {
- /* If it's a FIFO, calling `copy-file' will hang if it's a
- inter-file system move, so do it here. (It will signal
- an error in that case, but it won't hang in any case.) */
- if (!NILP (ok_if_already_exists))
- barf_or_query_if_file_exists (newname, false,
- "rename to it",
- FIXNUMP (ok_if_already_exists),
- false);
- if (rename (SSDATA (encoded_file), SSDATA (encoded_newname)) != 0)
- report_file_errno ("Renaming", list2 (file, newname), errno);
- return Qnil;
- }
+ else if (S_ISREG (file_st.st_mode))
+ Fcopy_file (file, newname, ok_if_already_exists, Qt, Qt, Qt);
+ else if (S_ISLNK (file_st.st_mode))
+ {
+ Lisp_Object target = emacs_readlinkat (AT_FDCWD,
+ SSDATA (encoded_file));
+ if (!NILP (target))
+ Fmake_symbolic_link (target, newname, ok_if_already_exists);
else
- Fcopy_file (file, newname, ok_if_already_exists, Qt, Qt, Qt);
+ report_file_error ("Renaming", list2 (file, newname));
}
+ else
+ report_file_errno ("Renaming", list2 (file, newname), rename_errno);
specpdl_ref count = SPECPDL_INDEX ();
specbind (Qdelete_by_moving_to_trash, Qnil);
--
2.38.1
prev parent reply other threads:[~2022-12-18 21:06 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-14 13:03 bug#34069: dired-do-rename on fifos hangs if cross device 積丹尼 Dan Jacobson
2020-08-31 10:26 ` Stefan Kangas
2022-05-08 13:41 ` bug#34069: rename-file " Lars Ingebrigtsen
2022-12-18 21:06 ` Paul Eggert [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=309f1805-7f3e-9b84-5a8b-5735b812a661@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=34069@debbugs.gnu.org \
--cc=jidanni@jidanni.org \
--cc=larsi@gnus.org \
--cc=stefan@marxist.se \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).