2014-07-12 03:57:47

by Bing Zhao

[permalink] [raw]
Subject: [PATCH 4/7] mwifiex: remove redundant TDLS setup action frame check

It's already been taken cared by the 'default' case in the
switch/case below.

Reported-by: Paul Stewart <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
Signed-off-by: Avinash Patil <[email protected]>
---
drivers/net/wireless/mwifiex/tdls.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/tdls.c b/drivers/net/wireless/mwifiex/tdls.c
index a414161..57f972d 100644
--- a/drivers/net/wireless/mwifiex/tdls.c
+++ b/drivers/net/wireless/mwifiex/tdls.c
@@ -791,19 +791,15 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv,
return;

peer = buf + ETH_ALEN;
- action = *(buf + sizeof(struct ethhdr) + 2);

- /* just handle TDLS setup request/response/confirm */
- if (action > WLAN_TDLS_SETUP_CONFIRM)
+ sta_ptr = mwifiex_add_sta_entry(priv, peer);
+ if (!sta_ptr)
return;

+ action = *(buf + sizeof(struct ethhdr) + 2);
dev_dbg(priv->adapter->dev,
"rx:tdls action: peer=%pM, action=%d\n", peer, action);

- sta_ptr = mwifiex_add_sta_entry(priv, peer);
- if (!sta_ptr)
- return;
-
switch (action) {
case WLAN_TDLS_SETUP_REQUEST:
if (len < (sizeof(struct ethhdr) + TDLS_REQ_FIX_LEN))
--
1.8.2.3



2014-07-15 04:07:02

by Bing Zhao

[permalink] [raw]
Subject: RE: [PATCH 4/7] mwifiex: remove redundant TDLS setup action frame check

SGkgUGF1bCwNCg0KVGhhbmtzIGZvciB5b3VyIGNvbW1lbnQuDQoNCj4gPiBAQCAtNzkxLDE5ICs3
OTEsMTUgQEAgdm9pZCBtd2lmaWV4X3Byb2Nlc3NfdGRsc19hY3Rpb25fZnJhbWUoc3RydWN0IG13
aWZpZXhfcHJpdmF0ZSAqcHJpdiwNCj4gPiAgICAgICAgICAgICAgICAgcmV0dXJuOw0KPiA+DQo+
ID4gICAgICAgICBwZWVyID0gYnVmICsgRVRIX0FMRU47DQo+ID4gLSAgICAgICBhY3Rpb24gPSAq
KGJ1ZiArIHNpemVvZihzdHJ1Y3QgZXRoaGRyKSArIDIpOw0KPiA+DQo+ID4gLSAgICAgICAvKiBq
dXN0IGhhbmRsZSBURExTIHNldHVwIHJlcXVlc3QvcmVzcG9uc2UvY29uZmlybSAqLw0KPiA+IC0g
ICAgICAgaWYgKGFjdGlvbiA+IFdMQU5fVERMU19TRVRVUF9DT05GSVJNKQ0KPiBJJ20gZ29pbmcg
dG8gYXNzdW1lIHRoYXQgdGhlIGludGVudCBvZiB0aGUgb3JpZ2luYWwgdGVzdCB3YXMgdG8gYXZv
aWQNCj4gYWxsb2NhdGlvbiBvZiBzdGFfcHRyIGJlbG93IGluIHRoZSBjYXNlIHdoZXJlIHRoZSBh
Y3Rpb24gd2FzIGludmFsaWQuDQo+IElzIGl0IG9rYXkgdG8gYWxsb2NhdGUgdGhpcyBwb2ludGVy
IGFuZCBub3QgZG8gYW55dGhpbmcgd2l0aCBpdCBiZWxvdz8NCg0KSSd2ZSBzZW50IHYyIHBhdGNo
LiBDb3VsZCB5b3UgcGxlYXNlIHJldmlldz8NCg0KVGhhbmtzLA0KQmluZw0KDQo+ID4gKyAgICAg
ICBzdGFfcHRyID0gbXdpZmlleF9hZGRfc3RhX2VudHJ5KHByaXYsIHBlZXIpOw0KPiA+ICsgICAg
ICAgaWYgKCFzdGFfcHRyKQ0KPiA+ICAgICAgICAgICAgICAgICByZXR1cm47DQo+ID4NCj4gPiAr
ICAgICAgIGFjdGlvbiA9ICooYnVmICsgc2l6ZW9mKHN0cnVjdCBldGhoZHIpICsgMik7DQo+ID4g
ICAgICAgICBkZXZfZGJnKHByaXYtPmFkYXB0ZXItPmRldiwNCj4gPiAgICAgICAgICAgICAgICAg
InJ4OnRkbHMgYWN0aW9uOiBwZWVyPSVwTSwgYWN0aW9uPSVkXG4iLCBwZWVyLCBhY3Rpb24pOw0K
PiA+DQo+ID4gLSAgICAgICBzdGFfcHRyID0gbXdpZmlleF9hZGRfc3RhX2VudHJ5KHByaXYsIHBl
ZXIpOw0KPiA+IC0gICAgICAgaWYgKCFzdGFfcHRyKQ0KPiA+IC0gICAgICAgICAgICAgICByZXR1
cm47DQo+ID4gLQ0KPiA+ICAgICAgICAgc3dpdGNoIChhY3Rpb24pIHsNCj4gPiAgICAgICAgIGNh
c2UgV0xBTl9URExTX1NFVFVQX1JFUVVFU1Q6DQo+ID4gICAgICAgICAgICAgICAgIGlmIChsZW4g
PCAoc2l6ZW9mKHN0cnVjdCBldGhoZHIpICsgVERMU19SRVFfRklYX0xFTikpDQo+ID4gLS0NCj4g
PiAxLjguMi4zDQo+ID4NCg==

2014-07-12 03:57:42

by Bing Zhao

[permalink] [raw]
Subject: [PATCH 5/7] mwifiex: define TDLS idle timeout macro with units

The unit of this timeout is in seconds.

Reported-by: Paul Stewart <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
Signed-off-by: Avinash Patil <[email protected]>
---
drivers/net/wireless/mwifiex/fw.h | 2 +-
drivers/net/wireless/mwifiex/sta_cmd.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/fw.h b/drivers/net/wireless/mwifiex/fw.h
index 5561573..49da2d5 100644
--- a/drivers/net/wireless/mwifiex/fw.h
+++ b/drivers/net/wireless/mwifiex/fw.h
@@ -713,7 +713,7 @@ struct mwifiex_ie_types_vendor_param_set {
u8 ie[MWIFIEX_MAX_VSIE_LEN];
};

-#define MWIFIEX_TDLS_IDLE_TIMEOUT 60
+#define MWIFIEX_TDLS_IDLE_TIMEOUT_IN_SEC 60

struct mwifiex_ie_types_tdls_idle_timeout {
struct mwifiex_ie_types_header header;
diff --git a/drivers/net/wireless/mwifiex/sta_cmd.c b/drivers/net/wireless/mwifiex/sta_cmd.c
index 0f077aa..733de92 100644
--- a/drivers/net/wireless/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/mwifiex/sta_cmd.c
@@ -1647,7 +1647,7 @@ mwifiex_cmd_tdls_oper(struct mwifiex_private *priv,
timeout = (void *)(pos + config_len);
timeout->header.type = cpu_to_le16(TLV_TYPE_TDLS_IDLE_TIMEOUT);
timeout->header.len = cpu_to_le16(sizeof(timeout->value));
- timeout->value = cpu_to_le16(MWIFIEX_TDLS_IDLE_TIMEOUT);
+ timeout->value = cpu_to_le16(MWIFIEX_TDLS_IDLE_TIMEOUT_IN_SEC);
config_len += sizeof(struct mwifiex_ie_types_tdls_idle_timeout);

break;
--
1.8.2.3


2014-07-12 03:57:58

by Bing Zhao

[permalink] [raw]
Subject: [PATCH 7/7] mwifiex: correct a typo in mwifiex_ret_tdls_oper

This patch fixes this typo.

Reported-by: Paul Stewart <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
Signed-off-by: Avinash Patil <[email protected]>
---
drivers/net/wireless/mwifiex/sta_cmdresp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mwifiex/sta_cmdresp.c b/drivers/net/wireless/mwifiex/sta_cmdresp.c
index 822357b..08b78ba 100644
--- a/drivers/net/wireless/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/mwifiex/sta_cmdresp.c
@@ -908,7 +908,7 @@ static int mwifiex_ret_tdls_oper(struct mwifiex_private *priv,
break;
default:
dev_err(priv->adapter->dev,
- "Unknown TDLS command action respnse %d", action);
+ "Unknown TDLS command action response %d", action);
return -1;
}

--
1.8.2.3


2014-07-12 03:57:43

by Bing Zhao

[permalink] [raw]
Subject: [PATCH 6/7] mwifiex: declare sta_ptr in smaller scope

sta_ptr is used only in an 'if' branch in this function.
Move it to the smaller scope where it is used.

Reported-by: Paul Stewart <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
Signed-off-by: Avinash Patil <[email protected]>
---
drivers/net/wireless/mwifiex/11n.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mwifiex/11n.c b/drivers/net/wireless/mwifiex/11n.c
index 9d6d8d9..62f5dbe 100644
--- a/drivers/net/wireless/mwifiex/11n.c
+++ b/drivers/net/wireless/mwifiex/11n.c
@@ -541,7 +541,6 @@ void mwifiex_create_ba_tbl(struct mwifiex_private *priv, u8 *ra, int tid,
int mwifiex_send_addba(struct mwifiex_private *priv, int tid, u8 *peer_mac)
{
struct host_cmd_ds_11n_addba_req add_ba_req;
- struct mwifiex_sta_node *sta_ptr;
u32 tx_win_size = priv->add_ba_param.tx_win_size;
static u8 dialog_tok;
int ret;
@@ -553,6 +552,8 @@ int mwifiex_send_addba(struct mwifiex_private *priv, int tid, u8 *peer_mac)
ISSUPP_TDLS_ENABLED(priv->adapter->fw_cap_info) &&
priv->adapter->is_hw_11ac_capable &&
memcmp(priv->cfg_bssid, peer_mac, ETH_ALEN)) {
+ struct mwifiex_sta_node *sta_ptr;
+
sta_ptr = mwifiex_get_sta_entry(priv, peer_mac);
if (!sta_ptr) {
dev_warn(priv->adapter->dev,
--
1.8.2.3


2014-07-12 14:01:24

by Paul Stewart

[permalink] [raw]
Subject: Re: [PATCH 4/7] mwifiex: remove redundant TDLS setup action frame check

On Fri, Jul 11, 2014 at 8:57 PM, Bing Zhao <[email protected]> wrote:
> It's already been taken cared by the 'default' case in the
> switch/case below.
>
> Reported-by: Paul Stewart <[email protected]>
> Signed-off-by: Bing Zhao <[email protected]>
> Signed-off-by: Avinash Patil <[email protected]>
> ---
> drivers/net/wireless/mwifiex/tdls.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/tdls.c b/drivers/net/wireless/mwifiex/tdls.c
> index a414161..57f972d 100644
> --- a/drivers/net/wireless/mwifiex/tdls.c
> +++ b/drivers/net/wireless/mwifiex/tdls.c
> @@ -791,19 +791,15 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv,
> return;
>
> peer = buf + ETH_ALEN;
> - action = *(buf + sizeof(struct ethhdr) + 2);
>
> - /* just handle TDLS setup request/response/confirm */
> - if (action > WLAN_TDLS_SETUP_CONFIRM)
I'm going to assume that the intent of the original test was to avoid
allocation of sta_ptr below in the case where the action was invalid.
Is it okay to allocate this pointer and not do anything with it below?
> + sta_ptr = mwifiex_add_sta_entry(priv, peer);
> + if (!sta_ptr)
> return;
>
> + action = *(buf + sizeof(struct ethhdr) + 2);
> dev_dbg(priv->adapter->dev,
> "rx:tdls action: peer=%pM, action=%d\n", peer, action);
>
> - sta_ptr = mwifiex_add_sta_entry(priv, peer);
> - if (!sta_ptr)
> - return;
> -
> switch (action) {
> case WLAN_TDLS_SETUP_REQUEST:
> if (len < (sizeof(struct ethhdr) + TDLS_REQ_FIX_LEN))
> --
> 1.8.2.3
>