Fix for mktime() issue

[Ton Voon]

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