[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 05 Dec 2013 16:31:38 +0000
From: Laurent Bercot <ska-dietlibc@...rnet.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCHv2] Add support for leap seconds in zoneinfo files
Eh, that's fine. I have workarounds for libcs that don't support
leap seconds. As long as that support is going to make it into musl
*eventually*, I'm not disappointed. So, let's discuss.
> - gmtime depends on the TZ variable
Yes.
Functions that display human time need to derive it from the system
time, so they need to know what the system time is. POSIX sidesteps
the problem by saying "the system time will be aligned with some
human time (UTC)"; but if you do not want the system time to be
aligned with UTC, then you have to make those functions aware of what
the system time is.
gmtime() cannot be the same function when your system time is UTC
and when it is TAI-10: it needs to be given that information.
Since zoneinfo files include that very information - the leap second
table - then I figured it made sense to use them: the timezone tells
gmtime() how to interpret the system time. Timezones are used not
only as a UTC <--> local time information provider, but also as a
UTC <--> system clock time information provider.
Note that TZ values pointing to POSIX timezone definitions will never
change gmtime()'s behaviour.
I am not fundamentally opposed to conveying the information in
another way, as long as it can be done consistently.
> - gmtime behaves unpredictably depending on whether a function that
> loads the timezone has or has not been called prior to calling
> gmtime.
This is indeed a bug - an oversight on my part, sorry.
Would calling do_tzset() at the beginning of gmtime_r() fix it ? Would
it be prohibitively expensive, and if so, what would be a better
solution ?
> - there's no synchronization in gmtime's (or gmtime_r's) access to the
> leap seconds data so the latter isn't even thread-safe.
OK, if changing timezones during the lifetime of a process is a valid
use case, then things must be protected. There needs to be a read-only
lock around __leapsecs_add() and __leapsecs_sub(), and a read-write lock
around the setting of __leapsecs_num and abbrevs_end. I couldn't find
such a thing as a shared lock system in musl, though. Should I go for
LOCK and UNLOCK, even though it would uselessly prevent concurrent
reads, or is there a better way ?
> What I can tell you is that this code (the time internals) is not
> something I expect to make any changes to between now and some time
> after 1.0, so for your own usage or usage by anybody else who wants
> it, your patch should continue to apply successfully.
I intend to maintain it until it gets integrated.
> And I don't mind
> further discussion of how to improve it in the mean time, but in the
> immediate future my focus on musl will be getting 0.9.15 and 1.0.0
> releases ready according to the existing plan, without big invasive
> changes.
Please consider integrating it for 1.0.0 if it meets all your
criteria by then.
--
Laurent
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
