Fix for mktime() issue

[Albrecht Dreß]

Am 18.03.10 03:09 schrieb(en) eponymous alias:
> --- On Tue, 3/16/10, Ton Voon <ton.voon at opsera.com> wrote:
>> 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.
> 
> Huh???  There's no such thing as undefined in C.  There's a bit pattern of some sort in that data cell, you'd better believe it. Deliberately blinding yourself to what it is just leaves you open to all manner of potential mischief when it sometimes turns out not to contain what you expect it to.

That's *exactly* my point!  Basically, it's like writing some random value to it.

>> (because I don't want to add new test cases and this is a recent change and Nagios didn't have issues here before)
> 
> That's a recipe for disaster, introducing deliberate ambiguity into the calculation.

Again, I 100% agree with you here.

> If you don't trust the mktime() implementation, then a few test cases run outside of Nagios itself should be able to prove that before the software is even built.

That was the purpose of my test app.  It works as advertised by POSIX/ISO 9899 on Linux 32 and 64 bit, and on Mac OS X 10.4.

Basically Ton's approach as far as I understand it is like in function check_time_against_period() of base/utils.c:

<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.  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) instead of shifting midnight around.

Best, Albrecht.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: not available
URL: <https://www.monitoring-lists.org/archive/developers/attachments/20100318/31fcff15/attachment.sig>
-------------- 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