Ticket #20 (new defect)

Opened 16 years ago

Last modified 11 years ago

savannah: superflous copying instead of moving

Reported by: ossi Owned by:
Priority: minor Milestone: Future Releases
Component: mc-core Version: master
Keywords: Cc: ip1024@…
Blocked By: Blocking:
Branch state: no branch Votes for changeset:

Description (last modified by ossi) (diff)

Original: http://savannah.gnu.org/bugs/?13728

Submitted by:Oswald Buddenhagen <ossi>Submitted on:Mon 11 Jul 2005 03:44:56 PM UTC
Category:CoreSeverity:3 - Normal
Assigned to:NoneOpen/Closed:Open
Release:current (CVS or snapshot)Operating System:All

Original submission:

foo/name/file exists.
bar/name/ exists.
both on the same partition.
you're in "foo/", cursor over "name".
press <f6>, enter "../bar", confirm.
effect: file is moved by copying. this is because mc incorrectly 
diagnoses a cross-device move.
the code in question is file.c, move_file_file() & move_dir_dir().
when fixing, please consider the case of moving a tree where only a 
subtree is on another device. but maybe it was already considered. :) 

Comment 1 by Pavel Tsekov <ptsekov> at Mon 11 Jul 2005 05:01:24 PM UTC:

Unfortunately there is nothing to fix here :( If the destination 
directory exits, MC decides to use copy_dir_dir() to perform the 
task. What you ask should be fully implemented from scratch. In 
fact move_dir_dir() is quite simplistic.

Comment 2 by Oswald Buddenhagen <ossi> at Mon 11 Jul 2005 05:09:14 PM UTC:

yes, indeed. i even started developing an algorithm to post here, 
but stopped because the existance checks and file/dir 
discrimination started turning the few lines into real work. :)=
anyway, the basic idea is: when a move fails, if the source is a 
file, move via copy, otherwise create the target dir (if missing) 
and recursively call the move function for all files in the source 
dir. or something like that. :)

Comment 3 by Pavel Tsekov <ptsekov> at Tue 12 Jul 2005 11:53:46 AM UTC:

Does `bar/name/' contain any entries or is it empty ? I ask this 
question because of the following paragraph in the description of 
rename() in SUSv3:

If the old argument points to the pathname of a directory, the new 
argument shall not point to the pathname of a file that is not a 
directory. If the directory named by the new argument exists, it 
shall be removed and old renamed to new. In this case, a link named 
new shall exist throughout the renaming operation and shall refer 
either to the directory referred to by new or old before the 
operation began. If new names an existing directory, it shall be 
required to be an empty directory.

So if we have an empty destination directory we could perform an 
efficient move instead of copying . It is as simple as that.

Comment 4 by Oswald Buddenhagen <ossi> at Tue 12 Jul 2005 01:09:19 PM UTC:

> Does `bar/name/' contain any entries or is it empty ?

empty, but this should not matter. i don't think we want this susv3-
compliant behavior. at the top level, if the target is an existing 
directory (empty or not), we want to move into this directory (like 
we currently do). once we started recursing, we just merge into 
existing directories.
frankly, move should behave just like copy. i think it would even 
make sense to merge move into copy with an additional flag. if a 
real move succeeds, skip the recursion. if it fails, do a recursive 
copy+delete. because of the recursion, a real move is attempted at 
every level - exactly what we want.

Comment 5 by Pavel Tsekov <ptsekov>:

A really fast move can happen only under the circumstances 
described in the quoted text - everything else will be suboptimal. 
I don't want to block any attempts to move the directory if the 
existing directory is not empty, but it cannot be performed as 
fast. The more dupicate directories exist the performance will get 
worse (in large trees).

Change History

comment:1 Changed 16 years ago by slavazanko

  • Priority changed from major to minor
  • Component changed from mc-core to vfs

comment:2 Changed 15 years ago by styx

  • Milestone set to 4.7

comment:3 Changed 15 years ago by igorp1024

  • Cc ip1024@… added
  • severity set to no branch

comment:4 Changed 15 years ago by andrew_b

  • Version set to master
  • Component changed from mc-vfs to mc-core

comment:5 Changed 14 years ago by ossi

  • Cc ossi@… added

comment:6 Changed 13 years ago by andrew_b

  • Branch state set to no branch

Related to #3210.

comment:7 Changed 13 years ago by andrew_b

  • Milestone changed from 4.7 to Future Releases

comment:8 Changed 11 years ago by ossi

  • Cc ossi@… removed
  • Description modified (diff)
  • Reporter changed from slavazanko to ossi
Note: See TracTickets for help on using tickets.