Skip to content

SystemTime conversions invite programmer error #52522

@ssokolow

Description

@ssokolow

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsC-enhancementCategory: An issue proposing an enhancement or a PR with one.T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions