2011-06-03 02:12:02

by Greg Dietsche

Subject: [PATCH 1/5] iwlegacy: remove unreachable code

return; at the end of the function is unecessary.

Signed-off-by: Greg Dietsche <[email protected]>
---
drivers/net/wireless/iwlegacy/iwl-eeprom.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/iwl-eeprom.c b/drivers/net/wireless/iwlegacy/iwl-eeprom.c
index cb346d1..5bf3f49 100644
--- a/drivers/net/wireless/iwlegacy/iwl-eeprom.c
+++ b/drivers/net/wireless/iwlegacy/iwl-eeprom.c
@@ -316,7 +316,6 @@ static void iwl_legacy_init_band_reference(const struct iwl_priv *priv,
break;
default:
BUG();
- return;
}
}

--
1.7.2.5

2011-06-03 02:12:12

by Greg Dietsche

Subject: [PATCH 2/5] iwlegacy: remove unecessary if statement

the code always returns ret regardless, so if(ret) check is unecessary.
Signed-off-by: Greg Dietsche <[email protected]>
---
drivers/net/wireless/iwlegacy/iwl-4965.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/iwl-4965.c b/drivers/net/wireless/iwlegacy/iwl-4965.c
index f5433c7..ffda05c 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965.c
@@ -1185,8 +1185,6 @@ static int iwl4965_send_rxon_assoc(struct iwl_priv *priv,

ret = iwl_legacy_send_cmd_pdu_async(priv, REPLY_RXON_ASSOC,
sizeof(rxon_assoc), &rxon_assoc, NULL);
- if (ret)
- return ret;

return ret;
}
--
1.7.2.5

2011-06-03 02:12:14

by Greg Dietsche

Subject: [PATCH 3/5] iwlegacy: return -EINVAL instead of -1

Cleanup the code to return -EINVAL instead of -1
Signed-off-by: Greg Dietsche <[email protected]>
---
drivers/net/wireless/iwlegacy/iwl-4965.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/iwl-4965.c b/drivers/net/wireless/iwlegacy/iwl-4965.c
index ffda05c..06dcee5 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965.c
@@ -496,7 +496,7 @@ static s32 iwl4965_get_tx_atten_grp(u16 channel)
channel <= CALIB_IWL_TX_ATTEN_GR4_LCH)
return CALIB_CH_GROUP_4;

- return -1;
+ return -EINVAL;
}

static u32 iwl4965_get_sub_band(const struct iwl_priv *priv, u32 channel)
--
1.7.2.5

2011-06-03 02:12:16

by Greg Dietsche

Subject: [PATCH 4/5] iwlegacy: propagate error return value

propogate the return value from iwl4965_get_tx_atten_grp instead
of implicitly returning -EINVAL in the error case.

Signed-off-by: Greg Dietsche <[email protected]>
---
drivers/net/wireless/iwlegacy/iwl-4965.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/iwl-4965.c b/drivers/net/wireless/iwlegacy/iwl-4965.c
index 06dcee5..79e7ce6 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965.c
@@ -915,7 +915,7 @@ static int iwl4965_fill_txpower_tbl(struct iwl_priv *priv, u8 band, u16 channel,
if (txatten_grp < 0) {
IWL_ERR(priv, "Can't find txatten group for channel %d.\n",
channel);
- return -EINVAL;
+ return txatten_grp;
}

IWL_DEBUG_TXPOWER(priv, "channel %d belongs to txatten group %d\n",
--
1.7.2.5

2011-06-03 02:12:37

by Greg Dietsche

Subject: [PATCH 5/5] iwlegacy: add missing null check

lq_sta has other null checks in this funcion.
assuming they are correct, this additional null check
should be added too.

Signed-off-by: Greg Dietsche <[email protected]>
---
drivers/net/wireless/iwlegacy/iwl-4965-rs.c | 66 ++++++++++++++-------------
1 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
index 24d1499..a475aac 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
@@ -2275,40 +2275,42 @@ iwl4965_rs_get_rate(void *priv_r, struct ieee80211_sta *sta, void *priv_sta,
if (rate_control_send_low(sta, priv_sta, txrc))
return;

- rate_idx = lq_sta->last_txrate_idx;
-
- if (lq_sta->last_rate_n_flags & RATE_MCS_HT_MSK) {
- rate_idx -= IWL_FIRST_OFDM_RATE;
- /* 6M and 9M shared same MCS index */
- rate_idx = (rate_idx > 0) ? (rate_idx - 1) : 0;
- if (iwl4965_rs_extract_rate(lq_sta->last_rate_n_flags) >=
- IWL_RATE_MIMO2_6M_PLCP)
- rate_idx = rate_idx + MCS_INDEX_PER_STREAM;
- info->control.rates[0].flags = IEEE80211_TX_RC_MCS;
- if (lq_sta->last_rate_n_flags & RATE_MCS_SGI_MSK)
- info->control.rates[0].flags |=
- IEEE80211_TX_RC_SHORT_GI;
- if (lq_sta->last_rate_n_flags & RATE_MCS_DUP_MSK)
- info->control.rates[0].flags |=
- IEEE80211_TX_RC_DUP_DATA;
- if (lq_sta->last_rate_n_flags & RATE_MCS_HT40_MSK)
- info->control.rates[0].flags |=
- IEEE80211_TX_RC_40_MHZ_WIDTH;
- if (lq_sta->last_rate_n_flags & RATE_MCS_GF_MSK)
- info->control.rates[0].flags |=
- IEEE80211_TX_RC_GREEN_FIELD;
- } else {
- /* Check for invalid rates */
- if ((rate_idx < 0) || (rate_idx >= IWL_RATE_COUNT_LEGACY) ||
- ((sband->band == IEEE80211_BAND_5GHZ) &&
- (rate_idx < IWL_FIRST_OFDM_RATE)))
- rate_idx = rate_lowest_index(sband, sta);
- /* On valid 5 GHz rate, adjust index */
- else if (sband->band == IEEE80211_BAND_5GHZ)
+ if (lq_sta) {
+ rate_idx = lq_sta->last_txrate_idx;
+
+ if (lq_sta->last_rate_n_flags & RATE_MCS_HT_MSK) {
rate_idx -= IWL_FIRST_OFDM_RATE;
- info->control.rates[0].flags = 0;
+ /* 6M and 9M shared same MCS index */
+ rate_idx = (rate_idx > 0) ? (rate_idx - 1) : 0;
+ if (iwl4965_rs_extract_rate(lq_sta->last_rate_n_flags) >=
+ IWL_RATE_MIMO2_6M_PLCP)
+ rate_idx = rate_idx + MCS_INDEX_PER_STREAM;
+ info->control.rates[0].flags = IEEE80211_TX_RC_MCS;
+ if (lq_sta->last_rate_n_flags & RATE_MCS_SGI_MSK)
+ info->control.rates[0].flags |=
+ IEEE80211_TX_RC_SHORT_GI;
+ if (lq_sta->last_rate_n_flags & RATE_MCS_DUP_MSK)
+ info->control.rates[0].flags |=
+ IEEE80211_TX_RC_DUP_DATA;
+ if (lq_sta->last_rate_n_flags & RATE_MCS_HT40_MSK)
+ info->control.rates[0].flags |=
+ IEEE80211_TX_RC_40_MHZ_WIDTH;
+ if (lq_sta->last_rate_n_flags & RATE_MCS_GF_MSK)
+ info->control.rates[0].flags |=
+ IEEE80211_TX_RC_GREEN_FIELD;
+ } else {
+ /* Check for invalid rates */
+ if ((rate_idx < 0) || (rate_idx >= IWL_RATE_COUNT_LEGACY) ||
+ ((sband->band == IEEE80211_BAND_5GHZ) &&
+ (rate_idx < IWL_FIRST_OFDM_RATE)))
+ rate_idx = rate_lowest_index(sband, sta);
+ /* On valid 5 GHz rate, adjust index */
+ else if (sband->band == IEEE80211_BAND_5GHZ)
+ rate_idx -= IWL_FIRST_OFDM_RATE;
+ info->control.rates[0].flags = 0;
+ }
+ info->control.rates[0].idx = rate_idx;
}
- info->control.rates[0].idx = rate_idx;

}

--
1.7.2.5

2011-06-03 03:07:47

by Gustavo Padovan

Subject: Re: [PATCH 5/5] iwlegacy: add missing null check

Hi Greg,

* Greg Dietsche <[email protected]> [2011-06-02 21:06:10 -0500]:

> lq_sta has other null checks in this funcion.
> assuming they are correct, this additional null check
> should be added too.
>
> Signed-off-by: Greg Dietsche <[email protected]>
> ---
> drivers/net/wireless/iwlegacy/iwl-4965-rs.c | 66 ++++++++++++++-------------
> 1 files changed, 34 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> index 24d1499..a475aac 100644
> --- a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> +++ b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> @@ -2275,40 +2275,42 @@ iwl4965_rs_get_rate(void *priv_r, struct ieee80211_sta *sta, void *priv_sta,
> if (rate_control_send_low(sta, priv_sta, txrc))
> return;
>
> - rate_idx = lq_sta->last_txrate_idx;
> -
> - if (lq_sta->last_rate_n_flags & RATE_MCS_HT_MSK) {
> - rate_idx -= IWL_FIRST_OFDM_RATE;
> - /* 6M and 9M shared same MCS index */
> - rate_idx = (rate_idx > 0) ? (rate_idx - 1) : 0;
> - if (iwl4965_rs_extract_rate(lq_sta->last_rate_n_flags) >=
> - IWL_RATE_MIMO2_6M_PLCP)
> - rate_idx = rate_idx + MCS_INDEX_PER_STREAM;
> - info->control.rates[0].flags = IEEE80211_TX_RC_MCS;
> - if (lq_sta->last_rate_n_flags & RATE_MCS_SGI_MSK)
> - info->control.rates[0].flags |=
> - IEEE80211_TX_RC_SHORT_GI;
> - if (lq_sta->last_rate_n_flags & RATE_MCS_DUP_MSK)
> - info->control.rates[0].flags |=
> - IEEE80211_TX_RC_DUP_DATA;
> - if (lq_sta->last_rate_n_flags & RATE_MCS_HT40_MSK)
> - info->control.rates[0].flags |=
> - IEEE80211_TX_RC_40_MHZ_WIDTH;
> - if (lq_sta->last_rate_n_flags & RATE_MCS_GF_MSK)
> - info->control.rates[0].flags |=
> - IEEE80211_TX_RC_GREEN_FIELD;
> - } else {
> - /* Check for invalid rates */
> - if ((rate_idx < 0) || (rate_idx >= IWL_RATE_COUNT_LEGACY) ||
> - ((sband->band == IEEE80211_BAND_5GHZ) &&
> - (rate_idx < IWL_FIRST_OFDM_RATE)))
> - rate_idx = rate_lowest_index(sband, sta);
> - /* On valid 5 GHz rate, adjust index */
> - else if (sband->band == IEEE80211_BAND_5GHZ)
> + if (lq_sta) {

Then do
if (!lq_sta)
return;

to avoid an exatra level of indentation.

--
Gustavo F. Padovan
http://profusion.mobi

2011-06-03 03:24:15

by Greg Dietsche

Subject: [PATCH 5/5 v2] iwlegacy: add missing null check

lq_sta has other null checks in this function.
assuming they are correct, this additional null check
should be added too.

Incorporating suggestion from Gustavo Padovan.

Signed-off-by: Greg Dietsche <[email protected]>
---
drivers/net/wireless/iwlegacy/iwl-4965-rs.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
index 24d1499..9b65153 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
@@ -2275,6 +2275,9 @@ iwl4965_rs_get_rate(void *priv_r, struct ieee80211_sta *sta, void *priv_sta,
if (rate_control_send_low(sta, priv_sta, txrc))
return;

+ if (!lq_sta)
+ return;
+
rate_idx = lq_sta->last_txrate_idx;

if (lq_sta->last_rate_n_flags & RATE_MCS_HT_MSK) {
--
1.7.2.5

2011-06-13 14:28:16

by Greg Dietsche

Subject: Re: [PATCH 1/5] iwlegacy: remove unreachable code

On 06/02/2011 09:06 PM, Greg Dietsche wrote:
> return; at the end of the function is unecessary.
>
> Signed-off-by: Greg Dietsche<[email protected]>
> ---
> drivers/net/wireless/iwlegacy/iwl-eeprom.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlegacy/iwl-eeprom.c b/drivers/net/wireless/iwlegacy/iwl-eeprom.c
> index cb346d1..5bf3f49 100644
> --- a/drivers/net/wireless/iwlegacy/iwl-eeprom.c
> +++ b/drivers/net/wireless/iwlegacy/iwl-eeprom.c
> @@ -316,7 +316,6 @@ static void iwl_legacy_init_band_reference(const struct iwl_priv *priv,
> break;
> default:
> BUG();
> - return;
> }
> }
>
>

any comments/reviews/acks on this patch series?

thanks,
Greg

2011-06-13 14:38:55

by Stanislaw Gruszka

Subject: Re: [PATCH 1/5] iwlegacy: remove unreachable code

On Mon, Jun 13, 2011 at 09:28:14AM -0500, Greg Dietsche wrote:
> On 06/02/2011 09:06 PM, Greg Dietsche wrote:
> >return; at the end of the function is unecessary.
> >
> >Signed-off-by: Greg Dietsche<[email protected]>
> >---
> > drivers/net/wireless/iwlegacy/iwl-eeprom.c | 1 -
> > 1 files changed, 0 insertions(+), 1 deletions(-)
> >
> >diff --git a/drivers/net/wireless/iwlegacy/iwl-eeprom.c b/drivers/net/wireless/iwlegacy/iwl-eeprom.c
> >index cb346d1..5bf3f49 100644
> >--- a/drivers/net/wireless/iwlegacy/iwl-eeprom.c
> >+++ b/drivers/net/wireless/iwlegacy/iwl-eeprom.c
> >@@ -316,7 +316,6 @@ static void iwl_legacy_init_band_reference(const struct iwl_priv *priv,
> > break;
> > default:
> > BUG();
> >- return;
> > }
> > }
> >
>
> any comments/reviews/acks on this patch series?

All 5 patches are applied in wireless-testing tree.

Thanks
Stanislaw