On Wed, Jan 24, 2018 at 3:56 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > On Tue, Jan 23, 2018 at 5:25 PM, Deepa Dinamani <deepa.kernel@xxxxxxxxx> wrote: >> On Mon, Jan 22, 2018 at 12:25 PM, Linus Torvalds >> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: >>> On Sun, Jan 21, 2018 at 6:04 PM, Deepa Dinamani <deepa.kernel@xxxxxxxxx> wrote: >>>> + t.tv_nsec -= t.tv_nsec % gran; >>> >>> This doesn't actuall ywork if tv_nsec is negative. >> >> Right. >> >>> Which may not be an issue in most cases, but did somebody check >>> utimensat() or whatever? >> >> I checked POSIX again. There is no mention of tv_nsec being positive >> always for utimes. >> And, the long term plan is to replace all the callers of >> timespec_trunc() to use this new api instead for filesystems. >> So this will need to be fixed. I will fix this and post an update. > > I found this on > http://pubs.opengroup.org/onlinepubs/9699919799/functions/utimes.html > > ERRORS > These functions shall fail if: > ... > [EINVAL] > Either of the times argument structures specified a tv_nsec value that was > neither UTIME_NOW nor UTIME_OMIT, and was a value less than zero or > greater than or equal to 1000 million. > > which is the same as the Linux man page and what the kernel actually > does for all the syscalls. The POSIX description seems a bit ambiguous > to whether it also expects or allows EINVAL for utimes() with a tv_usec > over 1000000 microseconds, or if it just applies to the utimensat and > futimens(). Older descriptions that only explain utimes() don't mention > the range check on tv_usec either. Right. This is in keeping with the kernel implementation of the corresponding syscalls. But, this timespec_truncate() is being called from current_time() and will be extended to other calls. C99 says "When integers are divided, the result of the / operator is the algebraic quotient with any fractional part discarded (often called "truncation toward zero"). If the quotient a/b is representable, the expression (a/b)*b + a%b shall equal a." Also, we are already checking for gran being non-zero and in the nanoseconds range. So I think the right answer here is to add a comment that the function expects timespec to be normalized, and the functions calling it can take care of validation. -Deepa
