GNU bug report logs - #14763
mv directory cross-filesystem where destination exists fails to remove dest with EISDIR

Previous Next

Package: coreutils;

Reported by: ken <at> booths.org.uk

Date: Mon, 1 Jul 2013 21:25:02 UTC

Severity: normal

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: ken <at> booths.org.uk
Subject: bug#14763: closed (Re: bug#14763: mv directory cross-filesystem
 where destination exists fails to remove dest with EISDIR)
Date: Wed, 24 Jul 2013 17:16:02 +0000
[Message part 1 (text/plain, inline)]
Your bug report

#14763: mv directory cross-filesystem where destination exists fails to remove dest with EISDIR

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 14763 <at> debbugs.gnu.org.

-- 
14763: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=14763
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Pádraig Brady <P <at> draigBrady.com>
To: ken <at> booths.org.uk
Cc: 14763-done <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>,
 Ken Booth <kbooth <at> redhat.com>
Subject: Re: bug#14763: mv directory cross-filesystem where destination exists
 fails to remove dest with EISDIR
Date: Wed, 24 Jul 2013 18:14:29 +0100
On 07/02/2013 01:29 AM, Ken Booth wrote:
> Hi Eric,
> 
> Thanks for the reference to the POSIX standards.
> 
> The reason I wrote the patch the way I did was to emulate Solaris behaviour for a non-empty directory, however I read at
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html
> the sentence "If /new/ names an existing directory, it shall be required to be an empty directory. "
> 
> Therefore I conclude that Solaris is not POSIX compliant.
> 
> After reading the instructions you suggested I hope the following patch is in the correct format, conforms to the requirements, and is also POSIX compliant ...
> 
> From 8b6549f321c06ee81262f58c6d7bd7e9c9092b30 Mon Sep 17 00:00:00 2001
> From: Ken Booth <ken <at> booths.org.uk>
> Date: Tue, 2 Jul 2013 01:06:32 +0100
> Subject: [PATCH] mv: if inter-device move target is directory use rmdir, not
>  unlink
> 
> ---
>  src/copy.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/src/copy.c b/src/copy.c
> index c1c8273..5137f27 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -2173,13 +2173,27 @@ copy_internal (char const *src_name, char const *dst_name,
>        /* The rename attempt has failed.  Remove any existing destination
>           file so that a cross-device 'mv' acts as if it were really using
>           the rename syscall.  */
> -      if (unlink (dst_name) != 0 && errno != ENOENT)
> +      if (S_ISDIR (src_mode))
>          {
> -          error (0, errno,
> -             _("inter-device move failed: %s to %s; unable to remove target"),
> -                 quote_n (0, src_name), quote_n (1, dst_name));
> -          forget_created (src_sb.st_ino, src_sb.st_dev);
> -          return false;
> +          if (rmdir (dst_name) != 0 && errno != ENOENT)
> +            {
> +              error (0, errno,
> +                 _("inter-device move failed: %s to %s; unable to remove target directory"),
> +                     quote_n (0, src_name), quote_n (1, dst_name));
> +              forget_created (src_sb.st_ino, src_sb.st_dev);
> +              return false;
> +            }
> +        }
> +      else
> +        {
> +          if (unlink (dst_name) != 0 && errno != ENOENT)
> +            {
> +              error (0, errno,
> +                 _("inter-device move failed: %s to %s; unable to remove target"),
> +                     quote_n (0, src_name), quote_n (1, dst_name));
> +              forget_created (src_sb.st_ino, src_sb.st_dev);
> +              return false;
> +            }
>          }
> 
>        new_dst = true;

This looks good, thanks.
Though I don't think there's a need to duplicate the blocks above:
One could minimize to:

-      if (unlink (dst_name) != 0 && errno != ENOENT)
+      if ((S_ISDIR (src_mode) ? rmdir : unlink) (dst_name) != 0
+          && errno != ENOENT)

Though I think unlink at least can be a function like macro,
and so the above could cause compile issues on some platforms.
Therefore I'm adjusting to the following equivalent and will then push:

-      if (unlink (dst_name) != 0 && errno != ENOENT)
+      if ((S_ISDIR (src_mode) ? rmdir (dst_name) : unlink (dst_name)) != 0
+          && errno != ENOENT)

Note we're checking the type of src which is a bit confusing
since we're removing dst.  Now we could check dst as the
overwrite dir with non dir case is checked for earlier (and vice versa).
But I guess this is a double check for this case.
I'll add a comment along these lines.

I'll add a test too.

thanks,
Pádraig.

[Message part 3 (message/rfc822, inline)]
From: Ken Booth <ken <at> booths.org.uk>
To: bug-coreutils <at> gnu.org
Cc: Ken Booth <kbooth <at> redhat.com>
Subject: mv directory cross-filesystem where destination exists fails to remove
 dest with EISDIR
Date: Mon, 01 Jul 2013 22:21:36 +0100
Hi,

I have found a bug which affects RHEL 5.9, RHEL 6.4, Fedora 18, and the 
latest version of coreutils from the git repo this morning (1st July 
2013) in exactly the same way, and yet does not affect Solaris 10's mv 
command.

Test case:

f18 # mkdir /test /home/test
f18 # cp /etc/passwd /home/test
f18 # mv /home/test /
mv: overwrite ‘/test’? y
mv: inter-device move failed: ‘/home/test’ to ‘/test’; unable to remove 
target: Is a directory

Actual results:
mv: inter-device move failed: ‘/home/test’ to ‘/test’; unable to remove 
target: Is a directory

Expected results:
/home/test/passwd is copied to /test/passwd

I have determined that the problem can be "fixed" (made to behave the 
same as Solaris) by editting src/copy.c as follows:

2176c2176
< if (unlink (dst_name) != 0 && errno != ENOENT)
---
>       if (unlink (dst_name) != 0 &&  errno != ENOENT && errno != EISDIR)
2267,2269c2267,2278
< error (0, errno, _("cannot create directory %s"),
< quote (dst_name));
< goto un_backup;
---
> if (errno == EEXIST)
> {
> if (lchmod (dst_name, dst_mode & ~omitted_permissions) != 0)
> {
> error (0, errno, _("setting permissions for %s"),
> quote (dst_name));
> }
> } else {
> error (0, errno, _("cannot create directory %s"),
> quote (dst_name));
> goto un_backup;
> }

This has the same effect as mv does on Solaris 10.

Where the destination directory exists on the new mount point, the 
permissions are modified and new files will overwrite existing files in 
the destination tree. However, unique files in the destination directory 
will not be removed.

The BIG question is:
There is obviously a bug in the GNU mv command as its failing to unlink 
the directory, however ...

should the GNU mv command behave the same way as the Solaris mv commnd?

e.g.
f18# ls -la /home/test /test
/home/test:
total 12
drwxr-sr-x. 2 root root 4096 Jul 1 21:18 .
drwxr-xr-x. 14 root root 4096 Jul 1 21:18 ..
-rw-r--r--. 1 root root 2768 Jul 1 21:18 passwd

/test:
total 16
drwxr-xr-x. 2 root root 4096 Jul 1 21:20 .
dr-xr-xr-x. 25 root root 4096 Jul 1 21:09 ..
-rw-r--r--. 1 root root 1162 Jul 1 12:53 group
-rw-r--r--. 1 root root 2777 Jul 1 21:20 passwd
f18# /home/ken/git/fedora/coreutils/src/mv /home/test /
f18# ls -la /home/test /test
ls: cannot access /home/test: No such file or directory
/test:
total 16
drwxr-sr-x. 2 root root 4096 Jul 1 21:18 .
dr-xr-xr-x. 25 root root 4096 Jul 1 21:09 ..
-rw-r--r--. 1 root root 1162 Jul 1 12:53 group
-rw-r--r--. 1 root root 2768 Jul 1 21:18 passwd

or should we really be deleting the directory as defined by the man page 
wording "overwrite" and observed by the failed attempt at unlinking ?

e.g.
should we delete /test/group

Which behaviour is correct?

Regards,
Ken Booth
Red Hat UK Ltd

PS: There is a Red Hat bugzilla for this
https://bugzilla.redhat.com/show_bug.cgi?id=980061




This bug report was last modified 11 years and 301 days ago.

Previous Next


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