RSSAfter doing some poking around in the source code for the Zune's clock driver (available free from the Freescale website), I found the root cause of the now-infamous Zune 30 leapyear issue that struck everyone on New Year's Eve.
The Zune's real-time clock stores the time in terms of days and seconds since January 1st, 1980. When the Zune's clock is accessed, the driver turns the number of days into years/months/days and the number of seconds into hours/minutes/seconds. Likewise, when the clock is set, the driver does the opposite.
The Zune frontend first accesses the clock toward the end of the boot sequence. Doing this triggers the code that reads the clock and converts it to a date and time. Below is the part of this code that determines the year component of the date:
year = ORIGINYEAR; /* = 1980 */
while (days > 365)
{
if (IsLeapYear(year))
{
if (days > 366)
{
days -= 366;
year += 1;
}
}
else
{
days -= 365;
year += 1;
}
}
Looks like it's a leap year thing and it might happen again in 4 years... For now those with ZUNEs can just wait a day. This bug and the android "run every word you type" bug - typing "reboot" would reboot the phone are 2008's weirdest mobile device bugs.
It looks like today is day 366, so the inner test ("if (days > 366)") knows enough not to increment the year yet, but the outer loop still thinks the year isn't done ("days > 365"). While loops can get tricky! (and by tricky, I mean fail in a bad way).
The code has two copyrights, so it looks like Microsoft modified Freescale's original code. I wonder who made the mistake, and if anyone else uses this code...
Thanks!
Reply to this comment
In 4 years... all the current Zune owners will have upgraded to an iPod.
Reply to this comment
while (days > 365 && !(IsLeapYear(year) && days == 366))
...
That's kind of a hack I guess, can anyone think of a better way?
Reply to this comment
the most explicit way to formulate the expression is, because it says, what the case for leap years and the case for regular years is:
while ( (days > 365 && !IsLeapYear(year)) || (days > 366 && IsLeapYear(year)))
to avoid calling IsLeapYear twice, it could be shortened to (has to be at least 366 days left, unless it is regular year, then 366 are okay to go to the next year)
while ((days > 366 || (IsLeapYear(year) && days == 366))
The little advantage is that at runtime, only the first condition days > 366 will be tested most of the time. So, IsLeapYear doesn't have to be checked.
How about (for concept, might have some bugs):
year = days / 366 + ORIGINYEAR;
days = days % 366;
leapYearDays = (year - ORIGINYEAR) / 4 + 1;
days += leapYearDays;
if ((days > 366 || (IsLeapYear(year) && days == 366)) {
days -= (IsLeapYear(year) ? 366 : 365;
year++;
}
Obviously, there is a bug every 100 years, because there are special leap year rules. Though, one more if-statement for that. And, those devices probably won't be around long enough to run into special leap year case.
Reply to this comment
year = days / 366 + ORIGINYEAR;
days = days % 366;
leapYearDays = (year - ORIGINYEAR) / 4 + 1;
days -= leapYearDays;
if ((days > 366 || (IsLeapYear(year) && days == 366)) {
days -= (IsLeapYear(year) ? 366 : 365;
year++;
}
Reply to this comment
Jeez, it's too late in the night. Though, you get the idea to use modulo and the knowledge that every four years is a leap year (mostly). That way no loop is needed.
I wasn't sure, if it was better to add the extra days or subtract them. I kept going back and force. That's how the errors ended up, because I had pieces left from either solution.
year = days / 366 + ORIGINYEAR;
days = days % 366;
leapYearDays = (year - ORIGINYEAR) / 4 + 1;
days -= leapYearDays;
if (days < 0) {
year--;
days += (IsLeapYear(year) ? 366 : 365;
}
Reply to this comment
...
if (days > 366)
{
days -= 366;
year += 1;
}
else
{
break;
}
...
:-D
Reply to this comment
yeah....just add a break
Reply to this comment
ommitting errors fixes it:
year = ORIGINYEAR; /* = 1980 */
while (days > 365)
{
if (IsLeapYear(year))
{
days -= 366;
year += 1;
}
else
{
days -= 365;
year += 1;
}
}
less is more!
Bebbo
Reply to this comment
.. and wouldn't work either, because:
while (days > 365) would execute on 31.12.2012 and it would be a leap year, but it would still be 2012, so
days -= 366 and year += 1 mustn't be executed.
Every loop will stay wrong as long as the exit condition is static.
This will work and is much less complicated:
int daysayear = (365 + (isLeapYear(year)?1:0) )
while (days > daysayear)
{
days -= daysayear;
year += 1;
daysayear = (365 + (isLeapYear(year)?1:0) )
}
Reply to this comment
you version is correct, but the fact that the first line appears two times is bugging me.
I would suggest an own function for daysAYear:
int daysAYear(int year) {
return 365 + (isLeapYear(year)?1:0);
}
then the code can be like this:
int year = ORIGINYEAR;
while (days > daysAYear(year)) {
days -= daysAYear(year);
year++;
};
The additional processing overhead should be tolerable. The main problem cannot be solved: the code was obviously not tested; or at least not well tested. A simple boundary value test would have saved the Zune users a lot of trouble. On the other hand: it's a pretty bad, if code for processing the clock has the possibility to hang the whole device. Just another example of the quality of software code offered nowadays.
Reply to this comment
Why are still figuring out how to design a leap-year calculator?
Wasn't this problem solved like 20 years ago?
Reply to this comment
Why is this a while loop?
if (days > 365)
{
if (IsLeapYear(year))
{
if (days > 366)
{
days -= 366;
year += 1;
}
}
else
{
days -= 365;
year += 1;
}
}
Reply to this comment
Why we are still inventing code for leap year calculations? Wasn't this problem solved 20 years ago?
Reply to this comment