The Exact Cause of the Zune Meltdown

An anonymous reader writes "The Zune 30 failure became national news when it happened just three days ago. The source code for the bad driver leaked soon after, and now, someone has come up with a very detailed explanation for where the code was bad as well as a number of solutions to deal with it. From a coding/QA standpoint, one has to wonder how this bug was missed if the quality assurance team wasn't slacking off. Worse yet: this bug affects every Windows CE device carrying this driver."

Comments

Re:Old

[LostCluster]

Yep, but it deserves to be covered so that everybody hears it. It's not just a laugh at Microsoft story, but also a lesson to aspiring programmers to watch there step when it comes to timekeeping. Gotta get a mention to the people who look at /. at work, gotta get a mention to the people who visit weeknights, gotta mention it for the weekend crowd.

Bigger bugs have gotten through on Windows CE

[msgmonkey]

For example I had some code I developed on Windows CE 4.2 .NET which kept on hanging on calling the FindWindow() fuction call.

Turns out that trying to find a window by class name will hang (this version of) CE every time, even though you would have thought its a very much used function call and would be caught by CE.

So no I'm not surprised at all that this bug got through.

Sad code, sad article

[gnasher719]

Both the original code and the various corrections in the article don't catch what the algorithm is supposed to do, and therefore create code that is too complicated.

The essence of the algorithm is this: We start with number of days since 1/Jan/1980, with the first day having the number one. We want to end up with the correct year, with a day number relative to the first day of that year, with the first day again having the number one. So we set year = 1980. And as long as day is greater than the number of days in that year, we can't have the right value yet, so we change day and year accordingly. This produces a very simple loop:

for (;;) {
    int daysInYear = IsLeapYear (year) ? 366 : 365;
    if (day = daysInYear) break;
    day -= daysInYear; year += 1;
}

This is what Knuth called an "N + 1/2" loop: A loop pattern where a more or less substantial bit of code has to be executed at the beginning of the loop before we can decide whether the loop needs exiting or continuing. By following the "N+1/2 loop" pattern we avoid repeating the same code (with possible small changes) completely. And that exactly was the problem here: The same code was used twice but slightly differently (one set number of days = 365, the other made it dependent on whether the year was a leap year or not). The solutions given in the article all contain repeated code; either two loop exits, or a duplicated calculation of the number of days in a year.

Re:Regardless of whatever code in it is faulty

[concernedadmin]

Lines 122, 521, 690, 710, and 748 scare me; gotos in C code...

They've used one form of a goto that's actually quite readable and useful. Would you rather have:

if (condition1 && condition2) {
/* boilerplate code with a return */
}

if (issue1 || issue2) {
/* same repeated boilerplate code with a return */
}

or

if (condition1 && condition2) {
goto cleanup;
}

if (issue1 || issue2) {
goto cleanup;
}
cleanup:
/* just one instance of this code,
no need for duplication of efforts */ Believe it or not, there are useful reasons to use goto, and Microsoft happened to use goto for the right reason here. The Linux kernel also happens to use this practice to boost the readability of the code.

Calendrical Calculations

[kabloom]

The proper way to do this would be with division and modulus, which gives you a nice constant time solution even if you're still using your Zune in 2108. They ought to read Calendrical Calculations by Nachum Dershowitz and Ed Reingold and learn how to do this properly.

Re:Regardless of whatever code in it is faulty

[QRDeNameland]

The addition of single bool avoids both the specialized cleanup() function and the goto:

bool needs_cleanup = false;

if (condition1 && condition2) {

needs_cleanup = true;

}

if (issue1 || issue2) {

needs_cleanup = true;

}

if (needs_cleanup) {

// clean up local vars exactly as you would have done

// have done under the cleanup: label with the goto

}