Re: Buggy mv --interactive --reply=no, hopefully solved here

[Jim Meyering]

Vlada Macek <address@hidden> wrote:
> I still believe there is a bug in `mv'. Try to run
>
>     mv --interactive --reply=no plain1 plain2
>
> and given both plains exist, plain2 gets overwritten. This is not
> expected behavior, not just by me (there is a Debian bug filled in).
> Command `cp' is not affected.
>
> The problem seems to be fixed by this:
>
> --- coreutils-cvs/src/copy.c    2005-04-11 22:06:34.000000000 +0200
> +++ coreutils/src/copy.c        2005-05-09 21:43:40.539405480 +0200

Thank you for the report and patch.
I've applied it, adjusted some comments, and added a test case.
The change is committed, but there will be some delay before
it reaches savannah.

2005-05-10  Jim Meyering  <address@hidden>

        * src/copy.c (abandon_move): Remove erroneous UNWRITABLE check.
        This makes `mv -i --reply=no f1 f2' work as expected (in not
        performing the move operation).  But note that specifying `-i'
        after `--reply=no' does *not* work.
        Tiny patch from Vlada Macek.
        Correct a comment.
        * tests/mv/reply-no: New file.  Test for the above fix.
        * tests/mv/Makefile.am (TESTS): Add reply-no.

Index: src/copy.c
===================================================================
RCS file: /fetish/cu/src/copy.c,v
retrieving revision 1.179
retrieving revision 1.180
diff -u -p -u -r1.179 -r1.180
--- src/copy.c  11 Apr 2005 20:06:34 -0000      1.179
+++ src/copy.c  10 May 2005 07:35:43 -0000      1.180
@@ -798,8 +798,8 @@ record_file (Hash_table *ht, char const
 
 /* When effecting a move (e.g., for mv(1)), and given the name DST_PATH
    of the destination and a corresponding stat buffer, DST_SB, return
-   true if the logical `move' operation should not proceed.
-   Return true if it may proceed.
+   true if the logical `move' operation should _not_ proceed.
+   Otherwise, return false.
    Depending on options specified in X, this code may issue an
    interactive prompt asking whether it's ok to overwrite DST_PATH.  */
 static bool
@@ -808,8 +808,7 @@ abandon_move (const struct cp_options *x
               struct stat const *dst_sb)
 {
   assert (x->move_mode);
-  return ((x->interactive == I_ALWAYS_NO
-           && UNWRITABLE (dst_path, dst_sb->st_mode))
+  return (x->interactive == I_ALWAYS_NO
           || ((x->interactive == I_ASK_USER
                || (x->interactive == I_UNSPECIFIED
                    && x->stdin_tty