Re: [Nagios-devel] Fix for mktime() issue
Posted: Fri Mar 19, 2010 12:04 am
On 18 Mar 2010, at 18:29, Albrecht Dre=DF wrote:
> Am 18.03.10 03:09 schrieb(en) eponymous alias:
>> If you don't trust the mktime() implementation, then a few test =20
>> cases run outside of Nagios itself should be able to prove that =20
>> before the software is even built.
>
> That was the purpose of my test app. It works as advertised by =20
> 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 =20
> function check_time_against_period() of base/utils.c:
>
>
> t=3Dlocaltime((time_t *)&test_time);
> test_time_year=3Dt->tm_year;
> test_time_mon=3Dt->tm_mon;
> test_time_mday=3Dt->tm_mday;
> test_time_wday=3Dt->tm_wday;
>
> /* calculate the start of the day (midnight, 00:00 hours) =20
> when the specified test time occurs */
> t->tm_sec=3D0;
> t->tm_min=3D0;
> t->tm_hour=3D0;
> /* Removed for the moment. This fixes a bug where the =20
> timeperiod is incorrectly calculated */
> /* See t-tap/test_timeperiods for a test failure */
> /* t->tm_isdst=3D-1; */
> midnight=3D(unsigned long)mktime(t);
>
>
> Regarding the tm_isdst field, this code "initialises" it implicitly =20=
> by calling localtime(), but then assumes for the calculation of =20
> midnight that the dst setting is still correct which may be plain =20
> wrong.
>
> Example: test_time is 1269734400 (Mar 28 01:00:00 2010 CET), =20
> midnight will correctly be calculated as Mar 28 00:00:00 2010. If =20
> test_time is 1269763200 (Mar 28 10:00:00 2010 CEST), midnight is =20
> calculated as Mar 27 23:00:00 2010 which is obviously wrong. This =20
> miscalculation is fixed, again, by properly initialising tm_isdst to =20=
> -1.
>
> Thus, either the "tests" missed the dst change cases, or the real =20
> use of midnight is not midnight but something else. In the latter =20
> case, the fix is of course to re-write the time span calculation to =20=
> work properly (taking into account that days may also be 23 or 25 =20
> hours long, when dst changes) instead of shifting midnight around.
It could be that the nagios code incorrectly calculating 1269763200 as =20=
23:00:00 is why the algorithm works (by time shifting everything back =20=
an hour). But I don't really care.
The fact is, the tests work with isdst unset.
=46rom comments in this thread, I don't think anyone has actually run =20=
the tests, so here are the steps:
wget http://nagios.sourceforge.net/download/ ... EAD.tar.gz
tar --gzip -xf nagios-HEAD.tar.gz
cd nagios-HEAD
./configure --enable-libtap
make nagios
make cgis
make test-tap
cd t-tap && ./test_timeperiods
Go on, take 5 mins to do it. I'll wait.
Tests will pass with the current code (Nagios 3.2.1).
Edit utils.c, add the tm_isdst=3D-1 in at line 857, recompile utils.o =20=
and test_timeperiods and execute test_timeperiods and 4 out of 3006 =20
tests will fail (I neglected to add a time_t output in the comment. =20
Committed now).
One of those failures is for TZ=3DEurope/London at 1256511661 =3D Sun =
Oct =20
25 23:01:01 2009, which is the date clocks go back one hour in Europe. =20=
This is precisely the problem everyone saw last year.
If you come back and tell me that my tests are wrong (preferably with =20=
why and what they should be), I will humbly apologise, fix it and then =20=
crawl back under my rock. Alternatively, if they are right, I'd like =20
to move forward with removing the other 16 cases of isdst=3D-1, with the =
=20
policy of only making changes when someone writes test cases ala =20
test_timeperiods to prove why the current code is wrong.
Let me spell this out: specs do not make an argument; test cases =20
against the nagios functions do.
Ton
This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]