3 ms·
since there is no situation in which that ever makes sense Actually, there is: mkdir foo bar ln -s ../foo bar/foo mv bar bar/foo ...so mv(1) leaves it
by caf 11y ago
since there is no situation in which that ever makes sense
Actually, there is:
mkdir foo bar
ln -s ../foo bar/foo
mv bar bar/foo
...so mv(1) leaves it up to the kernel to detect the error condition, as it should.
- userbinator 11y agoAfter 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.
- caf 11y agoThe kernel has to resolve the symbolic link - /bin/mv should not try because it cannot do so without a race.
- anon4 11y agoPresuming 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.
- cbr 11y ago> Presuming that you wouldn't invoke mv nanoseconds before creating the symlink This kind of thinking has led to lots of security bugs around symlinks.
- mh- 11y agoindeed, and not just symlinks. 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.
- pixelbeat 11y agoTrue, symlinks complicate things. 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" */ }