not understanding that the (non-existing) target will be a subdirectory of the source.
mv first tries to rename("/mnt", "/mnt/foo") and only after that fails with EXDEV it decides to mkdir("/mnt/foo")
To me this looks like a bug: the order of the checks has been reversed. mv should never attempt to put a directory into itself, since there is no situation in which that ever makes sense. Only after that check passes should it decide if rename() is enough, or if it has to do a copy/delete.
The problem here is that the error conditions are not mutually exclusive - it is possible to have a rename cross devices and attempt to move a directory into one of its descendants - but there's no specification in the standard[1] of which error should be returned in such a case. The priority of non-mutually-exclusive errors is seldom considered, yet this is a good example of why it's important.
In other words the "rename("/mnt", "/mnt/foo")" should have failed with EINVAL, not EXDEV. A look through the Linux source shows that at least there, cross-device has higher priority than parent-descendant: http://lxr.free-electrons.com/source/fs/namei.c#L4240
After resolving symbolic links that turns into "mv bar foo", which is perfectly acceptable since they are siblings. The checks for the destination being a descendant of the source should be performed after symlink resolution.
There's nothing in the standard about rename()'s error code priority (it could be EXDEV or EINVAL - they are not mutually exclusive), so I'd consider that a bug too.
Presuming that you wouldn't invoke mv nanoseconds before creating the symlink, I think mv should resolve the symlink first and, if the move doesn't make sense, abort. It will still need to handle the case where it looks like the move makes sense, but the kernel returns EINVAL or whatever.
being vigilant about not creating unnecessary race conditions is important. doubly so when you're building a tool upon which others will compose their own programs/scripts/etc.
We have to be careful about doing too much (adding edge cases and complexity), though in this case how about:
canon_src = canonicalize(src);
canon_dst = canonicalize(dst);
int ret = rename(src, dst);
if (ret == EXDEV) {
/* Note if there are symlinks being recreated
between the canonicalize() and rename() above,
then it's better to not fall back to a
cross device copy anyway. */
if (common_prefix(canon_src, canon_dst))
ret = EINVAL; /* Treat like "copy into self" */
}
> In other words the "rename("/mnt", "/mnt/foo")" should have failed with EINVAL, not EXDEV
I thought about that one, too. But I guess there is 2 kinds of subdirectory-of: In the mount hierarchy possibly across devices, and on a single device in terms of inode-reachability.
Absence of the first condition does not preclude the second condition: Many filesystems can be mounted at multiple places.
I think the point of EINVAL is to prevent screwing up the intra-device inode links. For this case, the check can't be made in the mount hierarchy. So it seems natural to me that the EXDEV check comes first.
If the problem were just historical compatibility, I don't see what would be preventing them from adding a flag --do-things-sensibly to turn on the reasonable behavior.
mv first tries to rename("/mnt", "/mnt/foo") and only after that fails with EXDEV it decides to mkdir("/mnt/foo")
To me this looks like a bug: the order of the checks has been reversed. mv should never attempt to put a directory into itself, since there is no situation in which that ever makes sense. Only after that check passes should it decide if rename() is enough, or if it has to do a copy/delete.
The problem here is that the error conditions are not mutually exclusive - it is possible to have a rename cross devices and attempt to move a directory into one of its descendants - but there's no specification in the standard[1] of which error should be returned in such a case. The priority of non-mutually-exclusive errors is seldom considered, yet this is a good example of why it's important.
In other words the "rename("/mnt", "/mnt/foo")" should have failed with EINVAL, not EXDEV. A look through the Linux source shows that at least there, cross-device has higher priority than parent-descendant: http://lxr.free-electrons.com/source/fs/namei.c#L4240
[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/re...