(umask_kill): With default acls, the umask is not to be applied.

Remove umask_kill, don't change the process umask, and let the kernel
apply the umask where appropriate.
(make_dir_parents_private): Fix logic for POSIX ACLs.

1 files changed, 66 insertions, 31 deletions

diff --git a/src/cp.c b/src/cp.c
index 4e08612a0..4fe344fb9 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -35,6 +35,7 @@
 #include "quotearg.h"
 #include "stat-time.h"
 #include "utimens.h"
+#include "acl.h"
 
 #define ASSIGN_BASENAME_STRDUPA(Dest, File_name)	\
   do							\
@@ -56,7 +57,8 @@
    need to be fixed after copying. */
 struct dir_attr
 {
-  bool is_new_dir;
+  mode_t mode;
+  bool restore_mode;
   size_t slash_offset;
   struct dir_attr *next;
 };
@@ -327,9 +329,14 @@ re_protect (char const *const_dst_name, size_t src_offset,
 	    }
 	}
 
-      if (x->preserve_mode | p->is_new_dir)
+      if (x->preserve_mode)
 	{
-	  if (chmod (dst_name, src_sb.st_mode & x->umask_kill))
+	  if (copy_acl (src_name, -1, dst_name, -1, src_sb.st_mode))
+	    return false;
+	}
+      else if (p->restore_mode)
+	{
+	  if (chmod (dst_name, p->mode))
 	    {
 	      error (0, errno, _("failed to preserve permissions for %s"),
 		     quote (dst_name));
@@ -347,8 +354,7 @@ re_protect (char const *const_dst_name, size_t src_offset,
 
    SRC_OFFSET is the index in CONST_DIR (which is a destination
    directory) of the beginning of the source directory name.
-   Create any leading directories that don't already exist,
-   giving them permissions MODE.
+   Create any leading directories that don't already exist.
    If VERBOSE_FMT_STRING is nonzero, use it as a printf format
    string for printing a message after successfully making a directory.
    The format should take two string arguments: the names of the
@@ -364,9 +370,9 @@ re_protect (char const *const_dst_name, size_t src_offset,
 
 static bool
 make_dir_parents_private (char const *const_dir, size_t src_offset,
-			  mode_t mode, char const *verbose_fmt_string,
+			  char const *verbose_fmt_string,
 			  struct dir_attr **attr_list, bool *new_dst,
-			  int (*xstat) ())
+			  const struct cp_options *x)
 {
   struct stat stats;
   char *dir;		/* A copy of CONST_DIR we can change.  */
@@ -385,7 +391,7 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
 
   *attr_list = NULL;
 
-  if ((*xstat) (dst_dir, &stats))
+  if (XSTAT (x, dst_dir, &stats))
     {
       /* A parent of CONST_DIR does not exist.
 	 Make all missing intermediate directories. */
@@ -400,20 +406,30 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
 	     fixing later. */
 	  struct dir_attr *new = xmalloc (sizeof *new);
 	  new->slash_offset = slash - dir;
+	  new->restore_mode = false;
 	  new->next = *attr_list;
 	  *attr_list = new;
 
 	  *slash = '\0';
-	  if ((*xstat) (dir, &stats))
+	  if (XSTAT (x, dir, &stats))
 	    {
+	      mode_t src_mode;
+
 	      /* This component does not exist.  We must set
-		 *new_dst and new->is_new_dir inside this loop because,
+		 *new_dst and new->mode inside this loop because,
 		 for example, in the command `cp --parents ../a/../b/c e_dir',
 		 make_dir_parents_private creates only e_dir/../a if
 		 ./b already exists. */
 	      *new_dst = true;
-	      new->is_new_dir = true;
-	      if (mkdir (dir, mode))
+	      if (XSTAT (x, src, &stats))
+		{
+		  error (0, errno, _("failed to get attributes of %s"),
+			 quote (src));
+		  return false;
+		}
+	      src_mode = stats.st_mode;
+
+	      if (mkdir (dir, src_mode))
 		{
 		  error (0, errno, _("cannot make directory %s"),
 			 quote (dir));
@@ -424,6 +440,41 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
 		  if (verbose_fmt_string != NULL)
 		    printf (verbose_fmt_string, src, dir);
 		}
+
+	      /* We need search and write permissions to the new directory
+	         for writing the directory's contents. Check if these
+		 permissions are there.  */
+
+	      if (lstat (dir, &stats))
+		{
+		  error (0, errno, _("failed to get attributes of %s"),
+			 quote (dir));
+		  return false;
+		}
+	      else
+	        {
+		  if (x->preserve_mode)
+		    {
+		      new->mode = src_mode;
+		      new->restore_mode = (src_mode != stats.st_mode);
+		    }
+
+		  if ((stats.st_mode & S_IRWXU) != S_IRWXU)
+		    {
+		      /* Make the new directory searchable and writable. The
+			 original permissions will be restored later.  */
+
+		      new->mode = stats.st_mode;
+		      new->restore_mode = true;
+
+		      if (chmod (dir, stats.st_mode | S_IRWXU))
+			{
+			  error (0, errno, _("setting permissions for %s"),
+				 quote (dir));
+			  return false;
+			}
+		    }
+		}
 	    }
 	  else if (!S_ISDIR (stats.st_mode))
 	    {
@@ -432,10 +483,7 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
 	      return false;
 	    }
 	  else
-	    {
-	      new->is_new_dir = false;
-	      *new_dst = false;
-	    }
+	    *new_dst = false;
 	  *slash++ = '/';
 
 	  /* Avoid unnecessary calls to `stat' when given
@@ -536,10 +584,6 @@ do_copy (int n_files, char **file, const char *target_directory,
 	 Copy the files `file1' through `filen'
 	 to the existing directory `edir'. */
       int i;
-      int (*xstat)() = (x->dereference == DEREF_COMMAND_LINE_ARGUMENTS
-			|| x->dereference == DEREF_ALWAYS
-			? stat
-			: lstat);
 
       /* Initialize these hash tables only if we'll need them.
 	 The problems they're used to detect can arise only if
@@ -585,9 +629,9 @@ do_copy (int n_files, char **file, const char *target_directory,
 	         leading directories. */
 	      parent_exists =
 		(make_dir_parents_private
-		 (dst_name, arg_in_concat - dst_name, S_IRWXU,
+		 (dst_name, arg_in_concat - dst_name,
 		  (x->verbose ? "%s -> %s\n" : NULL),
-		  &attr_list, &new_dst, xstat));
+		  &attr_list, &new_dst, x));
 	    }
 	  else
 	    {
@@ -697,12 +741,6 @@ cp_option_init (struct cp_options *x)
   /* Not used.  */
   x->stdin_tty = false;
 
-  /* Find out the current file creation mask, to knock the right bits
-     when using chmod.  The creation mask is set to be liberal, so
-     that created directories can be written, even if it would not
-     have been allowed with the mask this process was started with.  */
-  x->umask_kill = ~ umask (0);
-
   x->update = false;
   x->verbose = false;
   x->dest_info = NULL;
@@ -987,9 +1025,6 @@ main (int argc, char **argv)
 				   version_control_string)
 		   : no_backups);
 
-  if (x.preserve_mode)
-    x.umask_kill = ~ (mode_t) 0;
-
   if (x.dereference == DEREF_UNDEFINED)
     {
       if (x.recursive)