Re: backup-buffer-copy loops if old backup can't be deleted

From: martin rudalics <rudalics@gmx.at>
To: Martin von Gagern <Martin.vGagern@gmx.net>
Cc: bug-gnu-emacs@gnu.org, rms@gnu.org
Subject: Re: backup-buffer-copy loops if old backup can't be deleted
Date: Fri, 24 Aug 2007 11:10:35 +0200	[thread overview]
Message-ID: <46CEA08B.6080907@gmx.at> (raw)
In-Reply-To: <46CE8501.9040307@gmx.net>

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]

>>I think there is no need to explicitly check whether the directory
>>is writable.  What would be the purpose of that?
> 
> 
> I didn't write that patch, but I guess there could be a purpose for it.
> Namely it could help deleting a file in vain. If there are Filesystems
> that let you delete a writable file even though you can't create new
> ones in a non-writable dir, then without those lines, emacs would delete
> an old backup only to find out that it can't create a new one.
> 
> Most likely these would be cases where the old backup file was writable,
> so if you had some logic to write to existing files and only delete them
> if they are not writable, then the check whether the directory is
> writable should really be superfluous. Otherwise I'm not so sure.

Wouldn't it be simpler to try something like the attached patch.

[-- Attachment #2: files.patch --]
[-- Type: text/plain, Size: 2219 bytes --]

*** files.el	Fri Aug 24 07:28:20 2007
--- files.el	Fri Aug 24 11:04:42 2007
***************
*** 3172,3198 ****
  	    (file-error nil))))))

  (defun backup-buffer-copy (from-name to-name modes)
!   (let ((umask (default-file-modes))
! 	(dir (or (file-name-directory to-name)
! 		 default-directory)))
      (unwind-protect
  	(progn
  	  ;; Create temp files with strict access rights.  It's easy to
  	  ;; loosen them later, whereas it's impossible to close the
  	  ;; time-window of loose permissions otherwise.
  	  (set-default-file-modes ?\700)
! 	  (while (condition-case ()
! 		     (progn
! 		       (and (file-exists-p to-name)
! 			    (delete-file to-name))
! 		       (copy-file from-name to-name nil t)
! 		       nil)
! 		   (file-already-exists t))
! 	    ;; The file was somehow created by someone else between
! 	    ;; `delete-file' and `copy-file', so let's try again.
! 	    ;; rms says "I think there is also a possible race
! 	    ;; condition for making backup files" (emacs-devel 20070821).
! 	    nil))
        ;; Reset the umask.
        (set-default-file-modes umask)))
    (and modes
--- 3172,3200 ----
  	    (file-error nil))))))

  (defun backup-buffer-copy (from-name to-name modes)
!   (let ((umask (default-file-modes)))
      (unwind-protect
  	(progn
  	  ;; Create temp files with strict access rights.  It's easy to
  	  ;; loosen them later, whereas it's impossible to close the
  	  ;; time-window of loose permissions otherwise.
  	  (set-default-file-modes ?\700)
! 	  (when (condition-case nil
! 		    ;; Try to overwrite old backup first.
! 		    (copy-file from-name to-name t t)
! 		  (file-error t))
! 	    (while (condition-case nil
! 		       (progn
! 			 (when (file-exists-p to-name)
! 			   (delete-file to-name))
! 			 (copy-file from-name to-name nil t)
! 			 nil)
! 		     (file-already-exists t))
! 	      ;; The file was somehow created by someone else between
! 	      ;; `delete-file' and `copy-file', so let's try again.
! 	      ;; rms says "I think there is also a possible race
! 	      ;; condition for making backup files" (emacs-devel 20070821).
! 	      nil)))
        ;; Reset the umask.
        (set-default-file-modes umask)))
    (and modes

[-- Attachment #3: Type: text/plain, Size: 149 bytes --]

_______________________________________________
bug-gnu-emacs mailing list
bug-gnu-emacs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnu-emacs