bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

modifying cp, etc. to prefer fchown to chown and fchmod to chmod


From: Paul Eggert
Subject: modifying cp, etc. to prefer fchown to chown and fchmod to chmod
Date: Sat, 24 Sep 2005 20:12:04 -0700
User-agent: Gnus/5.1007 (Gnus v5.10.7) Emacs/21.4 (gnu/linux)

I installed this patch so that 'cp -p' prefers to use fchown to chown,
and fchmod to chmod, when copying regular files.  The f* versions
ought to be a bit faster since they needn't resolve file names.

I left a FIXME for preserving st_author, as I don't know how to
preserve that on the Hurd.  Perhaps a Hurd expert can fill that part
in.  In the mean time the existing code should still work on the Hurd.

2005-09-24  Paul Eggert  <address@hidden>

        * m4/jm-macros.m4 (gl_MACROS): Check for fchmod and fchown.
        * src/copy.c (HAVE_FCHMOD, HAVE_FCHOWN): Define to 0 if not defined.
        (copy_reg): New args CHOWN_SUCCEDED and DST_SB.  All callers changed.
        Add a "goto close_src_and_dst_desc;" that was missing in the
        previous patch.
        (copy_reg) [HAVE_FCHOWN]: Prefer fchown to chown.
        (copy_reg) [HAVE_FCHMOD]: Prefer fchmod to chmod.
        (copy_internal): Don't invoke chown if fchown worked,
        and likewise for chmod and fchmod.

Index: m4/jm-macros.m4
===================================================================
RCS file: /fetish/cu/m4/jm-macros.m4,v
retrieving revision 1.231
diff -p -u -r1.231 jm-macros.m4
--- m4/jm-macros.m4     23 Sep 2005 20:35:14 -0000      1.231
+++ m4/jm-macros.m4     24 Sep 2005 23:27:41 -0000
@@ -102,6 +102,8 @@ AC_DEFUN([gl_MACROS],
   AC_CHECK_FUNCS_ONCE( \
     endgrent \
     endpwent \
+    fchmod \
+    fchown \
     ftruncate \
     hasmntopt \
     isascii \
Index: src/copy.c
===================================================================
RCS file: /fetish/cu/src/copy.c,v
retrieving revision 1.189
diff -p -u -r1.189 copy.c
--- src/copy.c  23 Sep 2005 20:38:34 -0000      1.189
+++ src/copy.c  24 Sep 2005 23:27:41 -0000
@@ -51,6 +51,13 @@
 #include "xreadlink.h"
 #include "yesno.h"
 
+#ifndef HAVE_FCHMOD
+# define HAVE_FCHMOD false
+#endif
+#ifndef HAVE_FCHOWN
+# define HAVE_FCHOWN false
+#endif
+
 #define SAME_OWNER(A, B) ((A).st_uid == (B).st_uid)
 #define SAME_GROUP(A, B) ((A).st_gid == (B).st_gid)
 #define SAME_OWNER_AND_GROUP(A, B) (SAME_OWNER (A, B) && SAME_GROUP (A, B))
@@ -191,13 +198,16 @@ copy_dir (char const *src_name_in, char 
    Use DST_MODE as the 3rd argument in the call to open.
    X provides many option settings.
    Return true if successful.
-   *NEW_DST is as in copy_internal.  SRC_SB is the result
-   of calling xstat (aka stat in this case) on SRC_NAME.  */
+   *NEW_DST and *CHOWN_SUCCEEDED are as in copy_internal.
+   SRC_SB and DST_SB are the results of calling XSTAT (aka stat for
+   SRC_SB) on SRC_NAME and DST_NAME.  */
 
 static bool
 copy_reg (char const *src_name, char const *dst_name,
          const struct cp_options *x, mode_t dst_mode, bool *new_dst,
-         struct stat const *src_sb)
+         bool *chown_succeeded,
+         struct stat const *src_sb,
+         struct stat const *dst_sb)
 {
   char *buf;
   size_t buf_size;
@@ -400,6 +410,7 @@ copy_reg (char const *src_name, char con
        {
          error (0, errno, _("writing %s"), quote (dst_name));
          return_val = false;
+         goto close_src_and_dst_desc;
        }
     }
 
@@ -413,10 +424,57 @@ copy_reg (char const *src_name, char con
        {
          error (0, errno, _("preserving times for %s"), quote (dst_name));
          if (x->require_preserve)
-           return_val = false;
+           {
+             return_val = false;
+             goto close_src_and_dst_desc;
+           }
        }
     }
 
+#if HAVE_FCHOWN
+  if (x->preserve_ownership
+      && (*new_dst || !SAME_OWNER_AND_GROUP (*src_sb, *dst_sb)))
+    {
+      if (fchown (dest_desc, src_sb->st_uid, src_sb->st_gid) == 0)
+       *chown_succeeded = true;
+      else if (! chown_failure_ok (x))
+       {
+         error (0, errno, _("failed to preserve ownership for %s"),
+                quote (dst_name));
+         if (x->require_preserve)
+           {
+             return_val = false;
+             goto close_src_and_dst_desc;
+           }
+       }
+    }
+#endif
+
+#if HAVE_STRUCT_STAT_ST_AUTHOR
+  /* FIXME: Preserve the st_author field via the file descriptor dest_desc.  */
+#endif
+
+#if HAVE_FCHMOD
+  /* Permissions of newly-created regular files were set upon `open'.
+     But don't return early if there were any special bits and chown
+     succeeded, because the chown must have reset those bits.  */
+  if (!(*new_dst
+       && !(*chown_succeeded && (src_sb->st_mode & ~S_IRWXUGO)))
+      && (x->preserve_mode || *new_dst)
+      && (x->copy_as_regular || S_ISREG (src_sb->st_mode)))
+    {
+      if (fchmod (dest_desc, get_dest_mode (x, src_sb->st_mode)) != 0)
+       {
+         error (0, errno, _("setting permissions for %s"), quote (dst_name));
+         if (x->set_mode || x->require_preserve)
+           {
+             return_val = false;
+             goto close_src_and_dst_desc;
+           }
+       }
+    }
+#endif
+
 close_src_and_dst_desc:
   if (close (dest_desc) < 0)
     {
@@ -1458,15 +1516,15 @@ copy_internal (char const *src_name, cha
        }
     }
   else if (S_ISREG (src_type)
-          || (x->copy_as_regular && !S_ISDIR (src_type)
-              && !S_ISLNK (src_type)))
+          || (x->copy_as_regular && !S_ISLNK (src_type)))
     {
       copied_as_regular = true;
       /* POSIX says the permission bits of the source file must be
         used as the 3rd argument in the open call, but that's not consistent
         with historical practice.  */
       if (! copy_reg (src_name, dst_name, x,
-                     get_dest_mode (x, src_mode), &new_dst, &src_sb))
+                     get_dest_mode (x, src_mode), &new_dst, &chown_succeeded,
+                     &src_sb, &dst_sb))
        goto un_backup;
     }
   else
@@ -1593,7 +1651,7 @@ copy_internal (char const *src_name, cha
     }
 
   /* Avoid calling chown if we know it's not necessary.  */
-  if (x->preserve_ownership
+  if (!(copied_as_regular && HAVE_FCHOWN) && x->preserve_ownership
       && (new_dst || !SAME_OWNER_AND_GROUP (src_sb, dst_sb)))
     {
       if (chown (dst_name, src_sb.st_uid, src_sb.st_gid) == 0)
@@ -1624,11 +1682,12 @@ copy_internal (char const *src_name, cha
   }
 #endif
 
-  /* Permissions of newly-created regular files were set upon `open' in
-     copy_reg.  But don't return early if there were any special bits and
+  /* Permissions of newly-created regular files are set by open and/or fchmod
+     in copy_reg.  But don't return early if there were any special bits and
      chown succeeded, because the chown must have reset those bits.  */
-  if ((new_dst && copied_as_regular)
-      && !(chown_succeeded && (src_mode & ~S_IRWXUGO)))
+  if (copied_as_regular
+      && (HAVE_FCHMOD
+         || (new_dst && !(chown_succeeded && (src_mode & ~S_IRWXUGO)))))
     return delayed_ok;
 
   if ((x->preserve_mode || new_dst)




reply via email to

[Prev in Thread] Current Thread [Next in Thread]