pcnet: fix sparse triviality
Subsystems (2)
PATCH 1/1 pcnet: fix sparse triviality
Since data can never exceed u32, it can't even be larger than LONG_MAX/HZ.
---
drivers/net/pcnet32.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-)
drivers/net/pcnet32.c +2 -3
@@ -1101,9 +1101,8 @@ static int pcnet32_phys_id(struct net_device *dev, u32 data)mod_timer (&lp->blink_timer, jiffies);set_current_state (TASK_INTERRUPTIBLE ); -/* AV: the limit here makes no sense whatsoever */ -if ((!data) || (data > (u32) (MAX_SCHEDULE_TIMEOUT /HZ ))) - data = (u32) (MAX_SCHEDULE_TIMEOUT /HZ ); +if (!data) + data =INT_MAX ;msleep_interruptible (data *1000 );del_timer_sync (&lp->blink_timer);
Quoted 15 lines · Auke Kok
Since data can never exceed u32, it can't even be larger than LONG_MAX/HZ.
Signed-off-by Auke Kok <[email protected]>---
drivers/net/pcnet32.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-)drivers/net/pcnet32.c +0
-0@@ -1101,9 +1101,8 @@ static int pcnet32_phys_id(struct net_device *dev, u32 data)mod_timer (&lp->blink_timer, jiffies);
drivers/net/pcnet32.c static int pcnet32_phys_id(struct net_device *dev, u32 data) · line 1108
Quoted 10 lines
drivers/net/pcnet32.c +2
-3set_current_state (TASK_INTERRUPTIBLE ); -/* AV: the limit here makes no sense whatsoever */ -if ((!data) || (data > (u32) (MAX_SCHEDULE_TIMEOUT /HZ ))) - data = (u32) (MAX_SCHEDULE_TIMEOUT /HZ ); +if (!data) + data =INT_MAX ;msleep_interruptible (data *1000 );del_timer_sync (&lp->blink_timer);
Two comments:
1) I would prefer to pick a sane limit, like "1 day". The unit of 'data' is seconds, so IMO we should not allow stupid timeouts, much less INT_MAX ones :) But hey, then again, maybe we should permit root to hang themselves with own rope...
2) [tangent] someone really should add the obvious ssleep_interruptible()
Quoted 8 lines · Jeff Garzik
Quoted 7 lines · Auke Kok
Since data can never exceed u32, it can't even be larger than LONG_MAX/HZ.
Signed-off-by Auke Kok <[email protected]>---
Quoted 6 lines
Two comments:
1) I would prefer to pick a sane limit, like "1 day". The unit of 'data' is seconds, so IMO we should not allow stupid timeouts, much less
INT_MAXones :) But hey, then again, maybe we should permit root to hang themselves with own rope...
well, in this case it's only used for the blink interval for the LED on the back of the adapter. This is completely interruptable by the user with ^C when running ethtool, so it's not going to hang anything (traffic should continue nicely).
yes INT_MAX is insanely long. moving all of this checking to the ethtool generic code makes much more sense indeed.
Quoted 1 line
2) [tangent] someone really should add the obvious
ssleep_interruptible()
*pass* :)
Auke