From unknown Wed Jun 18 23:01:06 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#14763 <14763@debbugs.gnu.org> To: bug#14763 <14763@debbugs.gnu.org> Subject: Status: mv directory cross-filesystem where destination exists fails to remove dest with EISDIR Reply-To: bug#14763 <14763@debbugs.gnu.org> Date: Thu, 19 Jun 2025 06:01:06 +0000 retitle 14763 mv directory cross-filesystem where destination exists fails = to remove dest with EISDIR reassign 14763 coreutils submitter 14763 ken@booths.org.uk severity 14763 normal thanks From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 01 17:24:52 2013 Received: (at submit) by debbugs.gnu.org; 1 Jul 2013 21:24:52 +0000 Received: from localhost ([127.0.0.1]:51095 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Utlad-0004H8-S3 for submit@debbugs.gnu.org; Mon, 01 Jul 2013 17:24:52 -0400 Received: from eggs.gnu.org ([208.118.235.92]:57826) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1UtlXt-0004Bi-T5 for submit@debbugs.gnu.org; Mon, 01 Jul 2013 17:22:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UtlXj-0000DE-DU for submit@debbugs.gnu.org; Mon, 01 Jul 2013 17:21:56 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50,FREEMAIL_FROM, T_DKIM_INVALID autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:60120) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UtlXj-0000DA-AT for submit@debbugs.gnu.org; Mon, 01 Jul 2013 17:21:51 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59457) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UtlXd-0000oY-Fw for bug-coreutils@gnu.org; Mon, 01 Jul 2013 17:21:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UtlXY-0000AW-9X for bug-coreutils@gnu.org; Mon, 01 Jul 2013 17:21:45 -0400 Received: from mail-wg0-x229.google.com ([2a00:1450:400c:c00::229]:56362) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UtlXY-0000AG-4W for bug-coreutils@gnu.org; Mon, 01 Jul 2013 17:21:40 -0400 Received: by mail-wg0-f41.google.com with SMTP id y10so3997275wgg.0 for ; Mon, 01 Jul 2013 14:21:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:message-id:date:from:reply-to:user-agent:mime-version:to:cc :subject:content-type:content-transfer-encoding; bh=YRnad6cW6LuRYTdKXO6i0Alrypp4UnS6dWqY0ksCo2M=; b=R7vqJm/I4oLDdRiU9YQ+mAOZy9nBW6nP9A3tKa1HlckwmC+F3BdrDZbMZq2OtwlUx/ NTjhVbhcAFeQ6lsi0eTAk6Mi7hUSmJPvxc1GjRHF3R6gqeMNFAAyzAo7rIIFWSw3FIsn AX6mGXz8Ra0cc9PXHdxWNnwwYZlvy/3/CE5pWOpaF8o+MaSGzcQCeJ0Sx6n7/YTAe1XS JhZi/bPHJXc5EJDBIucD8mIX+Y2ndPxGk7d6JioyhI7/yZenqpaYrJPvHq/Q7ZkcLEHZ qrtlwevW6Tp/sLEG55//n05/5I8SFObw74KroFQIwh5N0d+oRDXo/wY5BbUtyHvY0NrW MHEQ== X-Received: by 10.194.82.193 with SMTP id k1mr20802729wjy.21.1372713699003; Mon, 01 Jul 2013 14:21:39 -0700 (PDT) Received: from banjo.usersys.redhat.com ([2001:470:69e3:1::15]) by mx.google.com with ESMTPSA id ev19sm19002511wid.2.2013.07.01.14.21.37 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Mon, 01 Jul 2013 14:21:38 -0700 (PDT) Message-ID: <51D1F2E0.2000100@booths.org.uk> Date: Mon, 01 Jul 2013 22:21:36 +0100 From: Ken Booth User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 MIME-Version: 1.0 To: bug-coreutils@gnu.org Subject: mv directory cross-filesystem where destination exists fails to remove dest with EISDIR Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -4.3 (----) X-Debbugs-Envelope-To: submit X-Mailman-Approved-At: Mon, 01 Jul 2013 17:24:50 -0400 Cc: Ken Booth X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: ken@booths.org.uk List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -4.3 (----) 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 From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 01 18:28:17 2013 Received: (at 14763) by debbugs.gnu.org; 1 Jul 2013 22:28:17 +0000 Received: from localhost ([127.0.0.1]:51166 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Utma0-0005rG-FD for submit@debbugs.gnu.org; Mon, 01 Jul 2013 18:28:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:8411) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1UtmZw-0005r1-Hx for 14763@debbugs.gnu.org; Mon, 01 Jul 2013 18:28:14 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r61MROaf025082 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 1 Jul 2013 18:27:25 -0400 Received: from [10.3.113.132] (ovpn-113-132.phx2.redhat.com [10.3.113.132]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r61MROE4002491; Mon, 1 Jul 2013 18:27:24 -0400 Message-ID: <51D2024B.1010203@redhat.com> Date: Mon, 01 Jul 2013 16:27:23 -0600 From: Eric Blake Organization: Red Hat, Inc. User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: ken@booths.org.uk Subject: Re: bug#14763: mv directory cross-filesystem where destination exists fails to remove dest with EISDIR References: <51D1F2E0.2000100@booths.org.uk> In-Reply-To: <51D1F2E0.2000100@booths.org.uk> X-Enigmail-Version: 1.5.1 OpenPGP: url=http://people.redhat.com/eblake/eblake.gpg Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2HNKWHOOPRGNNLPELWFSF" X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: 14763 Cc: 14763@debbugs.gnu.org, Ken Booth X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -5.0 (-----) This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2HNKWHOOPRGNNLPELWFSF Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/01/2013 03:21 PM, Ken Booth wrote: > Hi, >=20 > 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. >=20 > Test case: >=20 > f18 # mkdir /test /home/test > f18 # cp /etc/passwd /home/test > f18 # mv /home/test / > mv: overwrite =E2=80=98/test=E2=80=99? y > mv: inter-device move failed: =E2=80=98/home/test=E2=80=99 to =E2=80=98= /test=E2=80=99; unable to remove > target: Is a directory Let's read what POSIX has to say about this: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/mv.html We are using the second form (mv source_file... target_dir) with source_file of /home/test and target_dir of /. The destination path is thus constructed to be "/test". Step 1 says: >> If the destination path exists, the -f option is not specified, an= d either of the following conditions is true: >>=20 >> The permissions of the destination path do not permit writing = and the standard input is a terminal. >>=20 >> The -i option is specified. >>=20 >> the mv utility shall write a prompt to standard error and read a l= ine from standard input. If the response is not affirmative, mv shall do = nothing more with the current source_file and go on to any remaining sour= ce_files. The destination "/test" exists, but permissions allow writing (well, assuming your umask is sane when you did mkdir) and -i is not specified, so this step does not stop us, and we go on to step 2. >> If the source_file operand and destination path name the same existing= file... Not the same, so on to step 3. >> The mv utility shall perform actions equivalent to the rename() functi= on defined in the System Interfaces volume of POSIX.1-2008, called with t= he following arguments: >>=20 >> The source_file operand is used as the old argument. >>=20 >> The destination path is used as the new argument. >>=20 >> If this succeeds, mv shall do nothing more with the current source_fil= e and go on to any remaining source_files. If this fails for any reasons = other than those described for the errno [EXDEV] in the System Interfaces= volume of POSIX.1-2008, mv shall write a diagnostic message to standard = error, do nothing more with the current source_file, and go on to any rem= aining source_files. so now we have to cross-reference to see what is required for rename("/home/test", "/test"): http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html >> If the old argument points to the pathname of a directory, the new arg= ument 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 an= d old renamed to new. In this case, a link named new shall exist througho= ut the renaming operation and shall refer either to the directory referre= d to by new or old before the operation began. If new names an existing d= irectory, it shall be required to be an empty directory. Well, we just proved by your above construction that /test is empty, and therefore should silently be replaced by the (non-empty) directory that used to be named /home/test. Therefore, the rename() should succeed, and mv should do nothing further for the /home/test argument; and the result of the rename is that the former /home/test/passwd should now be located at /test/passwd. You are correct that GNU mv has a bug. Hmm, I seem to recall reporting a similar bug in the past regarding the behavior on symlinks - a bit of research turned up commit f1d1ee912 in May 2006 regarding non-atomic rename() of symlinks; now we are dealing with non-atomic rename() of empty destination directories. > Actual results: > mv: inter-device move failed: =E2=80=98/home/test=E2=80=99 to =E2=80=98= /test=E2=80=99; unable to remove > target: Is a directory >=20 > Expected results: > /home/test/passwd is copied to /test/passwd Concur that this is what POSIX requires. >=20 > I have determined that the problem can be "fixed" (made to behave the > same as Solaris) by editting src/copy.c as follows: >=20 > 2176c2176 > < if (unlink (dst_name) !=3D 0 && errno !=3D ENOENT) > --- >> if (unlink (dst_name) !=3D 0 && errno !=3D ENOENT && errno !=3D= EISDIR) Ed script patches are illegible. Please instead submit this as a context diff. These directions in HACKING may help: http://git.sv.gnu.org/gitweb/?p=3Dcoreutils.git;a=3Dblob;f=3DHACKING;h=3D= 49f97381;hb=3DHEAD As is, I don't think your ed script patch is correct - it is not atomic. In other words, I don't think we should be trying unlink() followed by special-case handling based on certain errno values, but instead should be trying rename() up front. Since the results are required to be atomic, the success or failure of rename() will tell us whether we the source directory was correctly renamed atop a previously empty destination directory. >=20 > This has the same effect as mv does on Solaris 10. >=20 > 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 director= y > will not be removed. Huh? Nothing in the POSIX wording talked about altering permissions of the destination directory after a rename of source dir to empty target di= r. > PS: There is a Red Hat bugzilla for this > https://bugzilla.redhat.com/show_bug.cgi?id=3D980061 Thanks for the pointer. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2HNKWHOOPRGNNLPELWFSF Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJR0gJLAAoJEKeha0olJ0Nq17oIAJxF0CInBZfWbxAaF9DUwVMx fBqB18FoyRc1xk6CkY7ILYN8CLTjA7p5+wDKiRgwyLd6ijmVq5xtovPfXorxLhEZ 9+qMl06/Rwu7dOlozJULDl5NQmJDWOhC8cxaYJ5SRuz5BQxk8yZ5AwKyJxKeCRVt 8vRc1O1yqDO5yF5yxsX8TZj4xB6AGpcvPvAmRo04padJiV5OAO+8lle82OxECPwJ 8MITExj9TbS1cpgP+y3n22TfD438frnlVf9hCxXSUXDAnaNUAD/ub56/7g6OZdQW 5PL22Lz862zr+DOh+/6Zfpfp/aE5Aj03O22+UcwLXJySTdbDS5xF9fNTh/koPcA= =Nw7W -----END PGP SIGNATURE----- ------enig2HNKWHOOPRGNNLPELWFSF-- From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 01 18:35:40 2013 Received: (at 14763) by debbugs.gnu.org; 1 Jul 2013 22:35:40 +0000 Received: from localhost ([127.0.0.1]:51174 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Utmh8-00063t-Sw for submit@debbugs.gnu.org; Mon, 01 Jul 2013 18:35:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13635) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Utmh5-00063h-Be for 14763@debbugs.gnu.org; Mon, 01 Jul 2013 18:35:36 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r61MYmn6020779 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 1 Jul 2013 18:34:48 -0400 Received: from [10.3.113.132] (ovpn-113-132.phx2.redhat.com [10.3.113.132]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r61MYlTv004480; Mon, 1 Jul 2013 18:34:47 -0400 Message-ID: <51D20407.6040701@redhat.com> Date: Mon, 01 Jul 2013 16:34:47 -0600 From: Eric Blake Organization: Red Hat, Inc. User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 Subject: Re: bug#14763: mv directory cross-filesystem where destination exists fails to remove dest with EISDIR References: <51D1F2E0.2000100@booths.org.uk> <51D2024B.1010203@redhat.com> In-Reply-To: <51D2024B.1010203@redhat.com> X-Enigmail-Version: 1.5.1 OpenPGP: url=http://people.redhat.com/eblake/eblake.gpg Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2QVBXHBSEAOJXUFLKMBKO" X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Spam-Score: -3.8 (---) X-Debbugs-Envelope-To: 14763 Cc: 14763@debbugs.gnu.org, ken@booths.org.uk, Ken Booth X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.8 (---) This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2QVBXHBSEAOJXUFLKMBKO Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/01/2013 04:27 PM, Eric Blake wrote: > Not the same, so on to step 3. >=20 >>> The mv utility shall perform actions equivalent to the rename() funct= ion defined in the System Interfaces volume of POSIX.1-2008, called with = the following arguments: >>> >>> The source_file operand is used as the old argument. >>> >>> The destination path is used as the new argument. >>> >>> If this succeeds, mv shall do nothing more with the current source_fi= le and go on to any remaining source_files. If this fails for any reasons= other than those described for the errno [EXDEV] in the System Interface= s volume of POSIX.1-2008, mv shall write a diagnostic message to standard= error, do nothing more with the current source_file, and go on to any re= maining source_files. >=20 > so now we have to cross-reference to see what is required for > rename("/home/test", "/test"): > http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html >=20 >>> If the old argument points to the pathname of a directory, the new ar= gument 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 a= nd old renamed to new. In this case, a link named new shall exist through= out the renaming operation and shall refer either to the directory referr= ed to by new or old before the operation began. If new names an existing = directory, it shall be required to be an empty directory. >=20 > Well, we just proved by your above construction that /test is empty, an= d > therefore should silently be replaced by the (non-empty) directory that= > used to be named /home/test. Therefore, the rename() should succeed, Oops, I missed one point - rename() is allowed to fail cross-filesystem with EXDEV, and that's part of your testcase (at least, I assume you mean that /test and /home/test are different filesystems, and therefore the atomic rename() fails and we have to plow on with the next steps in POSIX). So, because the failure is EXDEV, we go on to step 4: >> If the destination path exists, and it is a file of type directory and= source_file is not a file of type directory, or it is a file not of type= directory and source_file is a file of type directory, mv shall write a = diagnostic message to standard error, do nothing more with the current so= urce_file, and go on to any remaining source_files. If the destination pa= th exists and was created by a previous step, it is unspecified whether t= his will treated as an error or the destination path will be overwritten.= The destination exists, but we don't have mismatch between directory/non-directory. So we don't error out, but instead go on to step 5: >> If the destination path exists, mv shall attempt to remove it. If this= fails for any reason, mv shall write a diagnostic message to standard er= ror, do nothing more with the current source_file, and go on to any remai= ning source_files. Aha - we are attempting to remove it with unlink(). But for empty directories, we should be using rmdir(), or even better, we should be using remove() (which subsumes both unlink() and rmdir() at once). I wonder if a simpler patch would be to just s/unlink/rmdir/ in the line of code where you were adding special-casing on errno values. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2QVBXHBSEAOJXUFLKMBKO Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJR0gQHAAoJEKeha0olJ0Nq6BkH/0IuzHmpo5vxk9hTpXEvgqB6 sU1rZh6mdKTDwqVcSJ6VtJEGSw+UOnprZKjbSRuIBnBQZx6OKnS7kSMQRC0WwiBS fapA0CDE25VuUt+XkbehMlx/0Ld0OxbpmqFA5Clw2Nn1r5CMTVYSB0WGs7v1MMLl As1+deMKNDACZaBDqf9XX59B89b/pg5+lw8as191+lIdStBUSA2CUJKD8hsVnJII b7iSzWt57Xf9KruOilOYcBRiv6yXdu2eT1LT7tyf14pxgbg6bssrWYeFNsQ/NIjs lHWIi1BeI+l+nH5qqlLu8rqqRSYoOijsvKe+FxjE0cbRsgTTMK0JBNZp+KBUTGk= =gtpu -----END PGP SIGNATURE----- ------enig2QVBXHBSEAOJXUFLKMBKO-- From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 01 18:42:00 2013 Received: (at 14763) by debbugs.gnu.org; 1 Jul 2013 22:42:00 +0000 Received: from localhost ([127.0.0.1]:51185 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1UtmnH-0006GM-JZ for submit@debbugs.gnu.org; Mon, 01 Jul 2013 18:41:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:4700) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1UtmnD-0006G7-AK for 14763@debbugs.gnu.org; Mon, 01 Jul 2013 18:41:56 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r61Mf8ig029328 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 1 Jul 2013 18:41:08 -0400 Received: from [10.3.113.132] (ovpn-113-132.phx2.redhat.com [10.3.113.132]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r61Mf7gM020555; Mon, 1 Jul 2013 18:41:07 -0400 Message-ID: <51D20583.2050302@redhat.com> Date: Mon, 01 Jul 2013 16:41:07 -0600 From: Eric Blake Organization: Red Hat, Inc. User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 Subject: Re: bug#14763: mv directory cross-filesystem where destination exists fails to remove dest with EISDIR References: <51D1F2E0.2000100@booths.org.uk> <51D2024B.1010203@redhat.com> <51D20407.6040701@redhat.com> In-Reply-To: <51D20407.6040701@redhat.com> X-Enigmail-Version: 1.5.1 OpenPGP: url=http://people.redhat.com/eblake/eblake.gpg Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2OXUXKJLACOLPBAOPERPF" X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-Spam-Score: -3.8 (---) X-Debbugs-Envelope-To: 14763 Cc: 14763@debbugs.gnu.org, ken@booths.org.uk, Ken Booth X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.8 (---) This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2OXUXKJLACOLPBAOPERPF Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/01/2013 04:34 PM, Eric Blake wrote: >>> If the destination path exists, mv shall attempt to remove it. If thi= s fails for any reason, mv shall write a diagnostic message to standard e= rror, do nothing more with the current source_file, and go on to any rema= ining source_files. >=20 > Aha - we are attempting to remove it with unlink(). But for empty > directories, we should be using rmdir(), or even better, we should be > using remove() (which subsumes both unlink() and rmdir() at once). I > wonder if a simpler patch would be to just s/unlink/rmdir/ in the line > of code where you were adding special-casing on errno values. Then again, we DON'T want to replace a non-directory with a directory (or vice-versa, we don't want to replace an empty directory with a non-directory); so maybe it pays to be more careful about explicitly using unlink() vs. rmdir() on the destination (and not the shortcut of remove() which does not care about type), all based on what file type we already know that the source is, so that we can give the same sorts of failures as rename() would give on a local file system when attempting a cross-file-type rename. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2OXUXKJLACOLPBAOPERPF Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJR0gWDAAoJEKeha0olJ0NqRp4H/3SbFQNnuDfDMFRU4EDPbAXJ hzm5aEB+M1iYP4vsVDm34eNi4N8mewTyg1JXOisDvWtSay97Pm1lAWqbsjNZu2vH AuD3JVa2AmFcnIH01A1gqkhSCGg7WxlJ2chmqdxip2QPvq0FsljanLEP6kYDQkY/ 1MS/Ae5Lag0b0R98HCSFBqGd/KiJJLN8mPgZY293Vfc+Agj/EjddeoK5uNxEuep2 iVt68RsJAf4Rev3cWOwWKg8p/Tbq7wrOY2RuKeNnEIDHLHlkhDzhq9tmk5B01IAk Sw/gnTfV0UJWpQamZ3n4i28T/wyAlyXuN0jpu01zBpE1XASqeeNnMdb3rLemCaI= =P4ug -----END PGP SIGNATURE----- ------enig2OXUXKJLACOLPBAOPERPF-- From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 01 20:29:56 2013 Received: (at 14763) by debbugs.gnu.org; 2 Jul 2013 00:29:56 +0000 Received: from localhost ([127.0.0.1]:51313 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1UtoTj-0000Zp-NE for submit@debbugs.gnu.org; Mon, 01 Jul 2013 20:29:56 -0400 Received: from mail-wi0-f177.google.com ([209.85.212.177]:34050) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1UtoTg-0000Zb-Js for 14763@debbugs.gnu.org; Mon, 01 Jul 2013 20:29:53 -0400 Received: by mail-wi0-f177.google.com with SMTP id ey16so3637207wid.10 for <14763@debbugs.gnu.org>; Mon, 01 Jul 2013 17:29:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:message-id:date:from:reply-to:user-agent:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=kdIfYd7zbQAqMRgz1Dx+fHsMT5zym54reCrofu4w058=; b=U8rsSnjz9soMKcooW9YKImXhOg7As3PU2i/3vTxbsVei0Yr8slGmuzqDVNChp2G19+ ERCsCdjHVDyAc2EHB7rUgJVNdec+HCpoE9YE7js+M5cVTHO4DJg36FnmUpHTx9jpeuxL kd4XYLGIm9V+WfcQx/zP6wyuCclrILvbcaVGLmMBwN+yILaGttgOvlProgoc3kyL9ZzM 5PL7uPp53bny0KwJ1+5vfSgdaaC1kK7KeWdzXZH79jgKBhwRJXIkT/v/4EBcaHWPzmiH aFdCEe3A0r6+3iWslD0GXn2trx+UfaoSP4wcZFm1XLkhI+35gWf6225wkgq2EY/6mEQp tFAA== X-Received: by 10.195.12.133 with SMTP id eq5mr21715772wjd.27.1372724986803; Mon, 01 Jul 2013 17:29:46 -0700 (PDT) Received: from banjo.usersys.redhat.com ([2001:470:69e3:1::15]) by mx.google.com with ESMTPSA id b20sm19643454wiw.4.2013.07.01.17.29.45 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Mon, 01 Jul 2013 17:29:45 -0700 (PDT) Message-ID: <51D21EF8.3070104@booths.org.uk> Date: Tue, 02 Jul 2013 01:29:44 +0100 From: Ken Booth User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 MIME-Version: 1.0 To: Eric Blake Subject: Re: bug#14763: mv directory cross-filesystem where destination exists fails to remove dest with EISDIR References: <51D1F2E0.2000100@booths.org.uk> <51D2024B.1010203@redhat.com> <51D20407.6040701@redhat.com> <51D20583.2050302@redhat.com> In-Reply-To: <51D20583.2050302@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 14763 Cc: 14763@debbugs.gnu.org, Ken Booth X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: ken@booths.org.uk List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) 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 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; -- 1.8.1.4 I've tested it works with an empty directory, and produces the following error with a non-empty directory /home/ken/git/fedora/coreutils/src/mv: inter-device move failed: ‘/home/test’ to ‘/test’; unable to remove target directory: Directory not empty I'm not sure of the etiquette here, but should I push the change, or leave that to a maintainer? Regards, Ken. On 01/07/13 23:41, Eric Blake wrote: > On 07/01/2013 04:34 PM, Eric Blake wrote: >>>> If the destination path exists, mv shall attempt to remove it. If this fails for any reason, mv shall write a diagnostic message to standard error, do nothing more with the current source_file, and go on to any remaining source_files. >> Aha - we are attempting to remove it with unlink(). But for empty >> directories, we should be using rmdir(), or even better, we should be >> using remove() (which subsumes both unlink() and rmdir() at once). I >> wonder if a simpler patch would be to just s/unlink/rmdir/ in the line >> of code where you were adding special-casing on errno values. > Then again, we DON'T want to replace a non-directory with a directory > (or vice-versa, we don't want to replace an empty directory with a > non-directory); so maybe it pays to be more careful about explicitly > using unlink() vs. rmdir() on the destination (and not the shortcut of > remove() which does not care about type), all based on what file type we > already know that the source is, so that we can give the same sorts of > failures as rename() would give on a local file system when attempting a > cross-file-type rename. > From debbugs-submit-bounces@debbugs.gnu.org Wed Jul 24 13:15:26 2013 Received: (at 14763-done) by debbugs.gnu.org; 24 Jul 2013 17:15:26 +0000 Received: from localhost ([127.0.0.1]:47400 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1V22er-0008Dh-If for submit@debbugs.gnu.org; Wed, 24 Jul 2013 13:15:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54553) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1V22em-0008DM-0O for 14763-done@debbugs.gnu.org; Wed, 24 Jul 2013 13:15:23 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r6OHEWmL019895 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 24 Jul 2013 13:14:33 -0400 Received: from [10.36.116.79] (ovpn-116-79.ams2.redhat.com [10.36.116.79]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r6OHETE2015234 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 24 Jul 2013 13:14:31 -0400 Message-ID: <51F00B75.60500@draigBrady.com> Date: Wed, 24 Jul 2013 18:14:29 +0100 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: ken@booths.org.uk Subject: Re: bug#14763: mv directory cross-filesystem where destination exists fails to remove dest with EISDIR References: <51D1F2E0.2000100@booths.org.uk> <51D2024B.1010203@redhat.com> <51D20407.6040701@redhat.com> <51D20583.2050302@redhat.com> <51D21EF8.3070104@booths.org.uk> In-Reply-To: <51D21EF8.3070104@booths.org.uk> X-Enigmail-Version: 1.5.2 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: 14763-done Cc: 14763-done@debbugs.gnu.org, Eric Blake , Ken Booth X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -5.0 (-----) 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 > 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. From debbugs-submit-bounces@debbugs.gnu.org Wed Jul 24 17:51:13 2013 Received: (at 14763-done) by debbugs.gnu.org; 24 Jul 2013 21:51:13 +0000 Received: from localhost ([127.0.0.1]:47755 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1V26xk-0004FY-LY for submit@debbugs.gnu.org; Wed, 24 Jul 2013 17:51:13 -0400 Received: from mail-wi0-f170.google.com ([209.85.212.170]:46163) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1V26xh-0004FC-Iu for 14763-done@debbugs.gnu.org; Wed, 24 Jul 2013 17:51:10 -0400 Received: by mail-wi0-f170.google.com with SMTP id ey16so5696129wid.5 for <14763-done@debbugs.gnu.org>; Wed, 24 Jul 2013 14:51:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:message-id:date:from:reply-to:user-agent:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=6qL5BwtAaV2IYuIlBM8Xl+eCqcOgXxlLBMfdFaN+u5A=; b=kxM+8SY4AU/I1JZ1UDrT+K6u1pu1mR3HEmUf2CTGs8xHnwMQI4SJ3G373h0Wm3ql8D MCQ+1ECSM3fH3uaJ5amHQ7DZO2s2k0+nH0V7cJSqSKtjBOPoS38kRMiaXg5/p7lcQrFL QVj1usEzsCeAxygoV+gtKKGvMB1SDCOnH6f4MkvirLpBx/XnMsTurFbOYEAPSnYfXuWe U74i9q0xcdmPy59QRl8sgYL1oAwgbiBV+EPMVmiw+UmsCSdFRIJM22e1lEbkSaGYEuH4 vLyFs4AAdAvxSK+3GnSG9KRZ46puvD01npxUF2Cje0kv68VDsiqK5o2Wi15DFm4ET+gW NaTw== X-Received: by 10.194.1.226 with SMTP id 2mr17867146wjp.91.1374702663669; Wed, 24 Jul 2013 14:51:03 -0700 (PDT) Received: from banjo.usersys.redhat.com ([2001:470:69e3:1::15]) by mx.google.com with ESMTPSA id d8sm8247485wiz.0.2013.07.24.14.51.01 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Wed, 24 Jul 2013 14:51:02 -0700 (PDT) Message-ID: <51F04C3F.6070904@booths.org.uk> Date: Wed, 24 Jul 2013 22:50:55 +0100 From: Ken Booth User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= Subject: Re: bug#14763: mv directory cross-filesystem where destination exists fails to remove dest with EISDIR References: <51D1F2E0.2000100@booths.org.uk> <51D2024B.1010203@redhat.com> <51D20407.6040701@redhat.com> <51D20583.2050302@redhat.com> <51D21EF8.3070104@booths.org.uk> <51F00B75.60500@draigBrady.com> In-Reply-To: <51F00B75.60500@draigBrady.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 14763-done Cc: 14763-done@debbugs.gnu.org, Eric Blake , Ken Booth X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: ken@booths.org.uk List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) On 24/07/13 18:14, PĂ¡draig Brady wrote: > 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 >> 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. > Hi PĂ¡draig, I thought about the fact that we're only checking src and decided its the correct behaviour, as we intend to get an error if dst is the wrong type. I should have included a comment to that effect, just to make it clear that we dont want to check dst. So the code as you've written it should stand without an extra check, just a comment. Thanks for approving my first patch. Regards, Ken. From unknown Wed Jun 18 23:01:06 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Thu, 22 Aug 2013 11:24:08 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator