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.

diff --git a/ChangeLog b/ChangeLog
index 7f741fbe66961329145733b7742040f308bd5c31..d51b10411e01a265937128a99d8d81c30ed685bf 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 2006-09-18  Paul Eggert  <eggert@cs.ucla.edu>
 
+       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.
+
        * src/shuf.c (main): Quote the entire range when reporting an
        invalid one, rather than just the part that contained the error.
 

diff --git a/src/chmod.c b/src/chmod.c
index 29611366c9f39378c4350fbf1ebe72ba4cab23d3..eb1cfe7a0d1123a7289c2c2aa16509d6cbe46964 100644 (file)
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -85,6 +85,10 @@ static enum Verbosity verbosity = V_off;
    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 @@ process_file (FTS *fts, FTSENT *ent)
   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 @@ process_files (char **files, int bit_flags)
       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;
 }

diff --git a/src/chown-core.c b/src/chown-core.c
index 39cb34d8b38713e9920c8a22662efd72d2b54172..b5b0f3b4211b52f0eaa99c0a6f4f4f146e48f833 100644 (file)
--- a/src/chown-core.c
+++ b/src/chown-core.c
@@ -51,6 +51,10 @@ enum RCH_status
     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 @@ change_file_owner (FTS *fts, FTSENT *ent,
   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 @@ change_file_owner (FTS *fts, FTSENT *ent,
    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 @@ chown_files (char **files, int bit_flags,
                    ? 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 @@ chown_files (char **files, int bit_flags,
                               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;
 }

diff --git a/tests/install/basic-1 b/tests/install/basic-1
index bfddd6ef878800fa7ba69f531e900930bb4567e2..9c7d106ad8c45e218263b70789b91a55c3dd8989 100755 (executable)
--- a/tests/install/basic-1
+++ b/tests/install/basic-1
@@ -112,8 +112,7 @@ test -d xx/rel && fail=1
 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