Subject: Re: bug#17087: cp -i/yes gets ignored

From: Paul Eggert <[email-masked]>
Date: Mon, 24 Mar 2014 23:25:48 -0700

Pádraig Brady wrote:
> That code has been there "from the beginning" and I'm guessing it's
> to prompt users to allow them to chmod the file in a separate terminal?

If we go back far enough (i.e., before commit 6bf3479 dated 2000-08-27), 
this message was output only if -f was also given.  More recently we've 
seen similar complaints, e.g.: 
http://lists.gnu.org/archive/html/bug-coreutils/2007-03/msg00130.html

At least the diagnostic wording could be improved so that users who are 
prompted don't get the false impression that the copy will succeed; 
POSIX clearly allows this.  I installed the attached patches (the second 
is just a code cleanup).  Perhaps that's enough.

From 1e0db24cfb67a85416162bf482801b2ead7a8e8f Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Mon, 24 Mar 2014 23:14:43 -0700
Subject: [PATCH 1/2] cp: improve quality of overwrite prompt

* src/copy.c (overwrite_prompt): New arg X.  All callers changed.
Use X to improve the quality of the prompt (Bug#17087).
* tests/mv/i-2.sh, tests/mv/i-3.sh: Change test to match new prompt.
---
 src/copy.c      | 12 ++++++++----
 tests/mv/i-2.sh |  2 +-
 tests/mv/i-3.sh |  2 +-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 4998386..bd4df05 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1566,7 +1566,8 @@ writable_destination (char const *file, mode_t mode)
 }
 
 static void
-overwrite_prompt (char const *dst_name, struct stat const *dst_sb)
+overwrite_prompt (struct cp_options const *x, char const *dst_name,
+                  struct stat const *dst_sb)
 {
   if (! writable_destination (dst_name, dst_sb->st_mode))
     {
@@ -1574,7 +1575,10 @@ overwrite_prompt (char const *dst_name, struct stat const *dst_sb)
       strmode (dst_sb->st_mode, perms);
       perms[10] = '\0';
       fprintf (stderr,
-               _("%s: try to overwrite %s, overriding mode %04lo (%s)? "),
+               _((x->unlink_dest_before_opening
+                  || x->unlink_dest_after_failed_open)
+                 ? "%s: replace %s, overriding mode %04lo (%s)? "
+                 : "%s: unwritable %s (mode %04lo, %s); try anyway? "),
                program_name, quote (dst_name),
                (unsigned long int) (dst_sb->st_mode & CHMOD_MODE_BITS),
                &perms[1]);
@@ -1638,7 +1642,7 @@ abandon_move (const struct cp_options *x,
                || (x->interactive == I_UNSPECIFIED
                    && x->stdin_tty
                    && ! writable_destination (dst_name, dst_sb->st_mode)))
-              && (overwrite_prompt (dst_name, dst_sb), 1)
+              && (overwrite_prompt (x, dst_name, dst_sb), 1)
               && ! yesno ()));
 }
 
@@ -1913,7 +1917,7 @@ copy_internal (char const *src_name, char const *dst_name,
               if (! S_ISDIR (src_mode)
                   && (x->interactive == I_ALWAYS_NO
                       || (x->interactive == I_ASK_USER
-                          && (overwrite_prompt (dst_name, &dst_sb), 1)
+                          && (overwrite_prompt (x, dst_name, &dst_sb), 1)
                           && ! yesno ())))
                 return true;
             }
diff --git a/tests/mv/i-2.sh b/tests/mv/i-2.sh
index 62d63af..4202b5c 100755
--- a/tests/mv/i-2.sh
+++ b/tests/mv/i-2.sh
@@ -36,7 +36,7 @@ cp -if e f < y > out 2>&1 || fail=1
 
 # Make sure out contains the prompt.
 case "$(cat out)" in
-  "cp: try to overwrite 'f', overriding mode 0000 (---------)?"*) ;;
+  "cp: replace 'f', overriding mode 0000 (---------)?"*) ;;
   *) fail=1 ;;
 esac
 
diff --git a/tests/mv/i-3.sh b/tests/mv/i-3.sh
index e03dc99..690af73 100755
--- a/tests/mv/i-3.sh
+++ b/tests/mv/i-3.sh
@@ -46,7 +46,7 @@ check_overwrite_prompt()
 {
   local delay="$1"
   case "$(cat out)" in
-    "mv: try to overwrite 'g', overriding mode 0000"*) ;;
+    "mv: replace 'g', overriding mode 0000"*) ;;
     *) sleep $delay; return 1;;
   esac
 }
-- 
1.8.5.3

From 6fbac25af9d2227af2611ab33677d61e49be4d9a Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Mon, 24 Mar 2014 23:17:02 -0700
Subject: [PATCH 2/2] cp: simplify overwrite logic

* src/copy.c (overwrite_ok): Rename from overwrite_prompt.  Invoke
yesno instead of having the caller do it; that's cleaner.  Return
bool, not void.  All callers changed.
---
 src/copy.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index bd4df05..71813dc 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1565,9 +1565,9 @@ writable_destination (char const *file, mode_t mode)
           || euidaccess (file, W_OK) == 0);
 }
 
-static void
-overwrite_prompt (struct cp_options const *x, char const *dst_name,
-                  struct stat const *dst_sb)
+static bool
+overwrite_ok (struct cp_options const *x, char const *dst_name,
+              struct stat const *dst_sb)
 {
   if (! writable_destination (dst_name, dst_sb->st_mode))
     {
@@ -1588,6 +1588,8 @@ overwrite_prompt (struct cp_options const *x, char const *dst_name,
       fprintf (stderr, _("%s: overwrite %s? "),
                program_name, quote (dst_name));
     }
+
+  return yesno ();
 }
 
 /* Initialize the hash table implementing a set of F_triple entries
@@ -1642,8 +1644,7 @@ abandon_move (const struct cp_options *x,
                || (x->interactive == I_UNSPECIFIED
                    && x->stdin_tty
                    && ! writable_destination (dst_name, dst_sb->st_mode)))
-              && (overwrite_prompt (x, dst_name, dst_sb), 1)
-              && ! yesno ()));
+              && ! overwrite_ok (x, dst_name, dst_sb)));
 }
 
 /* Print --verbose output on standard output, e.g. 'new' -> 'old'.
@@ -1917,8 +1918,7 @@ copy_internal (char const *src_name, char const *dst_name,
               if (! S_ISDIR (src_mode)
                   && (x->interactive == I_ALWAYS_NO
                       || (x->interactive == I_ASK_USER
-                          && (overwrite_prompt (x, dst_name, &dst_sb), 1)
-                          && ! yesno ())))
+                          && ! overwrite_ok (x, dst_name, &dst_sb))))
                 return true;
             }
 
-- 
1.8.5.3