Re: modify chmod

[Jim Meyering]

jeff.liu wrote:
> It looks the "Modify chmod" task has been in TODO list for a long time.
>
> I have looked through the discussion thread referred to
> http://bugs.debian.org/497514.
>
> Does it make sense to make chmod do not change the inode's ctime
> if the new permission bits is identical to the old as the default behavior?
>
> I'd like to handle it if it is still a valid task and nobody is working
> on for now.

Hi Jeff,

Thanks for bringing that up.
I refreshed my memory, wrote a tiny patch
and tested the result.  Running as non-root,
must "chmod u+x /" fail?  With coreutils-8.4, it does:

    $ chmod u+x /
    chmod: changing permissions of `/': Operation not permitted

With the patch below, it skips the chmodat call altogether, because
chmod notices that "u+x" would not modify the existing permissions:

    $ ./chmod u+x /
    $

If we were to insist that the modified chmod
produce the same diagnostic, we *could* try.
But we'd have to compare effective uid to stat.st_uid, ...
and what about the possibility of ACLs?  So I think that is
not an option.

This new behavior could be useful...
maybe noticeably more efficient in some cases, too.
Perhaps worth a new option, if not the default.

Opinions?

diff --git a/src/chmod.c b/src/chmod.c
index 3dab92e..838ba2f 100644
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -258,16 +258,24 @@ process_file (FTS *fts, FTSENT *ent)
       new_mode = mode_adjust (old_mode, S_ISDIR (old_mode) != 0, umask_value,
                               change, NULL);

-      if (! S_ISLNK (old_mode))
+      if ((old_mode & CHMOD_MODE_BITS) == new_mode)
         {
-          if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
-            chmod_succeeded = true;
-          else
+          /* Perm+special bits are the same, so skip the chmod altogether.  */
+          chmod_succeeded = true;
+        }
+      else
+        {
+          if (! S_ISLNK (old_mode))
             {
-              if (! force_silent)
-                error (0, errno, _("changing permissions of %s"),
-                       quote (file_full_name));
-              ok = false;
+              if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
+                chmod_succeeded = true;
+              else
+                {
+                  if (! force_silent)
+                    error (0, errno, _("changing permissions of %s"),
+                           quote (file_full_name));
+                  ok = false;
+                }
             }
         }
     }