GNU bug report logs - #964
trash - runaway recursion for cross-device delete-file ops

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 964 in the body.
You can then email your comments to 964 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: David De La Harpe Golden <david <at> harpegolden.net>
To: submit <at> debbugs.gnu.org
Subject: trash - runaway recursion for cross-device delete-file ops
Date: Fri, 12 Sep 2008 03:07:31 +0100

[Message part 1 (text/plain, inline)]

Package: emacs
Version: 23.0.60
Severity: normal
Tags: patch

Steps to reproduce:

Turn on delete-by-moving-to-trash on gnu+linux.
Make a file on a different filesystem to your home dir.
Try to delete-file the file in emacs once.
Look inside ~/.Trash, full of hundreds of redundant
copies of the file.

Reason:

When delete-by-moving-to-trash is on, delete-file calls move-file-to-trash.

move-file-to-trash, when using its fallback emacs trashcan
implementation* (i.e. when there's no system-move-file-to-trash) picks a
name for the trashed file, and calls rename-file.

rename-file tries a C rename(), but that doesn't work cross-device on
gnu+linux (and several other OSes), so rename-file falls back to copying
the file to the new name, and then calls delete-file to get rid of the
old one (~line 2247 of fileio.c)

move-file-to-trash decides the existing filename under .Trash is taken,
seeing as a file exists with that name, picks a new name, and calls
rename-file again...

... Lather, rinse, repeat, until emacs gets bored with a
"Variable binding depth exceeds max-specpdl-size"

Fix?:

(i) Well, binding delete-by-moving-to-trash to nil around the
rename-file in move-file-to-trash might be adequate, fixes immediate
issue?  trivial patch attached.  Though another strikes me:

(ii) As it stands, won't an actual rename-file will sometimes move a
copy of "renamed" files to trash (i.e. when they're on a different
device) if delete-by-moving-to-trash is on?  Not sure that's very nice -
maybe should bind delete-by-moving-to-trash to nil around its call to
delete-file, or maybe just use C unlink()?


* which really isn't suitable for gnu+linux/freedesktop.org desktops,
it seems to be something very like (if not identical to) the macosx
convention, but that's a matter for a different bug.

[trash_xdevice_fix_r1.diff (text/x-patch, inline)]

Index: lisp/files.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/files.el,v
retrieving revision 1.995
diff -U 8 -r1.995 files.el
--- lisp/files.el	2 Sep 2008 16:10:44 -0000	1.995
+++ lisp/files.el	12 Sep 2008 01:43:51 -0000
@@ -5820,17 +5820,18 @@
            ;; make new-fn unique.
            ;; example: "~/.Trash/abc.txt" -> "~/.Trash/abc.txt.~1~"
            (let ((version-control t))
              (setq new-fn (car (find-backup-file-name new-fn)))))
       ;; stop processing if fn is same or parent directory of trash-dir.
       (and (string-match fn trash-dir)
            (error "Filename `%s' is same or parent directory of trash-directory"
                   filename))
-      (rename-file fn new-fn)))))
+      (let ((delete-by-moving-to-trash nil))
+	(rename-file fn new-fn))))))
 
 
 (define-key ctl-x-map "\C-f" 'find-file)
 (define-key ctl-x-map "\C-r" 'find-file-read-only)
 (define-key ctl-x-map "\C-v" 'find-alternate-file)
 (define-key ctl-x-map "\C-s" 'save-buffer)
 (define-key ctl-x-map "s" 'save-some-buffers)
 (define-key ctl-x-map "\C-w" 'write-file)

[ChangeLog.trash_xdevice_fix_r1 (text/plain, inline)]

2008-09-11  David De La Harpe Golden <david <at> harpegolden.net>

	files.el: in move-file-to-trash, bind delete-by-moving-to-trash to
	nil around rename-file to avoid recursive trashing if rename-file
	calls delete-file.

Message #10 received at 964-done <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: David De La Harpe Golden <david <at> harpegolden.net>
Cc: 964-done <at> debbugs.gnu.org
Subject: Re: bug#964: trash - runaway recursion for cross-device delete-file ops
Date: Sat, 20 Sep 2008 17:40:39 -0400

David De La Harpe Golden wrote:

> (i) Well, binding delete-by-moving-to-trash to nil around the
> rename-file in move-file-to-trash might be adequate, fixes immediate
> issue?  trivial patch attached.

Thanks; applied.

> (ii) As it stands, won't an actual rename-file will sometimes move a
> copy of "renamed" files to trash (i.e. when they're on a different
> device) if delete-by-moving-to-trash is on?  Not sure that's very nice -
> maybe should bind delete-by-moving-to-trash to nil around its call to
> delete-file, or maybe just use C unlink()?

I installed a fix using the former approach.

This feature doesn't seem very well thought-out on non-Windows platforms.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.