[patch] airo: remove pointless check

[patch] airo: remove pointless check

[Dan Carpenter]

It doesn't make sense to check "!ai->config.rates" here.
"ai->config.rates" is the address of an eight bytes array and it
can't ever be null here.  Also if it were NULL then trying to set:
	ai->config.rates[i] = basic_rate | 0x80;
would cause an oops.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index 1d05445..d806497 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -3886,8 +3886,7 @@ static u16 setup_card(struct airo_info *ai, u8 *mac, int lock)
 		}
 		if ( basic_rate > 0 ) {
 			for( i = 0; i < 8; i++ ) {
-				if ( ai->config.rates[i] == basic_rate ||
-				     !ai->config.rates ) {
+				if (ai->config.rates[i] == basic_rate) {
 					ai->config.rates[i] = basic_rate | 0x80;
 					break;
 				}

Re: [patch] airo: remove pointless check

[Stanislaw Gruszka]

On Fri, Sep 10, 2010 at 01:57:14PM +0200, Dan Carpenter wrote:
> It doesn't make sense to check "!ai->config.rates" here.
> "ai->config.rates" is the address of an eight bytes array and it
> can't ever be null here.  Also if it were NULL then trying to set:
> 	ai->config.rates[i] = basic_rate | 0x80;
> would cause an oops.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
> index 1d05445..d806497 100644
> --- a/drivers/net/wireless/airo.c
> +++ b/drivers/net/wireless/airo.c
> @@ -3886,8 +3886,7 @@ static u16 setup_card(struct airo_info *ai, u8 *mac, int lock)
>  		}
>  		if ( basic_rate > 0 ) {
>  			for( i = 0; i < 8; i++ ) {
> -				if ( ai->config.rates[i] == basic_rate ||
> -				     !ai->config.rates ) {
> +				if (ai->config.rates[i] == basic_rate) {
>  					ai->config.rates[i] = basic_rate | 0x80;
>  					break;
>  				}

I think code author wonted to do "!ai->config.rates[i], what mean "if
rate is invalid use basic rate". 

I'm not sure if basic_rate module parameter is used by anyone. It's not
described, seems it was created for development purpose. I think we can
remove it ...

Stanislaw

[patch v2] airo: remove "basic_rate" module option

[Dan Carpenter]

The "basic_rate" module option is not implemented correctly.  If the
rate was set to zero it was supposed to set it to "basic_rate | 0x80".
Unfortunately the check to see if what zero was wrong and it checked
"!ai->config.rates" (which is always false) instead of
"!ai->config.rates[i]".

This option was just used for development and it wasn't documented
anywhere.  Instead of fixing it, we can just remove it.

Reported-by: Stanislaw Gruszka <sgruszka@redhat.com>
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
V2:  Version 1 was just a cleanup to please the static checkers but
didn't fix anything.  Stanislaw suggests that we can remove the feature
entirely so that's what I've done in version 2.

diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index 1d05445..c20cad7 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -217,7 +217,6 @@ static char *statsLabels[] = {
    (no spaces) list of rates (up to 8). */
 
 static int rates[8];
-static int basic_rate;
 static char *ssids[3];
 
 static int io[4];
@@ -250,7 +249,6 @@ MODULE_LICENSE("Dual BSD/GPL");
 MODULE_SUPPORTED_DEVICE("Aironet 4500, 4800 and Cisco 340/350");
 module_param_array(io, int, NULL, 0);
 module_param_array(irq, int, NULL, 0);
-module_param(basic_rate, int, 0);
 module_param_array(rates, int, NULL, 0);
 module_param_array(ssids, charp, NULL, 0);
 module_param(auto_wep, int, 0);
@@ -3884,15 +3882,6 @@ static u16 setup_card(struct airo_info *ai, u8 *mac, int lock)
 				ai->config.rates[i] = rates[i];
 			}
 		}
-		if ( basic_rate > 0 ) {
-			for( i = 0; i < 8; i++ ) {
-				if ( ai->config.rates[i] == basic_rate ||
-				     !ai->config.rates ) {
-					ai->config.rates[i] = basic_rate | 0x80;
-					break;
-				}
-			}
-		}
 		set_bit (FLAG_COMMIT, &ai->flags);
 	}