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; + } } } }