cp: fix the --no-preserve=mode option

The --no-preserve=mode option did not do what its name implies:
it would mistakenly preserve permission mode bits.
* NEWS: Mention the fix.
* TODO: Remove an entry.
* src/copy.c (copy_reg): Add a condition to properly
handle the --no-preserve=mode option for files
(copy_internal): Add a condition to properly handle the
--no-preserve=mode option for directories.
* src/copy.h (struct cp_options): Add a new boolean.
* src/cp.c (cp_option_init,decode_preserve_arg): Set the
new boolean value according to specified options.
* src/install.c (struct cp_options): Initialize the new boolean.
* src/mv.c (struct cp_options): Initialize the new boolean.
* tests/cp/preserve-mode.sh: Add a new test.
* tests/cp/link-preserve.sh (-a --no-preserve=mode): Adjust the
expected perms: now, --no-preserve=mode overrides the --preserve=mode
that is inherent in -a, as it should.
* tests/local.mk: Add the new test to the list.

5 files changed, 16 insertions, 0 deletions

diff --git a/src/copy.c b/src/copy.c
index 2558fea14..16aed036d 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1151,6 +1151,11 @@ preserve_metadata:
       if (set_acl (dst_name, dest_desc, x->mode) != 0)
         return_val = false;
     }
+  else if (x->explicit_no_preserve_mode)
+    {
+      set_acl (dst_name, dest_desc, 0666 & ~cached_umask ());
+      return_val = false;
+    }
   else if (omitted_permissions)
     {
       omitted_permissions &= ~ cached_umask ();
@@ -2570,6 +2575,11 @@ copy_internal (char const *src_name, char const *dst_name,
       if (set_acl (dst_name, -1, x->mode) != 0)
         return false;
     }
+  else if (x->explicit_no_preserve_mode)
+    {
+      if (set_acl (dst_name, -1, 0777 & ~cached_umask ()) != 0)
+        return false;
+    }
   else
     {
       if (omitted_permissions)
diff --git a/src/copy.h b/src/copy.h
index d70c09ea8..440d3bb24 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -157,6 +157,7 @@ struct cp_options
   bool preserve_ownership;
   bool preserve_mode;
   bool preserve_timestamps;
+  bool explicit_no_preserve_mode;
 
   /* Enabled for mv, and for cp by the --preserve=links option.
      If true, attempt to preserve in the destination files any
diff --git a/src/cp.c b/src/cp.c
index 6649af2e9..61b31afdc 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -783,6 +783,7 @@ cp_option_init (struct cp_options *x)
   x->preserve_links = false;
   x->preserve_mode = false;
   x->preserve_timestamps = false;
+  x->explicit_no_preserve_mode = false;
   x->preserve_security_context = false;
   x->require_preserve_context = false;
   x->preserve_xattr = false;
@@ -860,6 +861,7 @@ decode_preserve_arg (char const *arg, struct cp_options *x, bool on_off)
         {
         case PRESERVE_MODE:
           x->preserve_mode = on_off;
+          x->explicit_no_preserve_mode = !on_off;
           break;
 
         case PRESERVE_TIMESTAMPS:
@@ -889,6 +891,7 @@ decode_preserve_arg (char const *arg, struct cp_options *x, bool on_off)
           x->preserve_timestamps = on_off;
           x->preserve_ownership = on_off;
           x->preserve_links = on_off;
+          x->explicit_no_preserve_mode = !on_off;
           if (selinux_enabled)
             x->preserve_security_context = on_off;
           x->preserve_xattr = on_off;
diff --git a/src/install.c b/src/install.c
index 854436aa3..8ea549135 100644
--- a/src/install.c
+++ b/src/install.c
@@ -275,6 +275,7 @@ cp_option_init (struct cp_options *x)
   x->preserve_links = false;
   x->preserve_mode = false;
   x->preserve_timestamps = false;
+  x->explicit_no_preserve_mode = false;
   x->reduce_diagnostics=false;
   x->data_copy_required = true;
   x->require_preserve = false;
diff --git a/src/mv.c b/src/mv.c
index 4f5708eb2..5b08fdd58 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -118,6 +118,7 @@ cp_option_init (struct cp_options *x)
   x->preserve_links = true;
   x->preserve_mode = true;
   x->preserve_timestamps = true;
+  x->explicit_no_preserve_mode= false;
   x->preserve_security_context = selinux_enabled;
   x->reduce_diagnostics = false;
   x->data_copy_required = true;