Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SystemTime conversions invite programmer error #52522

Open
ssokolow opened this Issue Jul 19, 2018 · 1 comment

Comments

Projects
None yet
3 participants
@ssokolow
Copy link

ssokolow commented Jul 19, 2018

Yesterday, I found and reported a panic bug in serde_json which traces its origins to the mechanism for extracting usable information from SystemTime.

Specifically, using the best method they believed to be available, it would panic when I tried to serialize metadata for files with negative mtime values. (I have one file with an mtime of -1 and another with an mtime so far in the future that it overflows into a large negative number if the Rust code is compiled for the i686-unknown-linux-musl target.

The current design of the API encourages users to assume that sources of SystemTime values use unsigned values internally (ie. that modifications to the system clock are the only source of Err with valid input), and, further more, in the bug I reported for serde_json, it led @lambda to the incorrect conclusion that there is currently no way to losslessly serialize and deserialize SystemTime values.

If nothing else, the documentation should be improved in the following ways:

  1. The documentation for both UNIX_EPOCH definitions should explicitly mention that the underlying OS may return valid SystemTime values prior to the epoch and code should be prepared for such an eventuality. (Perhaps with the concrete example of ext3 filesystems allowing negative mtime values to be stored)

  2. The documentation for SystemTime::duration_since should use "may return SystemTimeError" rather than "may fail" to avoid the connotation that an Err result is erroneous in any sense other than being an "until" rather than a "since".

    That is, "failure" connotes a response that should prompt either retry or error-propagation, while it is not necessary to re-run the operation with the arguments reversed in order to extract the Duration and, depending on the input, nothing may be wrong at all. (ie. the Result discriminant is semantically equivalent to a sign bit in this case.)

  3. "and the error contains how far from self the time is" is too easy to overlook, given how important it is for robust software to handle this eventuality. It should explicitly mention SystemTimeError::duration to make it more eye-catching.

    Perhaps "Returns an Err if earlier is later than self. A Duration representing how far self is before earlier can be retrieved via the duration() function on the resulting SystemTimeError.

There are probably other ways to better tune the writing to guard against human error, but I don't want to be here all night trying to perfect this when you might think of the same improvements immediately, simply by virtue of having a different perspective.

@retep998

This comment has been minimized.

Copy link
Member

retep998 commented Jul 26, 2018

Another concrete example of valid SystemTime values prior to the unix epoch is Windows whose epoch is well before the unix epoch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.