On 02/01/2012 01:47 PM, Jim Meyering wrote:
> Bernhard Voelker wrote:
>> Playing around with the latest mv checkin,
>> I noticed another corner case:
>>
>> Create a file 'f', a symlink 'l' to it, and
>> then a hardlink 's' to that symlink:
>>
>>   $ touch f && ln -s f l && ln l s && ls -ogi
>>   total 0
>>   6444 -rw-r--r-- 1 0 Feb  1 08:52 f
>>   6462 lrwxrwxrwx 2 1 Feb  1 08:52 l -> f
>>   6462 lrwxrwxrwx 2 1 Feb  1 08:52 s -> f
> 
> Hi Bernhard,
> Thanks for the report.
> 
> Here, you've already gotten into questionable territory, since the
> idea of a hard link to a symbolic link is not standardized.
> For example, on FreeBSD 9.0, you cannot even create one:
> (instead, it hard-links the referent of the symlink)
> 
>     freebsd$ touch f && ln -s f l && ln l s && ls -ogi f l s
>     1587047 -rw-r--r-- 2 0 Feb  1 04:58 f
>     1587100 lrwxr-xr-x 1 1 Feb  1 04:58 l -> f
>     1587047 -rw-r--r-- 2 0 Feb  1 04:58 s
> 
>> Trying to mv the hardlink over the symlink seems to succeed:
>>
>>   $ ~/git/coreutils/src/mv s l
>>
>> ... but the name 's' was not unlinked:
> 
> That is definitely undesirable behavior.
> For now, one possibility is to make "mv" reject
> that corner case with this diagnostic:
> 
>     mv: 's' and 'l' are the same file
> 
> Proposed patch below.
> 
>>   $ ls -ogi
>>   total 0
>>   6444 -rw-r--r-- 1 0 Feb  1 08:52 f
>>   6462 lrwxrwxrwx 2 1 Feb  1 08:52 l -> f
>>   6462 lrwxrwxrwx 2 1 Feb  1 08:52 s -> f
>>
>> Using the -v option looks also normal, but is a nop:
>>
>>   $ ~/git/coreutils/src/mv -v s l
>>   ‘s’ -> ‘l’
>>   $ ls -ogi
>>   total 0
>>   6444 -rw-r--r-- 1 0 Feb  1 08:52 f
>>   6462 lrwxrwxrwx 2 1 Feb  1 08:52 l -> f
>>   6462 lrwxrwxrwx 2 1 Feb  1 08:52 s -> f
>>
>> The strace only shows the rename():
>>
>>   stat("l", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
>>   lstat("s", {st_mode=S_IFLNK|0777, st_size=1, ...}) = 0
>>   lstat("l", {st_mode=S_IFLNK|0777, st_size=1, ...}) = 0
>>   rename("s", "l")                        = 0
>>
>>
>> I'd have expected either
>> a) the name 's' to be unlinked, or
>> b) an error message that something was wrong (well, whatever).
>> I'd prefer a) of course.
> 
> So would I.
> 
>> That behaviour didn't change at least since version 7.1
>> (on openSuSE-11.3), but back in the earlier days in 5.93
>> (SLES-10.3), 's' disappeared as expected:
>>
>>   stat("l", {st_mode=S_IFREG|0640, st_size=0, ...}) = 0
>>   lstat("s", {st_mode=S_IFLNK|0777, st_size=1, ...}) = 0
>>   lstat("l", {st_mode=S_IFLNK|0777, st_size=1, ...}) = 0
>>   unlink("l")                             = 0
>>   rename("s", "l")                        = 0
>>
>> Does mv now work as specified? Is this a kernel or
>> a coreutils (or a user) issue?
> 
> It's a standards and kernel issue.
> POSIX explicitly says (of rename)
> 
>     If the old argument and the new argument resolve to the same existing
>     file, rename( ) shall return successfully and perform no other action.
> 
> though that particular wording might be slated to change with POSIX 2008,
> to allow different (more desirable) behavior.  Search the austin-group
> archives if you need to be sure.
> 
> Also, I don't know whether the above wording applies to this particular
> case of two hard links to the same symlink.  Again, I think we're in
> unspecified territory.
> 
> Here's a proposed patch.
> This is such an improbable corner case that I think it's not worth trying
> to fix any other way (either by unlinking the destination or by
> replacing rename).
> 
> (of course, if I go with this, I'll insert a big comment justifying
> this tiny change)
> 
> diff --git a/src/copy.c b/src/copy.c
> index 4810de8..78cd8c8 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -1229,7 +1229,7 @@ same_file_ok (char const *src_name, struct stat const 
> *src_sb,
>           know this here IFF preserving symlinks), then it's ok -- as long
>           as they are distinct.  */
>        if (S_ISLNK (src_sb->st_mode) && S_ISLNK (dst_sb->st_mode))
> -        return ! same_name (src_name, dst_name);
> +        return ! same_name (src_name, dst_name) && ! same_link;
> 
>        src_sb_link = src_sb;
>        dst_sb_link = dst_sb;

Hi Jim,

well, if it's not standardized (yet), then I agree with you that we
should choose the simpler b) alternative.

Do you think it's work thinking about the -f case?

  $ ~/git/coreutils/src/mv -f s l
  /home/berny/git/coreutils/src/mv: ‘s’ and ‘l’ are the same file

The info pages are not quite clear in which cases mv tries to
overwrite the destination. Usually, I am tempted to think that
a "--force" will "just do it" - regardless of data loss.
But in this case, mv "just does not" and the only thing left
is to rm that hardlink.

That makes me think that the implementation of the a) alternative
just reduces to calling unlink() ... ;-)

Have a nice day,
Berny



Reply via email to