2022-04-18 04:54:21

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 0/6] staging: r8188eu: clean up mgt_dispatcher

Clean up the mgt_dispatcher function and remove unnecessary info from the
mlme_sta_tbl array.

Martin Kaiser (6):
staging: r8188eu: check receiver address only once
staging: r8188eu: replace the GetFrameSubType call
staging: r8188eu: the frame type is shifted out
staging: r8188eu: replace mlme_handler with function pointer
staging: r8188eu: don't call empty DoReserved function
staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl

drivers/staging/r8188eu/core/rtw_mlme_ext.c | 60 ++++++++-----------
.../staging/r8188eu/include/rtw_mlme_ext.h | 6 +-
2 files changed, 25 insertions(+), 41 deletions(-)

--
2.30.2


2022-04-18 08:16:45

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl

Use ARRAY_SIZE instead of hard-coding the number of entries in the
mlme_sta_tbl array.

Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index abb910f33c1c..973adebdd69c 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -404,7 +404,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
return;

index = (le16_to_cpu(hdr->frame_control) & IEEE80211_FCTL_STYPE) >> 4;
- if (index > 13)
+ if (index > ARRAY_SIZE(mlme_sta_tbl))
return;
fct = mlme_sta_tbl[index];

--
2.30.2

2022-04-18 12:37:31

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 2/6] staging: r8188eu: replace the GetFrameSubType call

The driver's local GetFrameSubType macro returns both frame type and
subtype.

Use the ieee80211 framework to extract the two fields. This shows more
clearly that both type and subtype are read.

Convert everything to host endianness before we use bit operations.

Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 3afd06120cb1..be1afabe77d1 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -395,7 +395,6 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
int index;
struct mlme_handler *ptable;
struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
- u8 *pframe = precv_frame->rx_data;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)precv_frame->rx_data;
struct sta_info *psta = rtw_get_stainfo(&padapter->stapriv, hdr->addr2);

@@ -409,8 +408,8 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)

ptable = mlme_sta_tbl;

- index = GetFrameSubType(pframe) >> 4;
-
+ index = (le16_to_cpu(hdr->frame_control) &
+ (IEEE80211_FCTL_STYPE | IEEE80211_FCTL_FTYPE)) >> 4;
if (index > 13)
return;
ptable += index;
--
2.30.2

2022-04-18 12:38:21

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 1/6] staging: r8188eu: check receiver address only once

Check only once in mgt_dispatcher that the receiver address is the local
address or the broadcast address. The second identical check can be
removed.

Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index b6ee6a24930a..3afd06120cb1 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -431,13 +431,8 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
ptable->func = &OnAuthClient;
}

- if (ptable->func) {
- /* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
- if (memcmp(hdr->addr1, myid(&padapter->eeprompriv), ETH_ALEN) &&
- !is_broadcast_ether_addr(hdr->addr1))
- return;
+ if (ptable->func)
ptable->func(padapter, precv_frame);
- }
}

static u32 p2p_listen_state_process(struct adapter *padapter, unsigned char *da)
--
2.30.2

2022-04-22 18:15:59

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl

On Sun, Apr 17, 2022 at 12:22:21PM +0200, Martin Kaiser wrote:
> Use ARRAY_SIZE instead of hard-coding the number of entries in the
> mlme_sta_tbl array.
>
> Signed-off-by: Martin Kaiser <[email protected]>
> ---
> drivers/staging/r8188eu/core/rtw_mlme_ext.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> index abb910f33c1c..973adebdd69c 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> @@ -404,7 +404,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
> return;
>
> index = (le16_to_cpu(hdr->frame_control) & IEEE80211_FCTL_STYPE) >> 4;
> - if (index > 13)
> + if (index > ARRAY_SIZE(mlme_sta_tbl))
^
This is an off by one. Should be >=. The auto builders would have
caught this eventually.

> return;
> fct = mlme_sta_tbl[index];

regards,
dan carpenter

2022-04-22 19:12:04

by Martin Kaiser

[permalink] [raw]
Subject: Re: [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl

Thus wrote Dan Carpenter ([email protected]):

> On Sun, Apr 17, 2022 at 12:22:21PM +0200, Martin Kaiser wrote:
> > Use ARRAY_SIZE instead of hard-coding the number of entries in the
> > mlme_sta_tbl array.

> > Signed-off-by: Martin Kaiser <[email protected]>
> > ---
> > drivers/staging/r8188eu/core/rtw_mlme_ext.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)

> > diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> > index abb910f33c1c..973adebdd69c 100644
> > --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> > @@ -404,7 +404,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
> > return;

> > index = (le16_to_cpu(hdr->frame_control) & IEEE80211_FCTL_STYPE) >> 4;
> > - if (index > 13)
> > + if (index > ARRAY_SIZE(mlme_sta_tbl))
> ^
> This is an off by one. Should be >=. The auto builders would have
> caught this eventually.

Thanks for spotting this, Dan. You're right, the array has 14 elements :-(

This has already made it into staging-next, I'll fix it in a new patch.

Best regards,
Martin

> > return;
> > fct = mlme_sta_tbl[index];

> regards,
> dan carpenter