Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix date parsing from TAL cachefiles
The code was writing dates in Zulu format, which was fine. But then, to read them back, it was loading them with mktime(), which is a local timezone function. The effects of this bug depend on the time zone. Files would expire from the cache up to 12 hours too early (UTC+) or late (UTC-), or be updated up to 12 hours late (UTC-). (There's no such thing as "updating too early" in this context, since Fort cannot refresh before the present.) I fixed this by swapping mktime() for timegm(), which is not great, because the latter is still a nonstandard function. But it'll have to do, because the other options are worse. Below is my thought process on timegm() and the other options: ================================================== Problem: 1. I want to store timestamps as human-readable strings. 2. Converting a timestamp to Zulu string is trivial: time_t -> gmtime_r() -> struct tm (GMT) -> strftime() -> char*. 3. But converting a string to timestamp relies on timegm(), which is nonstandard: char* -> strptime() -> struct tm (GMT) -> timegm() -> time_t. Brainstorm: 1. Store the dates in local time. Hideous option, but not ENTIRELY insane. Storing in local time will render the dates up to 24 hours off, I think. But this only happens when the cache changes timezone, which isn't really often. But it's still pretty clumsy, and also not future-proof, as new date fields would have to be constrained by the same limitation. 2/10 at best. 2. Ditch time_t, use struct tm in UTC for everything. So I would rely on gmtime_r() only to get out of time_t, and never need timegm() for anything. Comparing field by field would be a pain, but it's interesting to note that Fort is actually already doing it somewhere: tm_cmp(). I guess I have to admire the audaciousness of past me. What mainly scares me is that mktime() seems to be the only standard function that is contractually obligated to normalize the tm, which means I would have to keep mktime()ing every time I need to compare them. And mktime() is... a local time function. Probably wouldn't actually work. 4/10. I hate this API. 3. Bite the bullet: Go modern POSIX; assume time_t is integer seconds since the 1970 epoch. I'm fantasizing about an early environment assertion that checks the gmtime_r() for a known time_t, that shows people the door if the implementation isn't sane. Would work even in POSIX-adjacent systems like most Linuxes. This would have other benefits, like the ability to ditch difftime(), and perform arithmetic operations on time_t's. All the officially supported platforms get this aspect of POSIX right, so what's stopping me? Well, it would mean I would have to store the date as a seconds-since- epoch integer, which is not as human-friendly and defeats the whole point of the JSON format. And yet... this feels so right, I might end up doing it even regardless of the JSON data type and timegm(). But not today. 7/10. 4. Bite the bullet: Use timegm(). Interesting. timegm() might be added to C23: > Changes integrated into the latest working draft of C23 are listed > below. > (...) > - Add timegm() function in <time.h> to convert time structure into > calendar time value - similar to function in glibc and musl libraries. https://en.wikipedia.org/wiki/C23_(C_standard_revision) So this has some probability of not needing future tweaking. Also, it's very clean. (Except for the feature test macros.) 8/10.
- Loading branch information
