Hi Eric and Coreutils Members,
I know about the risk of race condition. It would be just an option.
If you tell about
it in usage help, users will be notified about the risks and decide
according to it.
There are several usecases when this race condition is acceptable.
Without this internal logic in chmod, scripters must write their own
logic in upper level (eg. shell script: compute numeric mode from
symbolic mode notation -> stat the file -> test the current mode
against the old one -> call chmod conditionally, etc.), which means
bigger probability of race condition.
So I would state the risks of this option in help text, explaining the
meaning of '--cheap', and of course make it NOT default.
You can find the patch in attachment.
2016-05-24 16:38 GMT+02:00, Eric Blake <[email protected]>:
> On 05/24/2016 06:42 AM, Hrubák, András wrote:
>> Dear Coreutils Dev Team,
>>
>> I made a patch for chmod.
>> It introduces an option `--cheap' to avoid real chmod (fchmodat) call
>> if the new mode seems equivalent to the old one.
>> It not only saves IO time, but also leaves ctime unchanged. While
>> ctime often plays role in updating/backuping/cleaning mechanisms, I
>> needed such a feature.
>> Please see it here: https://github.com/coreutils/coreutils/pull/6
>
> Please post patches to this list, rather than making us chase pull
> requests.
>
> That said, it is unlikely that your patch will be applied, as it is
> fundamentally a TOCTTOU race - there is no race-free way to determine a
> file's current mode to know that the changed mode will not be different,
> because a concurrent operation can occur between your check and the
> (potentially-skipped) chmod; so we prefer to always do the chmod.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>
@@ -67,6 +67,9 @@ static mode_t umask_value;
/* If true, change the modes of directories recursively. */
static bool recurse;
+/* Cheap operations - do chmod(1) only if new mode would differ from old one */
+static bool cheap_ops;
+
/* If true, force silence (suppress most of error messages). */
static bool force_silent;
@@ -101,6 +104,7 @@ static struct option const long_options[] =
{"reference", required_argument, NULL, REFERENCE_FILE_OPTION},
{"silent", no_argument, NULL, 'f'},
{"verbose", no_argument, NULL, 'v'},
+ {"cheap", no_argument, NULL, 'C'},
{GETOPT_HELP_OPTION_DECL},
{GETOPT_VERSION_OPTION_DECL},
{NULL, 0, NULL, 0}
@@ -269,7 +273,7 @@ process_file (FTS *fts, FTSENT *ent)
if (! S_ISLNK (old_mode))
{
- if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
+ if ((cheap_ops && (old_mode & CHMOD_MODE_BITS) == (new_mode &
CHMOD_MODE_BITS)) || chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
chmod_succeeded = true;
else
{
@@ -396,6 +400,9 @@ With --reference, change the mode of each FILE to that of
RFILE.\n\
fputs (_("\
-R, --recursive change files and directories recursively\n\
"), stdout);
+ fputs (_("\
+ -C, --cheap do not chmod unless new mode differs from old one\n\
+"), stdout);
fputs (HELP_OPTION_DESCRIPTION, stdout);
fputs (VERSION_OPTION_DESCRIPTION, stdout);
fputs (_("\
@@ -429,10 +436,10 @@ main (int argc, char **argv)
atexit (close_stdout);
- recurse = force_silent = diagnose_surprises = false;
+ recurse = force_silent = diagnose_surprises = cheap_ops = false;
while ((c = getopt_long (argc, argv,
- ("Rcfvr::w::x::X::s::t::u::g::o::a::,::+::=::"
+ ("RcfvCr::w::x::X::s::t::u::g::o::a::,::+::=::"
"0::1::2::3::4::5::6::7::"),
long_options, NULL))
!= -1)
@@ -501,6 +508,9 @@ main (int argc, char **argv)
case 'v':
verbosity = V_high;
break;
+ case 'C':
+ cheap_ops = true;
+ break;
case_GETOPT_HELP_CHAR;
case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
default:
