-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
Description
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:
-
The documentation for both
UNIX_EPOCHdefinitions should explicitly mention that the underlying OS may return validSystemTimevalues prior to the epoch and code should be prepared for such an eventuality. (Perhaps with the concrete example ofext3filesystems allowing negativemtimevalues to be stored) -
The documentation for
SystemTime::duration_sinceshould use "may returnSystemTimeError" rather than "may fail" to avoid the connotation that anErrresult 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
Durationand, depending on the input, nothing may be wrong at all. (ie. theResultdiscriminant is semantically equivalent to a sign bit in this case.) -
"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::durationto make it more eye-catching.Perhaps "Returns an Err if
earlieris later thanself. ADurationrepresenting how farselfis beforeearliercan be retrieved via theduration()function on the resultingSystemTimeError.
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.