bug#66546: 30.0.50; save-buffer to write-protected file without backup fails

[Jens Schmidt]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
>> Cc: 66546@debbugs.gnu.org
>> Date: Sat, 21 Oct 2023 22:17:18 +0200

>> Makes sense, will do.  In that case, would a plain `equal' be enough to
>> compare file paths?
>
> Yes, I think so.

Please review the next version, then.

Thanks.

>From 249a53e340c5c6cd4c9b8c405e5a9c6074876585 Mon Sep 17 00:00:00 2001
From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
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 | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index adfe8bd44b9..1be5b374ae8 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5934,9 +5934,10 @@ 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))
               ;; Change the mode back, after writing.
               (setq setmodes
@@ -5946,7 +5947,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 +5969,22 @@ 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, and there is no
+             ;; backup file, then we (most likely) made that file
+             ;; writable above.  Attempt to undo the write-access.
+             ((and setmodes (not success)
+                   (equal (nth 2 setmodes) buffer-file-name))
+             (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