[PATCH v2 00/13] staging: r8188eu: clean up action frame handlers

[PATCH v2 00/13] staging: r8188eu: clean up action frame handlers

[Martin Kaiser]

Clean up the handlers for action frames. Summarize common code, remove
unnecessary return values.

Please apply this on top of the "remove the last get_da calls" series.

Changes in v2
- remove on_action_public's ret variable in patch 3, not in patch 9

Martin Kaiser (13):
  staging: r8188eu: replace a GetAddr1Ptr call
  staging: r8188eu: remove duplicate category check
  staging: r8188eu: make on_action_public static void
  staging: r8188eu: make OnAction_back static void
  staging: r8188eu: make OnAction_p2p static void
  staging: r8188eu: remove category check in OnAction_p2p
  staging: r8188eu: replace switch-case with if
  staging: r8188eu: replace GetAddr1Ptr call in OnAction_p2p
  staging: r8188eu: clean up on_action_public
  staging: r8188eu: remove return value from on_action_public_vendor
  staging: r8188eu: remove return value from on_action_public_default
  staging: r8188eu: rtw_action_public_decache's token is a u8
  staging: r8188eu: check destination address in OnAction

 drivers/staging/r8188eu/core/rtw_mlme_ext.c   | 116 +++++-------------
 .../staging/r8188eu/include/rtw_mlme_ext.h    |   7 --
 2 files changed, 28 insertions(+), 95 deletions(-)

-- 
2.30.2

[PATCH v2 01/13] staging: r8188eu: replace a GetAddr1Ptr call

[Martin Kaiser]

Define a struct ieee80211_mgmt and use it to read the destination address.

This replaces one call to the driver-specific GetAddr1Ptr function, which
should eventually be removed.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 127dbc4e8b9a..5a366688a3f7 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3815,13 +3815,14 @@ static unsigned int on_action_public_default(struct recv_frame *precv_frame)
 
 unsigned int on_action_public(struct adapter *padapter, struct recv_frame *precv_frame)
 {
+	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)precv_frame->rx_data;
 	unsigned int ret = _FAIL;
 	u8 *pframe = precv_frame->rx_data;
 	u8 *frame_body = pframe + sizeof(struct ieee80211_hdr_3addr);
 	u8 category, action;
 
 	/* check RA matches or not */
-	if (memcmp(myid(&padapter->eeprompriv), GetAddr1Ptr(pframe), ETH_ALEN))
+	if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))
 		goto exit;
 
 	category = frame_body[0];
-- 
2.30.2

[PATCH v2 02/13] staging: r8188eu: remove duplicate category check

[Martin Kaiser]

The caller of on_action_public has already checked the action category. We
can remove the check in on_action_public.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 5a366688a3f7..7d4f208d161b 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3819,16 +3819,12 @@ unsigned int on_action_public(struct adapter *padapter, struct recv_frame *precv
 	unsigned int ret = _FAIL;
 	u8 *pframe = precv_frame->rx_data;
 	u8 *frame_body = pframe + sizeof(struct ieee80211_hdr_3addr);
-	u8 category, action;
+	u8 action;
 
 	/* check RA matches or not */
 	if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))
 		goto exit;
 
-	category = frame_body[0];
-	if (category != WLAN_CATEGORY_PUBLIC)
-		goto exit;
-
 	action = frame_body[1];
 	switch (action) {
 	case ACT_PUBLIC_VENDOR:
-- 
2.30.2

[PATCH v2 03/13] staging: r8188eu: make on_action_public static void

[Martin Kaiser]

The on_action_public function is called only by OnAction. This function
also lives in rtw_mlme_ext.c and does not check the return value from
on_action_public.

We can make on_action_public a static void function.

The ret variable is no longer needed if we don't return a value. It can
be removed.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
Changes in v2
- remove ret variable, it was written but never read

 drivers/staging/r8188eu/core/rtw_mlme_ext.c    | 12 ++++--------
 drivers/staging/r8188eu/include/rtw_mlme_ext.h |  2 --
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 7d4f208d161b..88600f62ffb4 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3813,30 +3813,26 @@ static unsigned int on_action_public_default(struct recv_frame *precv_frame)
 	return ret;
 }
 
-unsigned int on_action_public(struct adapter *padapter, struct recv_frame *precv_frame)
+static void on_action_public(struct adapter *padapter, struct recv_frame *precv_frame)
 {
 	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)precv_frame->rx_data;
-	unsigned int ret = _FAIL;
 	u8 *pframe = precv_frame->rx_data;
 	u8 *frame_body = pframe + sizeof(struct ieee80211_hdr_3addr);
 	u8 action;
 
 	/* check RA matches or not */
 	if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))
-		goto exit;
+		return;
 
 	action = frame_body[1];
 	switch (action) {
 	case ACT_PUBLIC_VENDOR:
-		ret = on_action_public_vendor(precv_frame);
+		on_action_public_vendor(precv_frame);
 		break;
 	default:
-		ret = on_action_public_default(precv_frame);
+		on_action_public_default(precv_frame);
 		break;
 	}
-
-exit:
-	return ret;
 }
 
 unsigned int OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_frame)
diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
index c8beaa927cba..ec2e9352011b 100644
--- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
@@ -538,8 +538,6 @@ void start_create_ibss(struct adapter *padapter);
 
 unsigned int OnAction_back(struct adapter *padapter,
 			   struct recv_frame *precv_frame);
-unsigned int on_action_public(struct adapter *padapter,
-			      struct recv_frame *precv_frame);
 unsigned int OnAction_p2p(struct adapter *padapter,
 			  struct recv_frame *precv_frame);
 
-- 
2.30.2

[PATCH v2 04/13] staging: r8188eu: make OnAction_back static void

[Martin Kaiser]

OnAction_back is called only by OnAction, its return value is not checked.
We can make it a static void function.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c    | 13 +++++--------
 drivers/staging/r8188eu/include/rtw_mlme_ext.h |  2 --
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 88600f62ffb4..779c022b1c50 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -1481,7 +1481,7 @@ static void OnDisassoc(struct adapter *padapter, struct recv_frame *precv_frame)
 	pmlmepriv->LinkDetectInfo.bBusyTraffic = false;
 }
 
-unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_frame)
+static void OnAction_back(struct adapter *padapter, struct recv_frame *precv_frame)
 {
 	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)precv_frame->rx_data;
 	struct sta_info *psta = NULL;
@@ -1494,21 +1494,20 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
 	struct sta_priv *pstapriv = &padapter->stapriv;
 	/* check RA matches or not */
 	if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))/* for if1, sta/ap mode */
-		return _SUCCESS;
+		return;
 
 	if ((pmlmeinfo->state & 0x03) != WIFI_FW_AP_STATE)
 		if (!(pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS))
-			return _SUCCESS;
+			return;
 
 	psta = rtw_get_stainfo(pstapriv, mgmt->sa);
-
 	if (!psta)
-		return _SUCCESS;
+		return;
 
 	frame_body = (unsigned char *)(pframe + sizeof(struct ieee80211_hdr_3addr));
 
 	if (!pmlmeinfo->HT_enable)
-		return _SUCCESS;
+		return;
 	/* All union members start with an action code, it's ok to use addba_req. */
 	switch (mgmt->u.action.u.addba_req.action_code) {
 	case WLAN_ACTION_ADDBA_REQ:
@@ -1550,8 +1549,6 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
 	default:
 		break;
 	}
-
-	return _SUCCESS;
 }
 
 static int get_reg_classes_full_count(struct p2p_channels *channel_list)
diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
index ec2e9352011b..4ccdce1ad9be 100644
--- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
@@ -536,8 +536,6 @@ void start_clnt_auth(struct adapter *padapter);
 void start_clnt_join(struct adapter *padapter);
 void start_create_ibss(struct adapter *padapter);
 
-unsigned int OnAction_back(struct adapter *padapter,
-			   struct recv_frame *precv_frame);
 unsigned int OnAction_p2p(struct adapter *padapter,
 			  struct recv_frame *precv_frame);
 
-- 
2.30.2

[PATCH v2 05/13] staging: r8188eu: make OnAction_p2p static void

[Martin Kaiser]

OnAction_p2p is called only by OnAction, its return value is not checked.
We can make it a static void function.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c    | 9 ++++-----
 drivers/staging/r8188eu/include/rtw_mlme_ext.h | 3 ---
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 779c022b1c50..f1054192bfb1 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3832,7 +3832,7 @@ static void on_action_public(struct adapter *padapter, struct recv_frame *precv_
 	}
 }
 
-unsigned int OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_frame)
+static void OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_frame)
 {
 	u8 *frame_body;
 	u8 category, OUI_Subtype;
@@ -3842,16 +3842,16 @@ unsigned int OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_fra
 
 	/* check RA matches or not */
 	if (memcmp(myid(&padapter->eeprompriv), GetAddr1Ptr(pframe), ETH_ALEN))/* for if1, sta/ap mode */
-		return _SUCCESS;
+		return;
 
 	frame_body = (unsigned char *)(pframe + sizeof(struct ieee80211_hdr_3addr));
 
 	category = frame_body[0];
 	if (category != RTW_WLAN_CATEGORY_P2P)
-		return _SUCCESS;
+		return;
 
 	if (be32_to_cpu(*((__be32 *)(frame_body + 1))) != P2POUI)
-		return _SUCCESS;
+		return;
 
 	len -= sizeof(struct ieee80211_hdr_3addr);
 	OUI_Subtype = frame_body[5];
@@ -3869,7 +3869,6 @@ unsigned int OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_fra
 	default:
 		break;
 	}
-	return _SUCCESS;
 }
 
 static void OnAction(struct adapter *padapter, struct recv_frame *precv_frame)
diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
index 4ccdce1ad9be..ce5b57e23e53 100644
--- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
@@ -536,9 +536,6 @@ void start_clnt_auth(struct adapter *padapter);
 void start_clnt_join(struct adapter *padapter);
 void start_create_ibss(struct adapter *padapter);
 
-unsigned int OnAction_p2p(struct adapter *padapter,
-			  struct recv_frame *precv_frame);
-
 void mlmeext_joinbss_event_callback(struct adapter *padapter, int join_res);
 void mlmeext_sta_del_event_callback(struct adapter *padapter);
 void mlmeext_sta_add_event_callback(struct adapter *padapter,
-- 
2.30.2

[PATCH v2 06/13] staging: r8188eu: remove category check in OnAction_p2p

[Martin Kaiser]

The caller of OnAction_p2p has already checked the action category. We can
remove the check in OnAction_p2p.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index f1054192bfb1..efcb2f3b6d3f 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3835,7 +3835,7 @@ static void on_action_public(struct adapter *padapter, struct recv_frame *precv_
 static void OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_frame)
 {
 	u8 *frame_body;
-	u8 category, OUI_Subtype;
+	u8 OUI_Subtype;
 	u8 *pframe = precv_frame->rx_data;
 	uint len = precv_frame->len;
 	struct	wifidirect_info	*pwdinfo = &padapter->wdinfo;
@@ -3846,10 +3846,6 @@ static void OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_fram
 
 	frame_body = (unsigned char *)(pframe + sizeof(struct ieee80211_hdr_3addr));
 
-	category = frame_body[0];
-	if (category != RTW_WLAN_CATEGORY_P2P)
-		return;
-
 	if (be32_to_cpu(*((__be32 *)(frame_body + 1))) != P2POUI)
 		return;
 
-- 
2.30.2

[PATCH v2 07/13] staging: r8188eu: replace switch-case with if

[Martin Kaiser]

OnAction_p2p has a switch-case statement where only a single case is
handled. Use if instead, this makes the code shorter and easier to read.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index efcb2f3b6d3f..64d01da9c814 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3852,19 +3852,8 @@ static void OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_fram
 	len -= sizeof(struct ieee80211_hdr_3addr);
 	OUI_Subtype = frame_body[5];
 
-	switch (OUI_Subtype) {
-	case P2P_NOTICE_OF_ABSENCE:
-		break;
-	case P2P_PRESENCE_REQUEST:
+	if (OUI_Subtype == P2P_PRESENCE_REQUEST)
 		process_p2p_presence_req(pwdinfo, pframe, len);
-		break;
-	case P2P_PRESENCE_RESPONSE:
-		break;
-	case P2P_GO_DISC_REQUEST:
-		break;
-	default:
-		break;
-	}
 }
 
 static void OnAction(struct adapter *padapter, struct recv_frame *precv_frame)
-- 
2.30.2

[PATCH v2 08/13] staging: r8188eu: replace GetAddr1Ptr call in OnAction_p2p

[Martin Kaiser]

Define a struct ieee80211_mgmt in OnAction_p2p and use it to check the
destination address. This replaces a call to the driver-specific
GetAddr1Ptr function.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 64d01da9c814..6d95d3bc23e6 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3834,6 +3834,7 @@ static void on_action_public(struct adapter *padapter, struct recv_frame *precv_
 
 static void OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_frame)
 {
+	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)precv_frame->rx_data;
 	u8 *frame_body;
 	u8 OUI_Subtype;
 	u8 *pframe = precv_frame->rx_data;
@@ -3841,7 +3842,7 @@ static void OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_fram
 	struct	wifidirect_info	*pwdinfo = &padapter->wdinfo;
 
 	/* check RA matches or not */
-	if (memcmp(myid(&padapter->eeprompriv), GetAddr1Ptr(pframe), ETH_ALEN))/* for if1, sta/ap mode */
+	if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))/* for if1, sta/ap mode */
 		return;
 
 	frame_body = (unsigned char *)(pframe + sizeof(struct ieee80211_hdr_3addr));
-- 
2.30.2

[PATCH v2 09/13] staging: r8188eu: clean up on_action_public

[Martin Kaiser]

Use the struct mgmt to read the action_code. This is much simpler than
parsing the message ourselves.

Add a comment about reading the action code. All members of the action
enum start with an action_code byte. It does not matter which member we
use.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
Changes in v2
- don't remove the ret variable, it's already gone

 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 6d95d3bc23e6..b3cef3504ad3 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3813,23 +3813,16 @@ static unsigned int on_action_public_default(struct recv_frame *precv_frame)
 static void on_action_public(struct adapter *padapter, struct recv_frame *precv_frame)
 {
 	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)precv_frame->rx_data;
-	u8 *pframe = precv_frame->rx_data;
-	u8 *frame_body = pframe + sizeof(struct ieee80211_hdr_3addr);
-	u8 action;
 
 	/* check RA matches or not */
 	if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))
 		return;
 
-	action = frame_body[1];
-	switch (action) {
-	case ACT_PUBLIC_VENDOR:
+	/* All members of the action enum start with action_code. */
+	if (mgmt->u.action.u.s1g.action_code == WLAN_PUB_ACTION_VENDOR_SPECIFIC)
 		on_action_public_vendor(precv_frame);
-		break;
-	default:
+	else
 		on_action_public_default(precv_frame);
-		break;
-	}
 }
 
 static void OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_frame)
-- 
2.30.2

[PATCH v2 10/13] staging: r8188eu: remove return value from on_action_public_vendor

[Martin Kaiser]

The only caller of on_action_public_vendor does not check the return
value. We can make it a void function.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index b3cef3504ad3..b395457a6a60 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3779,17 +3779,13 @@ static unsigned int on_action_public_p2p(struct recv_frame *precv_frame)
 	return _SUCCESS;
 }
 
-static unsigned int on_action_public_vendor(struct recv_frame *precv_frame)
+static void on_action_public_vendor(struct recv_frame *precv_frame)
 {
-	unsigned int ret = _FAIL;
 	u8 *pframe = precv_frame->rx_data;
 	u8 *frame_body = pframe + sizeof(struct ieee80211_hdr_3addr);
 
-	if (!memcmp(frame_body + 2, P2P_OUI, 4)) {
-		ret = on_action_public_p2p(precv_frame);
-	}
-
-	return ret;
+	if (!memcmp(frame_body + 2, P2P_OUI, 4))
+		on_action_public_p2p(precv_frame);
 }
 
 static unsigned int on_action_public_default(struct recv_frame *precv_frame)
-- 
2.30.2

[PATCH v2 11/13] staging: r8188eu: remove return value from on_action_public_default

[Martin Kaiser]

The only caller of on_action_public_default does not check the return
value. We can make it a void function.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index b395457a6a60..f5923792f067 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3788,22 +3788,15 @@ static void on_action_public_vendor(struct recv_frame *precv_frame)
 		on_action_public_p2p(precv_frame);
 }
 
-static unsigned int on_action_public_default(struct recv_frame *precv_frame)
+static void on_action_public_default(struct recv_frame *precv_frame)
 {
-	unsigned int ret = _FAIL;
 	u8 *pframe = precv_frame->rx_data;
 	u8 *frame_body = pframe + sizeof(struct ieee80211_hdr_3addr);
 	u8 token;
 
 	token = frame_body[2];
 
-	if (rtw_action_public_decache(precv_frame, token) == _FAIL)
-		goto exit;
-
-	ret = _SUCCESS;
-
-exit:
-	return ret;
+	rtw_action_public_decache(precv_frame, token);
 }
 
 static void on_action_public(struct adapter *padapter, struct recv_frame *precv_frame)
-- 
2.30.2

[PATCH v2 12/13] staging: r8188eu: rtw_action_public_decache's token is a u8

[Martin Kaiser]

Both callers of rtw_action_public_decache pass a u8 value for the token
parameter. We can change token from s32 to u8 and remove the code for
token < 0.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index f5923792f067..93f3d387e92d 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3490,7 +3490,7 @@ inline void issue_probereq_p2p(struct adapter *adapter, u8 *da)
 	_issue_probereq_p2p(adapter, da);
 }
 
-static s32 rtw_action_public_decache(struct recv_frame *recv_frame, s32 token)
+static s32 rtw_action_public_decache(struct recv_frame *recv_frame, u8 token)
 {
 	struct adapter *adapter = recv_frame->adapter;
 	struct mlme_ext_priv *mlmeext = &adapter->mlmeextpriv;
@@ -3499,21 +3499,13 @@ static s32 rtw_action_public_decache(struct recv_frame *recv_frame, s32 token)
 		(recv_frame->attrib.frag_num & 0xf);
 
 	if (GetRetry(frame)) {
-		if (token >= 0) {
-			if ((seq_ctrl == mlmeext->action_public_rxseq) &&
-			    (token == mlmeext->action_public_dialog_token))
-				return _FAIL;
-		} else {
-			if (seq_ctrl == mlmeext->action_public_rxseq)
-				return _FAIL;
-		}
+		if ((seq_ctrl == mlmeext->action_public_rxseq) &&
+		    (token == mlmeext->action_public_dialog_token))
+			return _FAIL;
 	}
 
 	mlmeext->action_public_rxseq = seq_ctrl;
-
-	if (token >= 0)
-		mlmeext->action_public_dialog_token = token;
-
+	mlmeext->action_public_dialog_token = token;
 	return _SUCCESS;
 }
 
-- 
2.30.2

[PATCH v2 13/13] staging: r8188eu: check destination address in OnAction

[Martin Kaiser]

All subfunctions of OnAction check if the destination address matches the
local interface's address. It's simpler to move this check to OnAction.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 93f3d387e92d..e985fc5fc575 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -1492,9 +1492,6 @@ static void OnAction_back(struct adapter *padapter, struct recv_frame *precv_fra
 	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
 	u8 *pframe = precv_frame->rx_data;
 	struct sta_priv *pstapriv = &padapter->stapriv;
-	/* check RA matches or not */
-	if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))/* for if1, sta/ap mode */
-		return;
 
 	if ((pmlmeinfo->state & 0x03) != WIFI_FW_AP_STATE)
 		if (!(pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS))
@@ -3795,10 +3792,6 @@ static void on_action_public(struct adapter *padapter, struct recv_frame *precv_
 {
 	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)precv_frame->rx_data;
 
-	/* check RA matches or not */
-	if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))
-		return;
-
 	/* All members of the action enum start with action_code. */
 	if (mgmt->u.action.u.s1g.action_code == WLAN_PUB_ACTION_VENDOR_SPECIFIC)
 		on_action_public_vendor(precv_frame);
@@ -3808,17 +3801,12 @@ static void on_action_public(struct adapter *padapter, struct recv_frame *precv_
 
 static void OnAction_p2p(struct adapter *padapter, struct recv_frame *precv_frame)
 {
-	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)precv_frame->rx_data;
 	u8 *frame_body;
 	u8 OUI_Subtype;
 	u8 *pframe = precv_frame->rx_data;
 	uint len = precv_frame->len;
 	struct	wifidirect_info	*pwdinfo = &padapter->wdinfo;
 
-	/* check RA matches or not */
-	if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))/* for if1, sta/ap mode */
-		return;
-
 	frame_body = (unsigned char *)(pframe + sizeof(struct ieee80211_hdr_3addr));
 
 	if (be32_to_cpu(*((__be32 *)(frame_body + 1))) != P2POUI)
@@ -3835,6 +3823,9 @@ static void OnAction(struct adapter *padapter, struct recv_frame *precv_frame)
 {
 	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)precv_frame->rx_data;
 
+	if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))
+		return;
+
 	switch (mgmt->u.action.category) {
 	case WLAN_CATEGORY_BACK:
 		OnAction_back(padapter, precv_frame);
-- 
2.30.2

Re: [PATCH v2 00/13] staging: r8188eu: clean up action frame handlers

[Philipp Hortmann]

On 10/30/22 18:33, Martin Kaiser wrote:
> Clean up the handlers for action frames. Summarize common code, remove
> unnecessary return values.
> 
> Please apply this on top of the "remove the last get_da calls" series.
> 
> Changes in v2
> - remove on_action_public's ret variable in patch 3, not in patch 9
> 
> Martin Kaiser (13):
>    staging: r8188eu: replace a GetAddr1Ptr call
>    staging: r8188eu: remove duplicate category check
>    staging: r8188eu: make on_action_public static void
>    staging: r8188eu: make OnAction_back static void
>    staging: r8188eu: make OnAction_p2p static void
>    staging: r8188eu: remove category check in OnAction_p2p
>    staging: r8188eu: replace switch-case with if
>    staging: r8188eu: replace GetAddr1Ptr call in OnAction_p2p
>    staging: r8188eu: clean up on_action_public
>    staging: r8188eu: remove return value from on_action_public_vendor
>    staging: r8188eu: remove return value from on_action_public_default
>    staging: r8188eu: rtw_action_public_decache's token is a u8
>    staging: r8188eu: check destination address in OnAction
> 
>   drivers/staging/r8188eu/core/rtw_mlme_ext.c   | 116 +++++-------------
>   .../staging/r8188eu/include/rtw_mlme_ext.h    |   7 --
>   2 files changed, 28 insertions(+), 95 deletions(-)
> 

Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> # Edimax N150

Re: [PATCH v2 00/13] staging: r8188eu: clean up action frame handlers

[Martin Kaiser]

Thus wrote Philipp Hortmann (philipp.g.hortmann@gmail.com):

> On 10/30/22 18:33, Martin Kaiser wrote:
> > Clean up the handlers for action frames. Summarize common code, remove
> > unnecessary return values.

> > Please apply this on top of the "remove the last get_da calls" series.

> > Changes in v2
> > - remove on_action_public's ret variable in patch 3, not in patch 9

> > Martin Kaiser (13):
> >    staging: r8188eu: replace a GetAddr1Ptr call
> >    staging: r8188eu: remove duplicate category check
> >    staging: r8188eu: make on_action_public static void
> >    staging: r8188eu: make OnAction_back static void
> >    staging: r8188eu: make OnAction_p2p static void
> >    staging: r8188eu: remove category check in OnAction_p2p
> >    staging: r8188eu: replace switch-case with if
> >    staging: r8188eu: replace GetAddr1Ptr call in OnAction_p2p
> >    staging: r8188eu: clean up on_action_public
> >    staging: r8188eu: remove return value from on_action_public_vendor
> >    staging: r8188eu: remove return value from on_action_public_default
> >    staging: r8188eu: rtw_action_public_decache's token is a u8
> >    staging: r8188eu: check destination address in OnAction

> >   drivers/staging/r8188eu/core/rtw_mlme_ext.c   | 116 +++++-------------
> >   .../staging/r8188eu/include/rtw_mlme_ext.h    |   7 --
> >   2 files changed, 28 insertions(+), 95 deletions(-)


> Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> # Edimax N150

Thanks, Philipp.

I just realized that I forgot to add your Tested-by from the previous
version. The final code hasn't changed in v2, only the intermediate
steps were fixed.

Martin

Re: [PATCH v2 00/13] staging: r8188eu: clean up action frame handlers

[Philipp Hortmann]

On 10/30/22 19:48, Martin Kaiser wrote:
> Thus wrote Philipp Hortmann (philipp.g.hortmann@gmail.com):
> 
>> On 10/30/22 18:33, Martin Kaiser wrote:
>>> Clean up the handlers for action frames. Summarize common code, remove
>>> unnecessary return values.
> 
>>> Please apply this on top of the "remove the last get_da calls" series.
> 
>>> Changes in v2
>>> - remove on_action_public's ret variable in patch 3, not in patch 9
> 
>>> Martin Kaiser (13):
>>>     staging: r8188eu: replace a GetAddr1Ptr call
>>>     staging: r8188eu: remove duplicate category check
>>>     staging: r8188eu: make on_action_public static void
>>>     staging: r8188eu: make OnAction_back static void
>>>     staging: r8188eu: make OnAction_p2p static void
>>>     staging: r8188eu: remove category check in OnAction_p2p
>>>     staging: r8188eu: replace switch-case with if
>>>     staging: r8188eu: replace GetAddr1Ptr call in OnAction_p2p
>>>     staging: r8188eu: clean up on_action_public
>>>     staging: r8188eu: remove return value from on_action_public_vendor
>>>     staging: r8188eu: remove return value from on_action_public_default
>>>     staging: r8188eu: rtw_action_public_decache's token is a u8
>>>     staging: r8188eu: check destination address in OnAction
> 
>>>    drivers/staging/r8188eu/core/rtw_mlme_ext.c   | 116 +++++-------------
>>>    .../staging/r8188eu/include/rtw_mlme_ext.h    |   7 --
>>>    2 files changed, 28 insertions(+), 95 deletions(-)
> 
> 
>> Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> # Edimax N150
> 
> Thanks, Philipp.
> 
> I just realized that I forgot to add your Tested-by from the previous
> version. The final code hasn't changed in v2, only the intermediate
> steps were fixed.
> 
> Martin

Hi Martin,

thanks for the offer but I prefer to test each new version by myself.

I do trust you a lot but mistakes can always happen.

Bye Philipp