v2:
- addressed issues from review
- race-condition issue discussed in patch description
Andreas Fenkart (7):
mwifiex: scan: simplify dereference of bss_desc fields
mwifiex: scan: factor out has_ieee_hdr/has_vendor_hdr
mwifiex: scan: simplify ternary operators using gnu extension
mwifiex: scan: factor out dbg_security_flags
mwifiex: scan: replace pointer arithmetic with array access
mwifiex: factor out mwifiex_cancel_pending_scan_cmd
mwifiex: make mwifiex_insert_cmd_to_free_q local static
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 125 +++++++-------
drivers/net/wireless/marvell/mwifiex/main.h | 3 +-
drivers/net/wireless/marvell/mwifiex/scan.c | 185 ++++++++-------------
drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 13 +-
4 files changed, 128 insertions(+), 198 deletions(-)
--
2.7.0
given this structure:
struct foo {
struct bar {
int baz;
}
}
these accesses are equivalent:
(*(foo->bar)).baz
foo->bar->baz
Signed-off-by: Andreas Fenkart <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/scan.c | 98 ++++++++++++++---------------
1 file changed, 46 insertions(+), 52 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index c20017c..a7bdde5 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -121,8 +121,8 @@ mwifiex_is_rsn_oui_present(struct mwifiex_bssdescriptor *bss_desc, u32 cipher)
struct ie_body *iebody;
u8 ret = MWIFIEX_OUI_NOT_PRESENT;
- if (((bss_desc->bcn_rsn_ie) && ((*(bss_desc->bcn_rsn_ie)).
- ieee_hdr.element_id == WLAN_EID_RSN))) {
+ if (bss_desc->bcn_rsn_ie &&
+ bss_desc->bcn_rsn_ie->ieee_hdr.element_id == WLAN_EID_RSN) {
iebody = (struct ie_body *)
(((u8 *) bss_desc->bcn_rsn_ie->data) +
RSN_GTK_OUI_OFFSET);
@@ -148,9 +148,9 @@ mwifiex_is_wpa_oui_present(struct mwifiex_bssdescriptor *bss_desc, u32 cipher)
struct ie_body *iebody;
u8 ret = MWIFIEX_OUI_NOT_PRESENT;
- if (((bss_desc->bcn_wpa_ie) &&
- ((*(bss_desc->bcn_wpa_ie)).vend_hdr.element_id ==
- WLAN_EID_VENDOR_SPECIFIC))) {
+ if (bss_desc->bcn_wpa_ie &&
+ bss_desc->bcn_wpa_ie->vend_hdr.element_id ==
+ WLAN_EID_VENDOR_SPECIFIC) {
iebody = (struct ie_body *) bss_desc->bcn_wpa_ie->data;
oui = &mwifiex_wpa_oui[cipher][0];
ret = mwifiex_search_oui_in_ie(iebody, oui);
@@ -181,8 +181,8 @@ mwifiex_is_bss_wapi(struct mwifiex_private *priv,
{
if (priv->sec_info.wapi_enabled &&
(bss_desc->bcn_wapi_ie &&
- ((*(bss_desc->bcn_wapi_ie)).ieee_hdr.element_id ==
- WLAN_EID_BSS_AC_ACCESS_DELAY))) {
+ bss_desc->bcn_wapi_ie->ieee_hdr.element_id ==
+ WLAN_EID_BSS_AC_ACCESS_DELAY)) {
return true;
}
return false;
@@ -197,12 +197,12 @@ mwifiex_is_bss_no_sec(struct mwifiex_private *priv,
struct mwifiex_bssdescriptor *bss_desc)
{
if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
- !priv->sec_info.wpa2_enabled && ((!bss_desc->bcn_wpa_ie) ||
- ((*(bss_desc->bcn_wpa_ie)).vend_hdr.element_id !=
- WLAN_EID_VENDOR_SPECIFIC)) &&
- ((!bss_desc->bcn_rsn_ie) ||
- ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id !=
- WLAN_EID_RSN)) &&
+ !priv->sec_info.wpa2_enabled &&
+ (!bss_desc->bcn_wpa_ie ||
+ bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
+ WLAN_EID_VENDOR_SPECIFIC) &&
+ (!bss_desc->bcn_rsn_ie ||
+ bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN) &&
!priv->sec_info.encryption_mode && !bss_desc->privacy) {
return true;
}
@@ -233,9 +233,10 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
struct mwifiex_bssdescriptor *bss_desc)
{
if (!priv->sec_info.wep_enabled && priv->sec_info.wpa_enabled &&
- !priv->sec_info.wpa2_enabled && ((bss_desc->bcn_wpa_ie) &&
- ((*(bss_desc->bcn_wpa_ie)).
- vend_hdr.element_id == WLAN_EID_VENDOR_SPECIFIC))
+ !priv->sec_info.wpa2_enabled &&
+ (bss_desc->bcn_wpa_ie &&
+ bss_desc->bcn_wpa_ie->vend_hdr.element_id ==
+ WLAN_EID_VENDOR_SPECIFIC)
/*
* Privacy bit may NOT be set in some APs like
* LinkSys WRT54G && bss_desc->privacy
@@ -245,12 +246,10 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
"info: %s: WPA:\t"
"wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
"EncMode=%#x privacy=%#x\n", __func__,
- (bss_desc->bcn_wpa_ie) ?
- (*bss_desc->bcn_wpa_ie).
- vend_hdr.element_id : 0,
- (bss_desc->bcn_rsn_ie) ?
- (*bss_desc->bcn_rsn_ie).
- ieee_hdr.element_id : 0,
+ bss_desc->bcn_wpa_ie ?
+ bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
+ bss_desc->bcn_rsn_ie ?
+ bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
(priv->sec_info.wep_enabled) ? "e" : "d",
(priv->sec_info.wpa_enabled) ? "e" : "d",
(priv->sec_info.wpa2_enabled) ? "e" : "d",
@@ -269,11 +268,10 @@ static bool
mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
struct mwifiex_bssdescriptor *bss_desc)
{
- if (!priv->sec_info.wep_enabled &&
- !priv->sec_info.wpa_enabled &&
+ if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
priv->sec_info.wpa2_enabled &&
- ((bss_desc->bcn_rsn_ie) &&
- ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id == WLAN_EID_RSN))) {
+ (bss_desc->bcn_rsn_ie &&
+ bss_desc->bcn_rsn_ie->ieee_hdr.element_id == WLAN_EID_RSN)) {
/*
* Privacy bit may NOT be set in some APs like
* LinkSys WRT54G && bss_desc->privacy
@@ -282,12 +280,10 @@ mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
"info: %s: WPA2:\t"
"wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
"EncMode=%#x privacy=%#x\n", __func__,
- (bss_desc->bcn_wpa_ie) ?
- (*bss_desc->bcn_wpa_ie).
- vend_hdr.element_id : 0,
- (bss_desc->bcn_rsn_ie) ?
- (*bss_desc->bcn_rsn_ie).
- ieee_hdr.element_id : 0,
+ bss_desc->bcn_wpa_ie ?
+ bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
+ bss_desc->bcn_rsn_ie ?
+ bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
(priv->sec_info.wep_enabled) ? "e" : "d",
(priv->sec_info.wpa_enabled) ? "e" : "d",
(priv->sec_info.wpa2_enabled) ? "e" : "d",
@@ -308,11 +304,11 @@ mwifiex_is_bss_adhoc_aes(struct mwifiex_private *priv,
{
if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
!priv->sec_info.wpa2_enabled &&
- ((!bss_desc->bcn_wpa_ie) ||
- ((*(bss_desc->bcn_wpa_ie)).
- vend_hdr.element_id != WLAN_EID_VENDOR_SPECIFIC)) &&
- ((!bss_desc->bcn_rsn_ie) ||
- ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id != WLAN_EID_RSN)) &&
+ (!bss_desc->bcn_wpa_ie ||
+ bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
+ WLAN_EID_VENDOR_SPECIFIC) &&
+ (!bss_desc->bcn_rsn_ie ||
+ bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN) &&
!priv->sec_info.encryption_mode && bss_desc->privacy) {
return true;
}
@@ -329,23 +325,21 @@ mwifiex_is_bss_dynamic_wep(struct mwifiex_private *priv,
{
if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
!priv->sec_info.wpa2_enabled &&
- ((!bss_desc->bcn_wpa_ie) ||
- ((*(bss_desc->bcn_wpa_ie)).
- vend_hdr.element_id != WLAN_EID_VENDOR_SPECIFIC)) &&
- ((!bss_desc->bcn_rsn_ie) ||
- ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id != WLAN_EID_RSN)) &&
+ (!bss_desc->bcn_wpa_ie ||
+ bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
+ WLAN_EID_VENDOR_SPECIFIC) &&
+ (!bss_desc->bcn_rsn_ie ||
+ bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN) &&
priv->sec_info.encryption_mode && bss_desc->privacy) {
mwifiex_dbg(priv->adapter, INFO,
"info: %s: dynamic\t"
"WEP: wpa_ie=%#x wpa2_ie=%#x\t"
"EncMode=%#x privacy=%#x\n",
__func__,
- (bss_desc->bcn_wpa_ie) ?
- (*bss_desc->bcn_wpa_ie).
- vend_hdr.element_id : 0,
- (bss_desc->bcn_rsn_ie) ?
- (*bss_desc->bcn_rsn_ie).
- ieee_hdr.element_id : 0,
+ bss_desc->bcn_wpa_ie ?
+ bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
+ bss_desc->bcn_rsn_ie ?
+ bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
priv->sec_info.encryption_mode,
bss_desc->privacy);
return true;
@@ -464,10 +458,10 @@ mwifiex_is_network_compatible(struct mwifiex_private *priv,
"info: %s: failed: wpa_ie=%#x wpa2_ie=%#x WEP=%s\t"
"WPA=%s WPA2=%s EncMode=%#x privacy=%#x\n",
__func__,
- (bss_desc->bcn_wpa_ie) ?
- (*bss_desc->bcn_wpa_ie).vend_hdr.element_id : 0,
- (bss_desc->bcn_rsn_ie) ?
- (*bss_desc->bcn_rsn_ie).ieee_hdr.element_id : 0,
+ bss_desc->bcn_wpa_ie ?
+ bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
+ bss_desc->bcn_rsn_ie ?
+ bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
(priv->sec_info.wep_enabled) ? "e" : "d",
(priv->sec_info.wpa_enabled) ? "e" : "d",
(priv->sec_info.wpa2_enabled) ? "e" : "d",
--
2.7.0
Hi Andreas,
> From: Andreas Fenkart [mailto:[email protected]]
> Sent: Thursday, March 10, 2016 2:14 PM
> To: [email protected]
> Cc: Amitkumar Karwar; Nishant Sarmukadam; Julian Calaby; Kalle Valo;
> Andreas Fenkart
> Subject: [PATCH v2 0/7] mwifiex: cleanups
>
> v2:
> - addressed issues from review
> - race-condition issue discussed in patch description
>
> Andreas Fenkart (7):
> mwifiex: scan: simplify dereference of bss_desc fields
> mwifiex: scan: factor out has_ieee_hdr/has_vendor_hdr
> mwifiex: scan: simplify ternary operators using gnu extension
> mwifiex: scan: factor out dbg_security_flags
> mwifiex: scan: replace pointer arithmetic with array access
> mwifiex: factor out mwifiex_cancel_pending_scan_cmd
> mwifiex: make mwifiex_insert_cmd_to_free_q local static
>
> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 125 +++++++-------
> drivers/net/wireless/marvell/mwifiex/main.h | 3 +-
> drivers/net/wireless/marvell/mwifiex/scan.c | 185 ++++++++------
> -------
> drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 13 +-
> 4 files changed, 128 insertions(+), 198 deletions(-)
>
Thanks for the efforts for this cleanup work. Patches look fine to me.
Acked-by: Amitkumar Karwar <[email protected]>
Regards,
Amitkumar
Releasing the scan_pending lock in mwifiex_check_next_scan_command
introduces a short window where pending scan commands can be removed
or added before removing them all in mwifiex_cancel_pending_scan_cmd.
I think this is safe, since the worst thing to happen is that a
pending scan cmd is removed by the command handler. Adding new scan
commands is not possible while one is pending, see scan_processing flag.
Since all commands are removed from the queue anyway, we don't care if
some commands are removed by a different code path earlier, the final
state remains the same.
I assume, that the critical section needed for the check has been
extended over clearing the pending scan queue out of convenience. The
lock was already held and releasing it and grab it again was just
more work. It doesn't seem to be necessary because of concurrency.
Signed-off-by: Andreas Fenkart <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 43 ++++++++++------------
drivers/net/wireless/marvell/mwifiex/main.h | 1 +
drivers/net/wireless/marvell/mwifiex/scan.c | 23 +++---------
drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 13 +------
4 files changed, 27 insertions(+), 53 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index cb25aa7..61426b3 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -991,6 +991,23 @@ mwifiex_cmd_timeout_func(unsigned long function_context)
adapter->if_ops.card_reset(adapter);
}
+void
+mwifiex_cancel_pending_scan_cmd(struct mwifiex_adapter *adapter)
+{
+ struct cmd_ctrl_node *cmd_node = NULL, *tmp_node;
+ unsigned long flags;
+
+ /* Cancel all pending scan command */
+ spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
+ list_for_each_entry_safe(cmd_node, tmp_node,
+ &adapter->scan_pending_q, list) {
+ list_del(&cmd_node->list);
+ cmd_node->wait_q_enabled = false;
+ mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
+ }
+ spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
+}
+
/*
* This function cancels all the pending commands.
*
@@ -1029,16 +1046,7 @@ mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter)
spin_unlock_irqrestore(&adapter->cmd_pending_q_lock, flags);
spin_unlock_irqrestore(&adapter->mwifiex_cmd_lock, cmd_flags);
- /* Cancel all pending scan command */
- spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
- list_for_each_entry_safe(cmd_node, tmp_node,
- &adapter->scan_pending_q, list) {
- list_del(&cmd_node->list);
-
- cmd_node->wait_q_enabled = false;
- mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
- }
- spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
+ mwifiex_cancel_pending_scan_cmd(adapter);
if (adapter->scan_processing) {
spin_lock_irqsave(&adapter->mwifiex_cmd_lock, cmd_flags);
@@ -1070,9 +1078,8 @@ mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter)
void
mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter)
{
- struct cmd_ctrl_node *cmd_node = NULL, *tmp_node = NULL;
+ struct cmd_ctrl_node *cmd_node = NULL;
unsigned long cmd_flags;
- unsigned long scan_pending_q_flags;
struct mwifiex_private *priv;
int i;
@@ -1094,17 +1101,7 @@ mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter)
mwifiex_recycle_cmd_node(adapter, cmd_node);
}
- /* Cancel all pending scan command */
- spin_lock_irqsave(&adapter->scan_pending_q_lock,
- scan_pending_q_flags);
- list_for_each_entry_safe(cmd_node, tmp_node,
- &adapter->scan_pending_q, list) {
- list_del(&cmd_node->list);
- cmd_node->wait_q_enabled = false;
- mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
- }
- spin_unlock_irqrestore(&adapter->scan_pending_q_lock,
- scan_pending_q_flags);
+ mwifiex_cancel_pending_scan_cmd(adapter);
if (adapter->scan_processing) {
spin_lock_irqsave(&adapter->mwifiex_cmd_lock, cmd_flags);
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 2f7f478..f71f894 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1043,6 +1043,7 @@ int mwifiex_alloc_cmd_buffer(struct mwifiex_adapter *adapter);
int mwifiex_free_cmd_buffer(struct mwifiex_adapter *adapter);
void mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter);
void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
+void mwifiex_cancel_pending_scan_cmd(struct mwifiex_adapter *adapter);
void mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
struct cmd_ctrl_node *cmd_node);
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 327b4d8..de210c3 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -564,8 +564,6 @@ mwifiex_scan_channel_list(struct mwifiex_private *priv,
int ret = 0;
struct mwifiex_chan_scan_param_set *tmp_chan_list;
struct mwifiex_chan_scan_param_set *start_chan;
- struct cmd_ctrl_node *cmd_node, *tmp_node;
- unsigned long flags;
u32 tlv_idx, rates_size, cmd_no;
u32 total_scan_time;
u32 done_early;
@@ -722,16 +720,7 @@ mwifiex_scan_channel_list(struct mwifiex_private *priv,
sizeof(struct mwifiex_ie_types_header) + rates_size;
if (ret) {
- spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
- list_for_each_entry_safe(cmd_node, tmp_node,
- &adapter->scan_pending_q,
- list) {
- list_del(&cmd_node->list);
- cmd_node->wait_q_enabled = false;
- mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
- }
- spin_unlock_irqrestore(&adapter->scan_pending_q_lock,
- flags);
+ mwifiex_cancel_pending_scan_cmd(adapter);
break;
}
}
@@ -1894,12 +1883,13 @@ mwifiex_active_scan_req_for_passive_chan(struct mwifiex_private *priv)
static void mwifiex_check_next_scan_command(struct mwifiex_private *priv)
{
struct mwifiex_adapter *adapter = priv->adapter;
- struct cmd_ctrl_node *cmd_node, *tmp_node;
+ struct cmd_ctrl_node *cmd_node;
unsigned long flags;
spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
if (list_empty(&adapter->scan_pending_q)) {
spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
+
spin_lock_irqsave(&adapter->mwifiex_cmd_lock, flags);
adapter->scan_processing = false;
spin_unlock_irqrestore(&adapter->mwifiex_cmd_lock, flags);
@@ -1921,13 +1911,10 @@ static void mwifiex_check_next_scan_command(struct mwifiex_private *priv)
}
} else if ((priv->scan_aborting && !priv->scan_request) ||
priv->scan_block) {
- list_for_each_entry_safe(cmd_node, tmp_node,
- &adapter->scan_pending_q, list) {
- list_del(&cmd_node->list);
- mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
- }
spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
+ mwifiex_cancel_pending_scan_cmd(adapter);
+
spin_lock_irqsave(&adapter->mwifiex_cmd_lock, flags);
adapter->scan_processing = false;
spin_unlock_irqrestore(&adapter->mwifiex_cmd_lock, flags);
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
index 9ac7aa2..81e23d8 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
@@ -44,7 +44,6 @@ static void
mwifiex_process_cmdresp_error(struct mwifiex_private *priv,
struct host_cmd_ds_command *resp)
{
- struct cmd_ctrl_node *cmd_node = NULL, *tmp_node;
struct mwifiex_adapter *adapter = priv->adapter;
struct host_cmd_ds_802_11_ps_mode_enh *pm;
unsigned long flags;
@@ -71,17 +70,7 @@ mwifiex_process_cmdresp_error(struct mwifiex_private *priv,
break;
case HostCmd_CMD_802_11_SCAN:
case HostCmd_CMD_802_11_SCAN_EXT:
- /* Cancel all pending scan command */
- spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
- list_for_each_entry_safe(cmd_node, tmp_node,
- &adapter->scan_pending_q, list) {
- list_del(&cmd_node->list);
- spin_unlock_irqrestore(&adapter->scan_pending_q_lock,
- flags);
- mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
- spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
- }
- spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
+ mwifiex_cancel_pending_scan_cmd(adapter);
spin_lock_irqsave(&adapter->mwifiex_cmd_lock, flags);
adapter->scan_processing = false;
--
2.7.0
improves readability
Reviewed-by: Julian Calaby <[email protected]>
Signed-off-by: Andreas Fenkart <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/scan.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index c137fd8..327b4d8 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1000,27 +1000,24 @@ mwifiex_config_scan(struct mwifiex_private *priv,
chan_idx++) {
channel = user_scan_in->chan_list[chan_idx].chan_number;
- (scan_chan_list + chan_idx)->chan_number = channel;
+ scan_chan_list[chan_idx].chan_number = channel;
radio_type =
user_scan_in->chan_list[chan_idx].radio_type;
- (scan_chan_list + chan_idx)->radio_type = radio_type;
+ scan_chan_list[chan_idx].radio_type = radio_type;
scan_type = user_scan_in->chan_list[chan_idx].scan_type;
if (scan_type == MWIFIEX_SCAN_TYPE_PASSIVE)
- (scan_chan_list +
- chan_idx)->chan_scan_mode_bitmap
+ scan_chan_list[chan_idx].chan_scan_mode_bitmap
|= (MWIFIEX_PASSIVE_SCAN |
MWIFIEX_HIDDEN_SSID_REPORT);
else
- (scan_chan_list +
- chan_idx)->chan_scan_mode_bitmap
+ scan_chan_list[chan_idx].chan_scan_mode_bitmap
&= ~MWIFIEX_PASSIVE_SCAN;
if (*filtered_scan)
- (scan_chan_list +
- chan_idx)->chan_scan_mode_bitmap
+ scan_chan_list[chan_idx].chan_scan_mode_bitmap
|= MWIFIEX_DISABLE_CHAN_FILT;
if (user_scan_in->chan_list[chan_idx].scan_time) {
@@ -1035,9 +1032,9 @@ mwifiex_config_scan(struct mwifiex_private *priv,
scan_dur = adapter->active_scan_time;
}
- (scan_chan_list + chan_idx)->min_scan_time =
+ scan_chan_list[chan_idx].min_scan_time =
cpu_to_le16(scan_dur);
- (scan_chan_list + chan_idx)->max_scan_time =
+ scan_chan_list[chan_idx].max_scan_time =
cpu_to_le16(scan_dur);
}
--
2.7.0
"x ? x : y" can be simplified as "x ? : y"
https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html#Conditionals
Reviewed-by: Julian Calaby <[email protected]>
Signed-off-by: Andreas Fenkart <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/scan.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index ed886af..f623834 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -845,14 +845,11 @@ mwifiex_config_scan(struct mwifiex_private *priv,
/* Set the BSS type scan filter, use Adapter setting if
unset */
scan_cfg_out->bss_mode =
- (user_scan_in->bss_mode ? (u8) user_scan_in->
- bss_mode : (u8) adapter->scan_mode);
+ (u8)(user_scan_in->bss_mode ?: adapter->scan_mode);
/* Set the number of probes to send, use Adapter setting
if unset */
- num_probes =
- (user_scan_in->num_probes ? user_scan_in->
- num_probes : adapter->scan_probes);
+ num_probes = user_scan_in->num_probes ?: adapter->scan_probes;
/*
* Set the BSSID filter to the incoming configuration,
--
2.7.0
merge copy/paste code
Signed-off-by: Andreas Fenkart <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/scan.c | 74 ++++++++++-------------------
1 file changed, 25 insertions(+), 49 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index f623834..c137fd8 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -76,6 +76,27 @@ static u8 mwifiex_rsn_oui[CIPHER_SUITE_MAX][4] = {
{ 0x00, 0x0f, 0xac, 0x04 }, /* AES */
};
+static void
+_dbg_security_flags(int log_level, const char *func, const char *desc,
+ struct mwifiex_private *priv,
+ struct mwifiex_bssdescriptor *bss_desc)
+{
+ _mwifiex_dbg(priv->adapter, log_level,
+ "info: %s: %s:\twpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\tEncMode=%#x privacy=%#x\n",
+ func, desc,
+ bss_desc->bcn_wpa_ie ?
+ bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
+ bss_desc->bcn_rsn_ie ?
+ bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
+ priv->sec_info.wep_enabled ? "e" : "d",
+ priv->sec_info.wpa_enabled ? "e" : "d",
+ priv->sec_info.wpa2_enabled ? "e" : "d",
+ priv->sec_info.encryption_mode,
+ bss_desc->privacy);
+}
+#define dbg_security_flags(mask, desc, priv, bss_desc) \
+ _dbg_security_flags(MWIFIEX_DBG_##mask, desc, __func__, priv, bss_desc)
+
static bool
has_ieee_hdr(struct ieee_types_generic *ie, u8 key)
{
@@ -243,19 +264,7 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
* LinkSys WRT54G && bss_desc->privacy
*/
) {
- mwifiex_dbg(priv->adapter, INFO,
- "info: %s: WPA:\t"
- "wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
- "EncMode=%#x privacy=%#x\n", __func__,
- bss_desc->bcn_wpa_ie ?
- bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
- bss_desc->bcn_rsn_ie ?
- bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
- (priv->sec_info.wep_enabled) ? "e" : "d",
- (priv->sec_info.wpa_enabled) ? "e" : "d",
- (priv->sec_info.wpa2_enabled) ? "e" : "d",
- priv->sec_info.encryption_mode,
- bss_desc->privacy);
+ dbg_security_flags(INFO, "WPA", priv, bss_desc);
return true;
}
return false;
@@ -276,19 +285,7 @@ mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
* Privacy bit may NOT be set in some APs like
* LinkSys WRT54G && bss_desc->privacy
*/
- mwifiex_dbg(priv->adapter, INFO,
- "info: %s: WPA2:\t"
- "wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
- "EncMode=%#x privacy=%#x\n", __func__,
- bss_desc->bcn_wpa_ie ?
- bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
- bss_desc->bcn_rsn_ie ?
- bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
- (priv->sec_info.wep_enabled) ? "e" : "d",
- (priv->sec_info.wpa_enabled) ? "e" : "d",
- (priv->sec_info.wpa2_enabled) ? "e" : "d",
- priv->sec_info.encryption_mode,
- bss_desc->privacy);
+ dbg_security_flags(INFO, "WAP2", priv, bss_desc);
return true;
}
return false;
@@ -325,17 +322,7 @@ mwifiex_is_bss_dynamic_wep(struct mwifiex_private *priv,
!has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC) &&
!has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN) &&
priv->sec_info.encryption_mode && bss_desc->privacy) {
- mwifiex_dbg(priv->adapter, INFO,
- "info: %s: dynamic\t"
- "WEP: wpa_ie=%#x wpa2_ie=%#x\t"
- "EncMode=%#x privacy=%#x\n",
- __func__,
- bss_desc->bcn_wpa_ie ?
- bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
- bss_desc->bcn_rsn_ie ?
- bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
- priv->sec_info.encryption_mode,
- bss_desc->privacy);
+ dbg_security_flags(INFO, "dynamic", priv, bss_desc);
return true;
}
return false;
@@ -448,18 +435,7 @@ mwifiex_is_network_compatible(struct mwifiex_private *priv,
}
/* Security doesn't match */
- mwifiex_dbg(adapter, ERROR,
- "info: %s: failed: wpa_ie=%#x wpa2_ie=%#x WEP=%s\t"
- "WPA=%s WPA2=%s EncMode=%#x privacy=%#x\n",
- __func__,
- bss_desc->bcn_wpa_ie ?
- bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
- bss_desc->bcn_rsn_ie ?
- bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
- (priv->sec_info.wep_enabled) ? "e" : "d",
- (priv->sec_info.wpa_enabled) ? "e" : "d",
- (priv->sec_info.wpa2_enabled) ? "e" : "d",
- priv->sec_info.encryption_mode, bss_desc->privacy);
+ dbg_security_flags(ERROR, "failed", priv, bss_desc);
return -1;
}
--
2.7.0
after factoring out mwifiex_cancel_pending_scan_cmd
the function is not called outside of cmdevt file
moved function to head of file to avoid forward declaration,
also moved mwifiex_recycle_cmd_node since they are very similar
Signed-off-by: Andreas Fenkart <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 82 +++++++++++++--------------
drivers/net/wireless/marvell/mwifiex/main.h | 2 -
2 files changed, 41 insertions(+), 43 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 61426b3..3d7de4a 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -105,6 +105,47 @@ mwifiex_clean_cmd_node(struct mwifiex_adapter *adapter,
}
/*
+ * This function returns a command to the command free queue.
+ *
+ * The function also calls the completion callback if required, before
+ * cleaning the command node and re-inserting it into the free queue.
+ */
+static void
+mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
+ struct cmd_ctrl_node *cmd_node)
+{
+ unsigned long flags;
+
+ if (!cmd_node)
+ return;
+
+ if (cmd_node->wait_q_enabled)
+ mwifiex_complete_cmd(adapter, cmd_node);
+ /* Clean the node */
+ mwifiex_clean_cmd_node(adapter, cmd_node);
+
+ /* Insert node into cmd_free_q */
+ spin_lock_irqsave(&adapter->cmd_free_q_lock, flags);
+ list_add_tail(&cmd_node->list, &adapter->cmd_free_q);
+ spin_unlock_irqrestore(&adapter->cmd_free_q_lock, flags);
+}
+
+/* This function reuses a command node. */
+void mwifiex_recycle_cmd_node(struct mwifiex_adapter *adapter,
+ struct cmd_ctrl_node *cmd_node)
+{
+ struct host_cmd_ds_command *host_cmd = (void *)cmd_node->cmd_skb->data;
+
+ mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
+
+ atomic_dec(&adapter->cmd_pending);
+ mwifiex_dbg(adapter, CMD,
+ "cmd: FREE_CMD: cmd=%#x, cmd_pending=%d\n",
+ le16_to_cpu(host_cmd->command),
+ atomic_read(&adapter->cmd_pending));
+}
+
+/*
* This function sends a host command to the firmware.
*
* The function copies the host command into the driver command
@@ -614,47 +655,6 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
}
/*
- * This function returns a command to the command free queue.
- *
- * The function also calls the completion callback if required, before
- * cleaning the command node and re-inserting it into the free queue.
- */
-void
-mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
- struct cmd_ctrl_node *cmd_node)
-{
- unsigned long flags;
-
- if (!cmd_node)
- return;
-
- if (cmd_node->wait_q_enabled)
- mwifiex_complete_cmd(adapter, cmd_node);
- /* Clean the node */
- mwifiex_clean_cmd_node(adapter, cmd_node);
-
- /* Insert node into cmd_free_q */
- spin_lock_irqsave(&adapter->cmd_free_q_lock, flags);
- list_add_tail(&cmd_node->list, &adapter->cmd_free_q);
- spin_unlock_irqrestore(&adapter->cmd_free_q_lock, flags);
-}
-
-/* This function reuses a command node. */
-void mwifiex_recycle_cmd_node(struct mwifiex_adapter *adapter,
- struct cmd_ctrl_node *cmd_node)
-{
- struct host_cmd_ds_command *host_cmd = (void *)cmd_node->cmd_skb->data;
-
- mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
-
- atomic_dec(&adapter->cmd_pending);
- mwifiex_dbg(adapter, CMD,
- "cmd: FREE_CMD: cmd=%#x, cmd_pending=%d\n",
- le16_to_cpu(host_cmd->command),
- atomic_read(&adapter->cmd_pending));
-}
-
-/*
* This function queues a command to the command pending queue.
*
* This in effect adds the command to the command list to be executed.
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index f71f894..840e5d4 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1045,8 +1045,6 @@ void mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter);
void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
void mwifiex_cancel_pending_scan_cmd(struct mwifiex_adapter *adapter);
-void mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
- struct cmd_ctrl_node *cmd_node);
void mwifiex_recycle_cmd_node(struct mwifiex_adapter *adapter,
struct cmd_ctrl_node *cmd_node);
--
2.7.0
Signed-off-by: Andreas Fenkart <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/scan.c | 52 +++++++++++++----------------
1 file changed, 23 insertions(+), 29 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index a7bdde5..ed886af 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -76,6 +76,18 @@ static u8 mwifiex_rsn_oui[CIPHER_SUITE_MAX][4] = {
{ 0x00, 0x0f, 0xac, 0x04 }, /* AES */
};
+static bool
+has_ieee_hdr(struct ieee_types_generic *ie, u8 key)
+{
+ return (ie && ie->ieee_hdr.element_id == key);
+}
+
+static bool
+has_vendor_hdr(struct ieee_types_vendor_specific *ie, u8 key)
+{
+ return (ie && ie->vend_hdr.element_id == key);
+}
+
/*
* This function parses a given IE for a given OUI.
*
@@ -121,8 +133,7 @@ mwifiex_is_rsn_oui_present(struct mwifiex_bssdescriptor *bss_desc, u32 cipher)
struct ie_body *iebody;
u8 ret = MWIFIEX_OUI_NOT_PRESENT;
- if (bss_desc->bcn_rsn_ie &&
- bss_desc->bcn_rsn_ie->ieee_hdr.element_id == WLAN_EID_RSN) {
+ if (has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN)) {
iebody = (struct ie_body *)
(((u8 *) bss_desc->bcn_rsn_ie->data) +
RSN_GTK_OUI_OFFSET);
@@ -148,9 +159,7 @@ mwifiex_is_wpa_oui_present(struct mwifiex_bssdescriptor *bss_desc, u32 cipher)
struct ie_body *iebody;
u8 ret = MWIFIEX_OUI_NOT_PRESENT;
- if (bss_desc->bcn_wpa_ie &&
- bss_desc->bcn_wpa_ie->vend_hdr.element_id ==
- WLAN_EID_VENDOR_SPECIFIC) {
+ if (has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC)) {
iebody = (struct ie_body *) bss_desc->bcn_wpa_ie->data;
oui = &mwifiex_wpa_oui[cipher][0];
ret = mwifiex_search_oui_in_ie(iebody, oui);
@@ -180,11 +189,8 @@ mwifiex_is_bss_wapi(struct mwifiex_private *priv,
struct mwifiex_bssdescriptor *bss_desc)
{
if (priv->sec_info.wapi_enabled &&
- (bss_desc->bcn_wapi_ie &&
- bss_desc->bcn_wapi_ie->ieee_hdr.element_id ==
- WLAN_EID_BSS_AC_ACCESS_DELAY)) {
+ has_ieee_hdr(bss_desc->bcn_wapi_ie, WLAN_EID_BSS_AC_ACCESS_DELAY))
return true;
- }
return false;
}
@@ -198,11 +204,8 @@ mwifiex_is_bss_no_sec(struct mwifiex_private *priv,
{
if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
!priv->sec_info.wpa2_enabled &&
- (!bss_desc->bcn_wpa_ie ||
- bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
- WLAN_EID_VENDOR_SPECIFIC) &&
- (!bss_desc->bcn_rsn_ie ||
- bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN) &&
+ !has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC) &&
+ !has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN) &&
!priv->sec_info.encryption_mode && !bss_desc->privacy) {
return true;
}
@@ -234,9 +237,7 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
{
if (!priv->sec_info.wep_enabled && priv->sec_info.wpa_enabled &&
!priv->sec_info.wpa2_enabled &&
- (bss_desc->bcn_wpa_ie &&
- bss_desc->bcn_wpa_ie->vend_hdr.element_id ==
- WLAN_EID_VENDOR_SPECIFIC)
+ has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC)
/*
* Privacy bit may NOT be set in some APs like
* LinkSys WRT54G && bss_desc->privacy
@@ -270,8 +271,7 @@ mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
{
if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
priv->sec_info.wpa2_enabled &&
- (bss_desc->bcn_rsn_ie &&
- bss_desc->bcn_rsn_ie->ieee_hdr.element_id == WLAN_EID_RSN)) {
+ has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN)) {
/*
* Privacy bit may NOT be set in some APs like
* LinkSys WRT54G && bss_desc->privacy
@@ -304,11 +304,8 @@ mwifiex_is_bss_adhoc_aes(struct mwifiex_private *priv,
{
if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
!priv->sec_info.wpa2_enabled &&
- (!bss_desc->bcn_wpa_ie ||
- bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
- WLAN_EID_VENDOR_SPECIFIC) &&
- (!bss_desc->bcn_rsn_ie ||
- bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN) &&
+ !has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC) &&
+ !has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN) &&
!priv->sec_info.encryption_mode && bss_desc->privacy) {
return true;
}
@@ -325,11 +322,8 @@ mwifiex_is_bss_dynamic_wep(struct mwifiex_private *priv,
{
if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
!priv->sec_info.wpa2_enabled &&
- (!bss_desc->bcn_wpa_ie ||
- bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
- WLAN_EID_VENDOR_SPECIFIC) &&
- (!bss_desc->bcn_rsn_ie ||
- bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN) &&
+ !has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC) &&
+ !has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN) &&
priv->sec_info.encryption_mode && bss_desc->privacy) {
mwifiex_dbg(priv->adapter, INFO,
"info: %s: dynamic\t"
--
2.7.0
> given this structure:
> struct foo {
> struct bar {
> int baz;
> }
> }
>
> these accesses are equivalent:
> (*(foo->bar)).baz
> foo->bar->baz
>
> Signed-off-by: Andreas Fenkart <[email protected]>
Thanks, 7 patches applied to wireless-drivers-next.git:
0a233c98a8e9 mwifiex: scan: simplify dereference of bss_desc fields
38329568c3f7 mwifiex: scan: factor out has_ieee_hdr/has_vendor_hdr
2ccf7cef0cf9 mwifiex: scan: simplify ternary operators using gnu extension
679b687bc96d mwifiex: scan: factor out dbg_security_flags
948ad6b34943 mwifiex: scan: replace pointer arithmetic with array access
c70ca8cb9a7c mwifiex: factor out mwifiex_cancel_pending_scan_cmd
85abfb1239ec mwifiex: make mwifiex_insert_cmd_to_free_q local static
Kalle Valo