FS#9357 - Cut/Paste does not remove old folder when replacing

Attached to Project: Rockbox
Opened by Shiloh Hawley (gree665) - Tuesday, 02 September 2008, 21:40 GMT
Last edited by Nils Wallménius (nls) - Monday, 01 December 2008, 11:32 GMT
Task Type Bugs
Category User Interface
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


I move a folder named 'foo' from my 'newmusic' folder to my 'music' folder using cut and paste in Rockbox. There is already a folder called 'foo' in 'music', so RB asks me if I want to overwrite it and I say yes. Then all of the files in 'foo' are moved as expected, but the empty 'foo' folder still remains in 'newmusic'.

I hope that makes sense, if not, or if you have any problems reproducing, let me know.
This task depends upon

Closed by  Nils Wallménius (nls)
Monday, 01 December 2008, 11:32 GMT
Reason for closing:  Fixed
Additional comments about closing:  Turns out safetydan's fix was almost correct and worked with a small change.
Committed as r19285, thanks!
Comment by Nils Wallménius (nls) - Friday, 28 November 2008, 16:45 GMT
Reproduced in a simulator, in fact no dirs that already exist in the destination will be
removed from the source, for example i tested with these two test dirs:



if i cut and paste test1 into test2's parent i get asked if i want to overwrite and if
i answer "yes" i end up with a test1 dir that looks as follows:


edit: meh, the whitespace is apparently mangled in comments so dr2 is a subdir of dir1.
Comment by Dan Everton (safetydan) - Saturday, 29 November 2008, 03:02 GMT
I'm guessing this is because the directory moving code just calls rename on the old directory and that fails. Though if that was the case then I'd expect that error to propagate up and show "Paste failed" or something. Either way, the paste code will probably need to explicitly remove the directory after it's done recursing through it or something like that.
Comment by Dan Everton (safetydan) - Saturday, 29 November 2008, 03:25 GMT
Hrm, actually, on closer inspection I think something like this might be needed:

At line 856 of apps/onplay.c add

if (!copy) {
remove_dir(src, srclen);

and see if that fixes it. I'd test myself but I don't have a build environment at the moment.
Comment by Nils Wallménius (nls) - Saturday, 29 November 2008, 09:01 GMT
That fixes half the problem, the subdir is now removed but the dir that i select for copying, dir1 in the example above, is still not.
Comment by Dan Everton (safetydan) - Saturday, 29 November 2008, 09:14 GMT
Then try adding this after line 916:

if (success && !clipboard_is_copy) {
remove_dir(clipboard_selection, strlen(clipboard_selection));

that should remove the clipboard selection on completion only if it worked.
Comment by Nils Wallménius (nls) - Sunday, 30 November 2008, 15:06 GMT
Yes, that seems to be the right thing, but even with the moderate dir depth
I use for testing i bump in to the max open dirs limit :(
So, let's mark this as depending on FS#9260.