<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On 20 Mar 2010, at 01:33, Holger Weiß wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>* Albrecht Dreß <<a href="mailto:albrecht.dress@arcor.de">albrecht.dress@arcor.de</a>> [2010-03-18 19:29]:<br><blockquote type="cite"><snip><br></blockquote><blockquote type="cite"> t=localtime((time_t *)&test_time);<br></blockquote><blockquote type="cite"> test_time_year=t->tm_year;<br></blockquote><blockquote type="cite"> test_time_mon=t->tm_mon;<br></blockquote><blockquote type="cite"> test_time_mday=t->tm_mday;<br></blockquote><blockquote type="cite"> test_time_wday=t->tm_wday;<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"> /* calculate the start of the day (midnight, 00:00 hours) when the specified test time occurs */<br></blockquote><blockquote type="cite"> t->tm_sec=0;<br></blockquote><blockquote type="cite"> t->tm_min=0;<br></blockquote><blockquote type="cite"> t->tm_hour=0;<br></blockquote><blockquote type="cite"> /* Removed for the moment. This fixes a bug where the timeperiod is incorrectly calculated */<br></blockquote><blockquote type="cite"> /* See t-tap/test_timeperiods for a test failure */<br></blockquote><blockquote type="cite"> /* t->tm_isdst=-1; */<br></blockquote><blockquote type="cite"> midnight=(unsigned long)mktime(t);<br></blockquote><blockquote type="cite"></snip><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Regarding the tm_isdst field, this code "initialises" it implicitly by<br></blockquote><blockquote type="cite">calling localtime() but then assumes for the calculation of midnight<br></blockquote><blockquote type="cite">that the dst setting is still correct which may be plain wrong.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Example: test_time is 1269734400 (Mar 28 01:00:00 2010 CET), midnight<br></blockquote><blockquote type="cite">will correctly be calculated as Mar 28 00:00:00 2010. If test_time is<br></blockquote><blockquote type="cite">1269763200 (Mar 28 10:00:00 2010 CEST), midnight is calculated as Mar 27<br></blockquote><blockquote type="cite">23:00:00 2010 which is obviously wrong. This miscalculation is fixed,<br></blockquote><blockquote type="cite">again, by properly initialising tm_isdst to -1.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Thus, either the "tests" missed the dst change cases, or the real use of<br></blockquote><blockquote type="cite">midnight is not midnight but something else.<br></blockquote><br>The real use of the midnight value is not "12 o'clock at night," but<br>"the zero point of the local time of the day," which differs from "12<br>o'clock at night" if tm_isdst changed since 12 o'clock at night. That<br>is, the offset of 10:00 AM from this zero point is 36,000 seconds,<br>regardless of whether tm_isdst changed since 12 o'clock at night.<br><br><blockquote type="cite">In the latter case, the fix is of course to re-write the time span<br></blockquote><blockquote type="cite">calculation to work properly (taking into account that days may also be<br></blockquote><blockquote type="cite">23 or 25 hours long, when dst changes)<br></blockquote><br>What makes you believe that the code in question does not work properly?<br></div></blockquote><div><br></div></div>Thankfully, Holger has opened my eyes to my "revert the tm_isdst=-1" position.<div><br></div><div>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".</div><div><br></div><div>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).</div><div><br></div><div>I think all other occurrences of tm_isdst=-1 are of type (2), so I am not planning on reverting those anymore.</div><div><br></div><div><div>Ton</div><div><br></div></div></body></html>