Skip to contentSkip to conversation

wl12xx: loop off by one.

PATCH In review Roel Kluin 2 participants 4 messages Jul 6, 2009 – Jul 16, 2009 Networking drivers (wireless)

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 wl12xx: loop off by one.

with while (loop++ < INIT_LOOP)' loop' reaches -1 after the loop.

Signed-off-by Roel Kluin <[email protected]>

---

drivers/net/wireless/wl12xx/boot.c +1 -1
index 48ac08c..8899905 100644
@@ -247,7 +247,7 @@ int wl12xx_boot_run_firmware(struct wl12xx *wl)
 		}
 	}
 
-	if (loop >= INIT_LOOP) {
+	if (loop > INIT_LOOP) {
 		wl12xx_error("timeout waiting for the hardware to "
 			     "complete initialization");
 		return -EIO;
#1 lore raw
Quoted 1 line · Roel Kluin

with while (loop++ < INIT_LOOP)' loop' reaches -1 after the loop.

Sorry, my brains are on vacation mode right now and I can't understand how loop can reach -1. Here's the code:

    loop = 0;
    while (loop++ < INIT_LOOP) {
          ...
    }

    if (loop >= INIT_LOOP) {
       wl1251_error("timeout waiting for the hardware to "
                    "complete initialization");
       return -EIO;
    }

What am I missing?

Also the patch won't apply to wireless-testing because main.c is renamed to wl1251_main.c.

Signature · 1 line
Kalle Valo
author
#2 lore raw
In reply to Roel Kluin

With while (loop++ < INIT_LOOP)' loop' becomes INIT_LOOP + 1 after the loop.

Signed-off-by Roel Kluin <[email protected]>

---

Quoted 4 lines
Quoted 1 line

with while (loop++ < INIT_LOOP)' loop' reaches -1 after the loop.

Sorry, my brains are on vacation mode right now and I can't understand how loop can reach -1.

The confusion is my fault. The changelog should have as above. The patch is correct though.

Quoted 12 lines

Here's the code:

   loop = 0;
   while (loop++ < INIT_LOOP) {
         ...
   }

   if (loop >= INIT_LOOP) {
      wl1251_error("timeout waiting for the hardware to "
                   "complete initialization");
      return -EIO;
   }

In the last iteration loop' is INIT_LOOP -1', the test occurs and then the increment to `INIT_LOOP'. If just then the break occurs:
`if (interrupt & wl->chip.intr_init_complete)' evaluates to true, then we error out, although there was no timeout. This could be very unlikely to occur.

Quoted 2 lines

Also the patch won't apply to wireless-testing because main.c is renamed to wl1251_main.c.

Ok, this is against your tree

drivers/net/wireless/wl12xx/wl1251_boot.c +1 -1
index d8a155d..b2ae71c 100644
@@ -247,7 +247,7 @@ int wl1251_boot_run_firmware(struct wl1251 *wl)
 		}
 	}
 
-	if (loop >= INIT_LOOP) {
+	if (loop > INIT_LOOP) {
 		wl1251_error("timeout waiting for the hardware to "
 			     "complete initialization");
 		return -EIO;
Acked-by
#3 lore raw
Quoted 4 lines · Roel Kluin

With while (loop++ < INIT_LOOP)' loop' becomes INIT_LOOP + 1 after the loop.

Signed-off-by Roel Kluin <[email protected]>
Quoted 8 lines

---

Quoted 4 lines
Quoted 1 line

with while (loop++ < INIT_LOOP)' loop' reaches -1 after the loop.

Sorry, my brains are on vacation mode right now and I can't understand how loop can reach -1.

The confusion is my fault. The changelog should have as above. The patch is correct though.

Ah. Thanks for explaining this to me, I understand this now. Good catch, thanks for fixing this.

Acked-by Kalle Valo <[email protected]>

John, please take this patch. Please holler if we need to resend it.

Signature · 1 line
Kalle Valo