[Please, please, please always CC linux-fsdevel on VFS related work.
It's even in MAINTAINERS now]
> --- linux-2.6.25-rc6-orig/fs/utimes.c 2008-04-07 22:25:08.000000000 +0200
> +++ linux-2.6.25-rc6/fs/utimes.c 2008-04-07 23:57:41.000000000 +0200
> @@ -95,27 +95,37 @@
>
> /* Don't worry, the checks are done in inode_change_ok() */
> newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
> - if (times) {
> + if (times && ! ((times[0].tv_nsec == UTIME_NOW &&
> + times[1].tv_nsec == UTIME_NOW) ||
> + (times[0].tv_nsec == UTIME_OMIT &&
> + times[1].tv_nsec == UTIME_OMIT))) {
> error = -EPERM;
> if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
> goto dput_and_out;
>
> - if (times[0].tv_nsec == UTIME_OMIT)
> - newattrs.ia_valid &= ~ATTR_ATIME;
> - else if (times[0].tv_nsec != UTIME_NOW) {
> + if (times[0].tv_nsec == UTIME_OMIT) {
> + newattrs.ia_atime = inode->i_atime;
> + newattrs.ia_valid |= ATTR_ATIME_SET;
This seems wrong. Why exactly was this change made?
Setting the time to inode->i_atime is *not* the same as not setting
the time. For some filesystems i_atime is just a cached value, that
may or may not match the actual last access time. For those
filesystems this could have very strange effects.
> + } else if (times[0].tv_nsec != UTIME_NOW) {
> newattrs.ia_atime.tv_sec = times[0].tv_sec;
> newattrs.ia_atime.tv_nsec = times[0].tv_nsec;
> newattrs.ia_valid |= ATTR_ATIME_SET;
> }
>
> - if (times[1].tv_nsec == UTIME_OMIT)
> - newattrs.ia_valid &= ~ATTR_MTIME;
> - else if (times[1].tv_nsec != UTIME_NOW) {
> + if (times[1].tv_nsec == UTIME_OMIT) {
> + newattrs.ia_mtime = inode->i_mtime;
> + newattrs.ia_valid |= ATTR_MTIME_SET;
Ditto.
> + } else if (times[1].tv_nsec != UTIME_NOW) {
> newattrs.ia_mtime.tv_sec = times[1].tv_sec;
> newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
> newattrs.ia_valid |= ATTR_MTIME_SET;
> }
> +
> + } else if (unlikely(times && times[0].tv_nsec == UTIME_OMIT &&
> + times[1].tv_nsec == UTIME_OMIT)) {
> + newattrs.ia_valid &= ~(ATTR_ATIME | ATTR_MTIME | ATTR_CTIME);
So this is a no-op? In which case this check could be moved to the
top of the function to just return zero.
Miklos
> } else {
> + /* times is NULL, or both tv_nsec fields are UTIME_NOW */
> error = -EACCES;
> if (IS_IMMUTABLE(inode))
> goto dput_and_out;
> @@ -150,14 +160,6 @@
> if (utimes) {
> if (copy_from_user(&tstimes, utimes, sizeof(tstimes)))
> return -EFAULT;
> - if ((tstimes[0].tv_nsec == UTIME_OMIT ||
> - tstimes[0].tv_nsec == UTIME_NOW) &&
> - tstimes[0].tv_sec != 0)
> - return -EINVAL;
> - if ((tstimes[1].tv_nsec == UTIME_OMIT ||
> - tstimes[1].tv_nsec == UTIME_NOW) &&
> - tstimes[1].tv_sec != 0)
> - return -EINVAL;
>
> /* Nothing to do, we must not even check the path. */
> if (tstimes[0].tv_nsec == UTIME_OMIT &&
>
>
> ==================================================
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
