Adjust chmod and chown to be similar if -c or -v are given. In

particular, a no-op chown is no longer reported as a change; this
reverts to previous behavior.  Also, fix both commands so that -v
report failures even if the failure is not due to the chmod or
chown syscalls.

* src/chown-core.c (change_file_owner): Return -1 on error, not
1 sometimes and -1 on others.  Our caller ORs together our results,
and (-1 | 1) == 0 on ones-complement hosts.
With -v report all errors verbosely, not just some.
Fix bug when chopt->root_dev_ino && !chopt->affect_symlink_referent:
file_stats wasn't set properly in that case.

diff --git a/src/chown-core.c b/src/chown-core.c
index b43e1d91dc9be7817b4be159f3e5caa98c22a3b7..2811ff92e4521dd13f2fbe2fb06e01d6b5356d9c 100644 (file)
--- a/src/chown-core.c
+++ b/src/chown-core.c
@@ -167,18 +167,19 @@ describe_change (const char *file, enum Change_status changed,
    to UID and/or GID as appropriate.
    FIXME: describe old_uid and old_gid.
    CHOPT specifies additional options.
-   Return nonzero upon error, zero otherwise.  */
+   Return 0 if successful, -1 if errors occurred.  */
 static int
 change_file_owner (FTS *fts, FTSENT *ent,
                   uid_t uid, gid_t gid,
                   uid_t old_uid, gid_t old_gid,
                   struct Chown_option const *chopt)
 {
-  const char *file_full_name = ent->fts_path;
+  char const *file_full_name = ent->fts_path;
+  char const *file = ent->fts_accpath;
   struct stat const *file_stats IF_LINT (= NULL);
   struct stat stat_buf;
   int errors = 0;
-  bool do_chown = true;
+  bool do_chown;
   bool symlink_changed = true;
 
   switch (ent->fts_info)
@@ -195,23 +196,30 @@ change_file_owner (FTS *fts, FTSENT *ent,
 
     case FTS_NS:
       error (0, ent->fts_errno, _("cannot access %s"), quote (file_full_name));
-      return 1;
+      errors = -1;
+      break;
 
     case FTS_ERR:
       error (0, ent->fts_errno, _("%s"), quote (file_full_name));
-      return 1;
+      errors = -1;
+      break;
 
     case FTS_DNR:
       error (0, ent->fts_errno, _("cannot read directory %s"),
             quote (file_full_name));
-      return 1;
+      errors = -1;
+      break;
 
     default:
       break;
     }
 
-  if (old_uid != (uid_t) -1 || old_gid != (gid_t) -1
-      || (chopt->root_dev_ino && chopt->affect_symlink_referent))
+  if (errors)
+    do_chown = false;
+  else if (old_uid == (uid_t) -1 && old_gid == (gid_t) -1
+          && chopt->verbosity == V_off && ! chopt->root_dev_ino)
+    do_chown = true;
+  else
     {
       file_stats = ent->fts_statp;
 
@@ -219,32 +227,30 @@ change_file_owner (FTS *fts, FTSENT *ent,
         stat it to get the permissions of the referent.  */
       if (S_ISLNK (file_stats->st_mode) && chopt->affect_symlink_referent)
        {
-         if (stat (ent->fts_accpath, &stat_buf) != 0)
+         if (stat (file, &stat_buf) != 0)
            {
              error (0, errno, _("cannot dereference %s"),
                     quote (file_full_name));
-             return 1;
+             errors = -1;
            }
 
          file_stats = &stat_buf;
        }
 
-      do_chown = ((old_uid == (uid_t) -1
-                  || file_stats->st_uid == old_uid)
-                 && (old_gid == (gid_t) -1
-                     || file_stats->st_gid == old_gid));
+      do_chown = (!errors
+                 && (old_uid == (uid_t) -1 || old_uid == file_stats->st_uid)
+                 && (old_gid == (gid_t) -1 || old_gid == file_stats->st_gid));
     }
 
-  if (do_chown)
+  if (do_chown && ROOT_DEV_INO_CHECK (chopt->root_dev_ino, file_stats))
     {
-      const char *file = ent->fts_accpath;
-
-      if (ROOT_DEV_INO_CHECK (chopt->root_dev_ino, file_stats))
-       {
-         ROOT_DEV_INO_WARN (file_full_name);
-         return 1;
-       }
+      ROOT_DEV_INO_WARN (file_full_name);
+      errors = -1;
+      do_chown = false;
+    }
 
+  if (do_chown)
+    {
       if (chopt->affect_symlink_referent)
        {
          /* Applying chown to a symlink and expecting it to affect
@@ -280,15 +286,23 @@ change_file_owner (FTS *fts, FTSENT *ent,
               quote (file_full_name));
     }
 
-  if (chopt->verbosity == V_high
-      || (chopt->verbosity == V_changes_only && !errors))
+  if (chopt->verbosity != V_off)
     {
-      enum Change_status ch_status = (!do_chown ? CH_NO_CHANGE_REQUESTED
-                                     : !symlink_changed ? CH_NOT_APPLIED
-                                     : errors ? CH_FAILED
-                                     : CH_SUCCEEDED);
-      describe_change (file_full_name, ch_status,
-                      chopt->user_name, chopt->group_name);
+      bool changed =
+       (do_chown && !errors && symlink_changed
+        && ! ((uid == (uid_t) -1 || uid == file_stats->st_uid)
+              && (gid == (gid_t) -1 || gid == file_stats->st_gid)));
+
+      if (changed || chopt->verbosity == V_high)
+       {
+         enum Change_status ch_status =
+           (errors ? CH_FAILED
+            : !symlink_changed ? CH_NOT_APPLIED
+            : !changed ? CH_NO_CHANGE_REQUESTED
+            : CH_SUCCEEDED);
+         describe_change (file_full_name, ch_status,
+                          chopt->user_name, chopt->group_name);
+       }
     }
 
   if ( ! chopt->recurse)
@@ -315,9 +329,8 @@ chown_files (char **files, int bit_flags,
   int fail = 0;
 
   /* Use lstat and stat only if they're needed.  */
-  int stat_flags = ((chopt->root_dev_ino
-                    || required_uid != (uid_t) -1
-                    || required_gid != (gid_t) -1)
+  int stat_flags = ((required_uid != (uid_t) -1 || required_gid != (gid_t) -1
+                    || chopt->verbosity != V_off || chopt->root_dev_ino)
                    ? 0
                    : FTS_NOSTAT);