Fix for mktime() issue
Ton Voon
ton.voon at opsera.com
Tue Mar 16 10:30:13 CET 2010
On 15 Mar 2010, at 20:10, Albrecht Dreß wrote:
> Am 15.03.10 20:42 schrieb(en) Ton Voon:
>> This patch was applied to Nagios Core and was in 3.2.0. I've fixed
>> a specific problem last year with timeperiods when DST moved back
>> one hour, but I didn't change the other occurrences of this patch.
>> My feeling was to get a test case for the specific changes before
>> reverting the patch.
>> However, I think there are still some timezone issues (as mentioned
>> by Mark Frost on nagios-users), so I'm thinking that I should
>> revert the entire patch and instead say that if you want to add the
>> isdst=-1 in, then add test cases in.
>
> The POSIX standard says [1]
>
> <snip>
> A positive or 0 value for tm_isdst shall cause mktime() to presume
> initially that Daylight Savings Time, respectively, is or is not in
> effect for the specified time. A negative value for tm_isdst shall
> cause mktime() to attempt to determine whether Daylight Savings Time
> is in effect for the specified time.
> </snip>
>
> Thus, unless you know for sure that daylight savings is in effect or
> not for a specific date, /not/ using tm_isdst < 0 is just plain
> wrong (iirc, the field was not initialised at all without my patch,
> which may give random results).
>
> Note that tm_isdst < 0 has been marked as POSIX extension to ISO
> 9899, i.e. broken systems which do not implement POSIX (read:
> Winbloze) may fail here. Probably the only resort is to use utc
> exclusively.
>
>> Opinions?
>
> What should be the purpose of a test case if the code strictly
> follows POSIX?
Because the test cases proves real-world usage, rather than some spec?
In t-tap/test_timeperiods.c, I've got tests for a specific case of
what happened over the DST change in the autumn. Adding the
tm_isdst=-1 gives errors, whereas undefined works.
What I don't have, are tests for the other 18 cases where is_dst has
been initialised, and so I didn't revert those other changes. But my
opinion is now that this is too cautious and we should just revert
back to Nagios pre-3.2.0 behaviour.
So what I am proposing is:
* remove all the is_dst initialisations (because I don't want to
add new test cases and this is a recent change and Nagios didn't have
issues here before)
* only add specific cases back when there is a test case that
proves that it should be initialised
The only way to prove this is running tests. Maybe some platforms
implement POSIX different. The best way to tell is to have test cases
that run on your system of choice and report to Tinderbox. There are
currently three servers running Tinderbox builds at the moment. I'm up
for anyone adding more: http://tinderbox.nagios.org and http://tinderbox.nagios.org/nagios/status.html
Ton
------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
More information about the Developers
mailing list