From 32c7d0858df1e5576a25ef64bb5ab971d1018c85 Mon Sep 17 00:00:00 2001 From: Jens Schmidt Date: Thu, 19 Oct 2023 23:00:32 +0200 Subject: [PATCH] Better handle errors when writing r-o files without backup * lisp/files.el (basic-save-buffer-2): Restore file permissions when writing read-only files without backup fails. (Bug#66546) --- lisp/files.el | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/lisp/files.el b/lisp/files.el index adfe8bd44b9..22c4f845a9b 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -5856,6 +5856,10 @@ basic-save-buffer-1 ;; This returns a value (MODES EXTENDED-ATTRIBUTES BACKUPNAME), like ;; backup-buffer. (defun basic-save-buffer-2 () + ;; Variable `tempsetmodes' tracks temporary changes to the file + ;; modes: We set it to t if we (probably) need to make the file + ;; writable, and later to `reset-on-error' if we actually made the + ;; file writable. (let (tempsetmodes setmodes) (if (not (file-writable-p buffer-file-name)) (let ((dir (file-name-directory buffer-file-name))) @@ -5934,10 +5938,14 @@ basic-save-buffer-2 t)) ;; If file not writable, see if we can make it writable ;; temporarily while we write it (its original modes will be - ;; restored in 'basic-save-buffer'). But no need to do so if - ;; we have just backed it up (setmodes is set) because that - ;; says we're superseding. + ;; restored in 'basic-save-buffer' or, in case of an error, in + ;; the `unwind-protect' below). But no need to do so if we + ;; have just backed it up (setmodes is set) because that says + ;; we're superseding. (cond ((and tempsetmodes (not setmodes)) + ;; Remember we made the file writable so that we can + ;; try to undo that in case of errors. + (setq tempsetmodes 'reset-on-error) ;; Change the mode back, after writing. (setq setmodes (list (file-modes buffer-file-name) @@ -5946,7 +5954,12 @@ basic-save-buffer-2 (file-extended-attributes buffer-file-name)) buffer-file-name)) ;; If set-file-extended-attributes fails to make the - ;; file writable, fall back on set-file-modes. + ;; file writable, fall back on set-file-modes. Calling + ;; set-file-extended-attributes here may or may not be + ;; actually necessary. However, since its exact + ;; behavior is highly port-specific, since calling it + ;; does not do any harm, and since the call has a long + ;; history, we decided to leave it in (bug#66546). (with-demoted-errors "Error setting attributes: %s" (set-file-extended-attributes buffer-file-name (nth 1 setmodes))) @@ -5963,12 +5976,21 @@ basic-save-buffer-2 buffer-file-name nil t buffer-file-truename) (when save-silently (message nil)) (setq success t)) - ;; If we get an error writing the new file, and we made - ;; the backup by renaming, undo the backing-up. - (and setmodes (not success) - (progn - (rename-file (nth 2 setmodes) buffer-file-name t) - (setq buffer-backed-up nil))))))) + (cond + ;; If we get an error writing the file which we + ;; previously made writable, attempt to undo the + ;; write-access. + ((and (eq tempsetmodes 'reset-on-error) (not success)) + (with-demoted-errors "Error setting file modes: %S" + (set-file-modes buffer-file-name (car setmodes))) + (with-demoted-errors "Error setting attributes: %s" + (set-file-extended-attributes buffer-file-name + (nth 1 setmodes)))) + ;; If we get an error writing the new file, and we made + ;; the backup by renaming, undo the backing-up. + ((and setmodes (not success)) + (rename-file (nth 2 setmodes) buffer-file-name t) + (setq buffer-backed-up nil))))))) setmodes)) (declare-function diff-no-select "diff" -- 2.30.2