Fix for mktime() issue

[Ton Voon]

On 20 Mar 2010, at 01:33, Holger Weiß wrote:

> * Albrecht Dreß <albrecht.dress at arcor.de> [2010-03-18 19:29]:
>> <snip>
>>        t=localtime((time_t *)&test_time);
>>        test_time_year=t->tm_year;
>>        test_time_mon=t->tm_mon;
>>        test_time_mday=t->tm_mday;
>>        test_time_wday=t->tm_wday;
>>
>>        /* calculate the start of the day (midnight, 00:00 hours)  
>> when the specified test time occurs */
>>        t->tm_sec=0;
>>        t->tm_min=0;
>>        t->tm_hour=0;
>>        /* Removed for the moment. This fixes a bug where the  
>> timeperiod is incorrectly calculated */
>>        /* See t-tap/test_timeperiods for a test failure */
>>        /* t->tm_isdst=-1; */
>>        midnight=(unsigned long)mktime(t);
>> </snip>
>>
>> Regarding the tm_isdst field, this code "initialises" it implicitly  
>> by
>> calling localtime() but then assumes for the calculation of midnight
>> that the dst setting is still correct which may be plain wrong.
>>
>> Example: test_time is 1269734400 (Mar 28 01:00:00 2010 CET), midnight
>> will correctly be calculated as Mar 28 00:00:00 2010.  If test_time  
>> is
>> 1269763200 (Mar 28 10:00:00 2010 CEST), midnight is calculated as  
>> Mar 27
>> 23:00:00 2010 which is obviously wrong.  This miscalculation is  
>> fixed,
>> again, by properly initialising tm_isdst to -1.
>>
>> Thus, either the "tests" missed the dst change cases, or the real  
>> use of
>> midnight is not midnight but something else.
>
> The real use of the midnight value is not "12 o'clock at night," but
> "the zero point of the local time of the day," which differs from "12
> o'clock at night" if tm_isdst changed since 12 o'clock at night.  That
> is, the offset of 10:00 AM from this zero point is 36,000 seconds,
> regardless of whether tm_isdst changed since 12 o'clock at night.
>
>> In the latter case, the fix is of course to re-write the time span
>> calculation to work properly (taking into account that days may  
>> also be
>> 23 or 25 hours long, when dst changes)
>
> What makes you believe that the code in question does not work  
> properly?

Thankfully, Holger has opened my eyes to my "revert the tm_isdst=-1"  
position.

Holger said (on IRC) "there were two different cases in the code: (1)  
at most places in the code, tm_isdst was initialized just fine (by  
localtime(3)), but (2) at other places, it was indeed used  
uninitialized".

I've already fixed on occurrence of (1) (in  
check_time_against_period()), and I think I've found the other  
occurrence now (in get_next_valid_time()). This is now fixed with  
updated test cases in test_timeperiods.c. There is one particular test  
that fails at the moment which I'm not sure how to fix - any ideas  
would be appreciated (it is marked as a TODO so full tests will not  
fail).

I think all other occurrences of tm_isdst=-1 are of type (2), so I am  
not planning on reverting those anymore.

Ton

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.monitoring-lists.org/archive/developers/attachments/20100322/374e6d59/attachment.html>
-------------- next part --------------
------------------------------------------------------------------------------
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
-------------- next part --------------
_______________________________________________
Nagios-devel mailing list
Nagios-devel at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nagios-devel