Messages in this thread |  | | | Date | Mon, 17 Jul 2006 20:53:30 +0200 | | From | Andreas Mohr <> | | Subject | kernel/timer.c: next_timer_interrupt() strange/buggy(?) code (2.6.18-rc1-mm2) |
| |
Hi all,
next_timer_interrupt() contains the following gem:
/* Check tv2-tv5. */ varray[0] = &base->tv2; varray[1] = &base->tv3; varray[2] = &base->tv4; varray[3] = &base->tv5; for (i = 0; i < 4; i++) { j = INDEX(i); do { if (list_empty(varray[i]->vec + j)) { j = (j + 1) & TVN_MASK; continue; } list_for_each_entry(nte, varray[i]->vec + j, entry) if (time_before(nte->expires, expires)) expires = nte->expires; if (j < (INDEX(i)) && i < 3) list = varray[i + 1]->vec + (INDEX(i + 1)); goto found; } while (j != (INDEX(i))); } found: if (list) {
Excuse me, but why do we have a while loop here if the last instruction in the while loop is a straight "goto found"? (a "continue" simply continue:s the loop without checking the loop condition at the bottom, right?)
Few lines above there's similar code:
/* Look for timer events in tv1. */ j = base->timer_jiffies & TVR_MASK; do { list_for_each_entry(nte, base->tv1.vec + j, entry) { expires = nte->expires; if (j < (base->timer_jiffies & TVR_MASK)) list = base->tv2.vec + (INDEX(0)); goto found; } j = (j + 1) & TVR_MASK; } while (j != (base->timer_jiffies & TVR_MASK));
However in this case now we process *one* list only, so the "goto found" should be ok since we don't need to iterate through the multiple tv2-tv5 lists as in the other potentially buggy loop.
Also, is the base->tv1.vec vs. base->tv2.vec difference above ok?
Possibly I'm too dense while reading this code, though...
Andreas Mohr - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |