Details
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 45082 Build 41970: arc lint + arc unit
Event Timeline
I agree with doing the refactor, I just worry about the name. A timespec can legitimately have a valid tv_sec that is < 0 if it is representing a wall-time before the epoch (so a negative time_t value). OTOH, these checks here are for timespecs that represent an interval relative rather than an absolute wall time and so requiring tv_sec to be non-negative makes sense. I worry that timespecvalid is too generic of a name as it would fail on valid wall-time structures and that the name would tempt people to use it when they shouldn't. (And I wonder in fact if the checks in compat/linux are too strict, e.g. if it would both an attempt to set a time on a file via utimes(2) or the like to a time before Jan 1, 1970.)
I'd recommend 'valid_interval' instead of timespecvalid or something similar. All instances in this patch are for intervals (relative time) rather than an absolute time (which is just a relative time to 1970, but semantically different and the term 'abs time' or 'absolute time' is used to refer to this sort of time).
Otherwise it looks good.
Yeah, agreed, renamed to timespecvalid_interval. Thanks!
compat/linux utimes does not uses this check and allows negative sec values.