sungem: limit reaches -1, but 0 tested
Subsystems (2)
PATCH 1/1 sungem: limit reaches -1, but 0 tested
With a postfix decrement these reach -1 rather than 0, but after the loop it is tested to have become 0.
---
drivers/net/sungem_phy.c +1 -1
@@ -79,7 +79,7 @@ static int reset_one_mii_phy(struct mii_phy* phy, int phy_id)udelay (100 ); -while (limit--) { +while (--limit) { val =__phy_read (phy, phy_id,MII_BMCR );if ((val &BMCR_RESET ) ==0 )break ;
David Miller
maintainerFrom: Roel Kluin <[email protected]>
Date: Sat, 31 Jan 2009 13:42:36 +0100
Quoted 4 lines
With a postfix decrement these reach -1 rather than 0, but after the loop it is tested to have become 0.
Signed-off-by Roel Kluin <[email protected]>
Just like the case just pointed out in your 'net' version of this patch, it is being tested "<= 0" so this fix is not necessary at all.
Please carefully review all of your changes of this kind to make sure the test is strictly equality to zero rather than <= 0.
I'm dropping all of these postfix decrement patches.
Quoted 11 lines · David Miller
From: Roel Kluin <[email protected]>
Date: Sat, 31 Jan 2009 13:42:36 +0100Quoted 4 lines
With a postfix decrement these reach -1 rather than 0, but after the loop it is tested to have become 0.
Signed-off-by Roel Kluin <[email protected]>Just like the case just pointed out in your 'net' version of this patch, it is being tested "<= 0" so this fix is not necessary at all.
The change is correct although the changelog is not clear. This is also true for the cassini patch, here included as well.
How about:
------------------>8----------------8<------------------------- while (limit--)
if (test()) break;
if (limit <= 0)
goto test_failed;
In the last iteration, limit is decremented after the test to 0. If just thereafter test() succeeds and a break occurs, the goto still occurs because limit is 0.
--- drivers/net/cassini.c | 4 ++-- drivers/net/sungem_phy.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/net/cassini.c +2 -2
@@ -806,7 +806,7 @@ static int cas_reset_mii_phy(struct cas *cp)cas_phy_write (cp,MII_BMCR ,BMCR_RESET );udelay (100 ); -while (limit--) { +while (--limit) { val =cas_phy_read (cp,MII_BMCR );if ((val &BMCR_RESET ) ==0 )break ;
@@ -979,7 +979,7 @@ static void cas_phy_init(struct cas *cp)writel (val, cp->regs +REG_PCS_MII_CTRL ); limit =STOP_TRIES ; -while (limit-- >0 ) { +while (--limit >0 ) {udelay (10 );if ((readl (cp->regs +REG_PCS_MII_CTRL ) &PCS_MII_RESET ) ==0 )
drivers/net/sungem_phy.c +1 -1
@@ -79,7 +79,7 @@ static int reset_one_mii_phy(struct mii_phy* phy, int phy_id)udelay (100 ); -while (limit--) { +while (--limit) { val =__phy_read (phy, phy_id,MII_BMCR );if ((val &BMCR_RESET ) ==0 )break ;
David Miller
maintainerFrom: Roel Kluin <[email protected]>
Date: Sun, 01 Feb 2009 18:15:27 +0100
Quoted 4 lines
while (limit--)
if (test()) break;
Quoted 8 lines
if (limit <= 0)
goto test_failed;In the last iteration, limit is decremented after the test to 0. If just thereafter
test()succeeds and a break occurs, the goto still occurs because limit is 0.Signed-off-by Roel Kluin <[email protected]>
Also applied, thanks.