drivers/net/wireless/airo.c: correct a wrong
RFC PATCH 1/1 drivers/net/wireless/airo.c: correct a wrong
if
Reply-To:
The Coverity checker correctly noted that this condition can't ever be fulfilled.
Can someone understanding this code check whether my guess what this should have been was right?
Or should the if get completely dropped?
drivers/net/wireless/airo.c renamed from drivers/net/wireless/airo.c.old +0 -3
@@ -3440,9 +3440,6 @@/* Make sure we got something */ if (rxd.rdy && rxd.valid ==0 ) { len = rxd.len +12 ; -if (len <12 && len >2048 ) - goto badrx; - skb =dev_alloc_skb (len);if (!skb) { ai->stats.rx_dropped++;
Quoted 14 lines · Adrian Bunk
if
Reply-To:The Coverity checker correctly noted that this condition can't ever be fulfilled.
Can someone understanding this code check whether my guess what this should have been was right?
Or should the if get completely dropped?
Signed-off-by Adrian Bunk <[email protected]>
drivers/net/wireless/airo.c line 3444
Quoted 5 lines
drivers/net/wireless/airo.c renamed from drivers/net/wireless/airo.c.old +0
-2@@ -3440,9 +3440,6 @@/* Make sure we got something */ if (rxd.rdy && rxd.valid ==0 ) { len = rxd.len +12 ; -if (len <12 && len >2048 ) - goto badrx;
Coverity is silly.
len is signed, and so can obviously be less than zero in edge cases. I don't see where the "> 2048" test is invalid, either.
Jeff
Quoted 27 lines · Jeff Garzik
Quoted 18 lines · Adrian Bunk
if
Reply-To:The Coverity checker correctly noted that this condition can't ever be fulfilled.
Can someone understanding this code check whether my guess what this should have been was right?
Or should the if get completely dropped?
Signed-off-by Adrian Bunk <[email protected]>--- linux-2.6.12-rc1-mm1-full/drivers/net/wireless/airo.c.old 2005-03-22
21:41:37.000000000 +0100
+++linux-2.6.12-rc1-mm1-full/drivers/net/wireless/airo.c2005-03-22
21:42:01.000000000 +0100drivers/net/wireless/airo.c line 3444
Quoted 5 lines
drivers/net/wireless/airo.c renamed from drivers/net/wireless/airo.c.old +0
-2@@ -3440,9 +3440,6 @@/* Make sure we got something */ if (rxd.rdy && rxd.valid ==0 ) { len = rxd.len +12 ; -if (len <12 && len >2048 ) - goto badrx;Coverity is silly.
Quoted 2 lines
len is signed, and so can obviously be less than zero in edge cases. I don't see where the "> 2048" test is invalid, either.
But if it's less than zero it can't be > 2048 at the same time?
The point is: len can't be both < 12 and > 2048 at the same time.
Is this "if" simply superfluous?
Or should the && be an || ?
Quoted 1 line
Jeff
cu
Adrian
Signature · 4 lines
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon SeedQuoted 31 lines · Adrian Bunk
Quoted 27 lines · Jeff Garzik
Quoted 18 lines · Adrian Bunk
if
Reply-To:The Coverity checker correctly noted that this condition can't ever be fulfilled.
Can someone understanding this code check whether my guess what this should have been was right?
Or should the if get completely dropped?
Signed-off-by Adrian Bunk <[email protected]>--- linux-2.6.12-rc1-mm1-full/drivers/net/wireless/airo.c.old 2005-03-22
21:41:37.000000000 +0100
+++linux-2.6.12-rc1-mm1-full/drivers/net/wireless/airo.c2005-03-22
21:42:01.000000000 +0100drivers/net/wireless/airo.c line 3444
Quoted 5 lines
drivers/net/wireless/airo.c renamed from drivers/net/wireless/airo.c.old +0
-2@@ -3440,9 +3440,6 @@/* Make sure we got something */ if (rxd.rdy && rxd.valid ==0 ) { len = rxd.len +12 ; -if (len <12 && len >2048 ) - goto badrx;Coverity is silly.
Quoted 2 lines
len is signed, and so can obviously be less than zero in edge cases. I don't see where the "> 2048" test is invalid, either.
Quoted 7 lines
But if it's less than zero it can't be > 2048 at the same time?
The point is: len can't be both < 12 and > 2048 at the same time.
Is this "if" simply superfluous?
Or should the && be an || ?
Yes, it looks like it should be "||".
Jeff
PATCH 1/1 drivers/net/wireless/airo.c: correct a wrong check
Quoted 6 lines · Jeff Garzik
On Tue, Mar 22, 2005 at 11:30:56PM +0100, Adrian Bunk wrote:
...Quoted 2 lines
Is this "if" simply superfluous?
Or should the && be an || ?Yes, it looks like it should be "||".
Patch below.
Quoted 1 line
Jeff
cu
Adrian
<-- snip -->
The Coverity checker correctly noted that this condition can't ever be fulfilled.
This patch changes it to what it should have been.
drivers/net/wireless/airo.c renamed from drivers/net/wireless/airo.c.old +1 -1
@@ -3440,7 +3440,7 @@/* Make sure we got something */ if (rxd.rdy && rxd.valid ==0 ) { len = rxd.len +12 ; -if (len <12 && len >2048 ) +if (len <12 || len >2048 ) goto badrx; skb =dev_alloc_skb (len);