"chmod a+rwx A B" doesn't chmod A before chmoding B

[Paul Eggert]

While testing the new mkdir -p stuff I noticed a bug in chmod:
it doesn't operate left-to-right as tradition and POSIX require:

  $ mkdir d d/e
  $ chmod 0 d/e d
  $ chmod a+rwx d d/e
  chmod: cannot access `d/e': Permission denied

The last chmod should succeed, but it fails.

I installed the following patch to fix this and test for the
bug.

I suppose another option would be to modify fts so that it
has a new flag saying "please do everything left-to-right",
but I suspect that'd be more complicated.

After installing this I noticed the following line in NEWS:

  fts no longer changes the current working directory, so its clients
  (chmod, chown, chgrp, du) no longer malfunction under extreme conditions.

so I guess the following patch is overly conservative, as it
checks for fts_close failing?  If you like, I can remove
these new checks, and this will simplify the change quite a
bit; but it seems to me that some of the code in fts.c
(e.g., fts_close returning -1) could also be removed.


2006-09-18  Paul Eggert  <address@hidden>

        Fix bug where chmod, chown, and chgrp did not process operands
        left-to-right in some cases.
        * src/chmod.c (wd_errno): New var.
        (chmod_file): New function, with most of the contents of the
        old prcess_file function.
        (process_files): Use it.  This gives file names to fts one
        at a time, so that they are processed left-to-right as POSIX
        requires.
        * src/chown-core.c (wd_errno, chown_files): Likewise.
        (chown_file): New function.
        * tests/install/basic-1: Redo test so as to not workaround
        the chmod bug, thereby testing for it.

Index: src/chmod.c
===================================================================
RCS file: /fetish/cu/src/chmod.c,v
retrieving revision 1.118
diff -u -r1.118 chmod.c
--- src/chmod.c 3 Sep 2006 02:53:16 -0000       1.118
+++ src/chmod.c 18 Sep 2006 22:08:58 -0000
@@ -85,6 +85,10 @@
    Otherwise NULL.  */
 static struct dev_ino *root_dev_ino;

+/* Error number associated with the working directory, or 0 if no
+   error has been found.  */
+static int wd_errno;
+
 /* For long options that have no equivalent short option, use a
    non-character as a pseudo short option, starting with CHAR_MAX + 1.  */
 enum
@@ -279,16 +283,19 @@
   return ok;
 }

-/* Recursively change the modes of the specified FILES (the last entry
-   of which is NULL).  BIT_FLAGS controls how fts works.
+/* Recursively change the modes of the command-line operand FILE.
+   BIT_FLAGS controls how fts works.
    Return true if successful.  */

 static bool
-process_files (char **files, int bit_flags)
+chmod_file (char *file, int bit_flags)
 {
+  char *files[2];
   bool ok = true;
-
-  FTS *fts = xfts_open (files, bit_flags, NULL);
+  FTS *fts;
+  files[0] = file;
+  files[1] = NULL;
+  fts = xfts_open (files, bit_flags, NULL);

   while (1)
     {
@@ -309,10 +316,31 @@
       ok &= process_file (fts, ent);
     }

-  /* Ignore failure, since the only way it can do so is in failing to
-     return to the original directory, and since we're about to exit,
-     that doesn't matter.  */
-  fts_close (fts);
+  if (fts_close (fts) != 0)
+    wd_errno = errno;
+
+  return ok;
+}
+
+/* Recursively change the modes of the specified FILES (the last entry
+   of which is NULL).  BIT_FLAGS controls how fts works.
+   Return true if successful.  */
+static bool
+process_files (char **files, int bit_flags)
+{
+  bool ok = true;
+  wd_errno = 0;
+
+  for (; *files; files++)
+    {
+      if (! IS_ABSOLUTE_FILE_NAME (*files) && wd_errno)
+       {
+         error (0, wd_errno, ".");
+         ok = false;
+       }
+      else
+       ok &= chmod_file (*files, bit_flags);
+    }

   return ok;
 }
Index: src/chown-core.c
===================================================================
RCS file: /fetish/cu/src/chown-core.c,v
retrieving revision 1.40
diff -u -r1.40 chown-core.c
--- src/chown-core.c    17 Jan 2006 17:26:15 -0000      1.40
+++ src/chown-core.c    18 Sep 2006 22:08:58 -0000
@@ -51,6 +51,10 @@
     RC_error
   };

+/* Error number associated with the working directory, or 0 if no
+   error has been found.  */
+static int wd_errno;
+
 extern void
 chopt_init (struct Chown_option *chopt)
 {
@@ -422,7 +426,7 @@
   return ok;
 }

-/* Change the owner and/or group of the specified FILES.
+/* Change the owner and/or group of the specified FILE.
    BIT_FLAGS specifies how to treat each symlink-to-directory
    that is encountered during a recursive traversal.
    CHOPT specifies additional options.
@@ -431,11 +435,11 @@
    If REQUIRED_UID and/or REQUIRED_GID is not -1, then change only
    files with user ID and group ID that match the non-(-1) value(s).
    Return true if successful.  */
-extern bool
-chown_files (char **files, int bit_flags,
-            uid_t uid, gid_t gid,
-            uid_t required_uid, gid_t required_gid,
-            struct Chown_option const *chopt)
+static bool
+chown_file (char *file, int bit_flags,
+           uid_t uid, gid_t gid,
+           uid_t required_uid, gid_t required_gid,
+           struct Chown_option const *chopt)
 {
   bool ok = true;

@@ -445,7 +449,11 @@
                    ? 0
                    : FTS_NOSTAT);

-  FTS *fts = xfts_open (files, bit_flags | stat_flags, NULL);
+  FTS *fts;
+  char *files[2];
+  files[0] = file;
+  files[1] = NULL;
+  fts = xfts_open (files, bit_flags | stat_flags, NULL);

   while (1)
     {
@@ -467,10 +475,40 @@
                               required_uid, required_gid, chopt);
     }

-  /* Ignore failure, since the only way it can do so is in failing to
-     return to the original directory, and since we're about to exit,
-     that doesn't matter.  */
-  fts_close (fts);
+  if (fts_close (fts) != 0)
+    wd_errno = errno;
+
+  return ok;
+}
+
+/* Change the owner and/or group of the specified FILES.
+   BIT_FLAGS specifies how to treat each symlink-to-directory
+   that is encountered during a recursive traversal.
+   CHOPT specifies additional options.
+   If UID is not -1, then change the owner id of each file to UID.
+   If GID is not -1, then change the group id of each file to GID.
+   If REQUIRED_UID and/or REQUIRED_GID is not -1, then change only
+   files with user ID and group ID that match the non-(-1) value(s).
+   Return true if successful.  */
+extern bool
+chown_files (char **files, int bit_flags,
+            uid_t uid, gid_t gid,
+            uid_t required_uid, gid_t required_gid,
+            struct Chown_option const *chopt)
+{
+  bool ok = true;
+
+  for (; *files; files++)
+    {
+      if (! IS_ABSOLUTE_FILE_NAME (*files) && wd_errno)
+       {
+         error (0, wd_errno, ".");
+         ok = false;
+       }
+      else
+       ok &= chown_file (*files, bit_flags, uid, gid,
+                         required_uid, required_gid, chopt);
+    }

   return ok;
 }
Index: tests/install/basic-1
===================================================================
RCS file: /fetish/cu/tests/install/basic-1,v
retrieving revision 1.20
diff -u -r1.20 basic-1
--- tests/install/basic-1       16 Sep 2006 20:03:57 -0000      1.20
+++ tests/install/basic-1       18 Sep 2006 22:08:58 -0000
@@ -112,8 +112,7 @@
 mkdir -p sub1/d || fail=1
 (cd sub1/d && chmod a-rx .. && chmod a-r . &&
  ginstall -d $abs/xx/zz rel/a rel/b 2> /dev/null) || fail=1
-chmod 755 sub1 || fail=1
-chmod 755 sub1/d || fail=1
+chmod 755 sub1 sub1/d || fail=1
 test -d xx/zz || fail=1
 test -d sub1/d/rel/a || fail=1
 test -d sub1/d/rel/b || fail=1