[PATCH v6 0/2] Cleanup duplicate NULL tests on a value

[PATCH v6 0/2] Cleanup duplicate NULL tests on a value

[Abraham Samuel Adekunle]

Changes in v5:
	- Splitted patch v4 into a patchset with two patches so as to
	 proper explanation of each change due to different code patterns
	 in the change as suggested by Julia Lawall.
	 - Modified commit messages in each patch to properly explain the changes
	 show the initial NULL tests on the value.
	 - Added Subject title to each patch to be specific to the value being NULL tested.
Changed in v4:
	- Separated initially integrated suggested change
	 "use modulo % 4096 over & 0xfff" to a different patch.
Changes in v3:
	- Changed other cases to use modulo (% 4096) over (& 0xofff).
	- Modified commit message to reflect these changes.
Changes in v2:
	- Dropped patch files for media drivers from patchset as it is
	  not meant for outreachy applicants.
	- Added full-stop aign to text in commit message.
	- Made code more readable by adding a line break.
	- Changed cases to use modulo (% 4096) over (& 0xfff).
Changes in v1
	- Patch for drivers/staging/media/av7110/sp8870.c and
	- drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
	  is split into two different patches in the patchset for use by the different
	  driver maintainers.
	- Added subject title for each of the separated patches.
	- Patch 1: Removed unnecessary curly braces {} initially inserted.
	- Patch 2: Unnecessary {} was also removed for v1.

Abraham Samuel Adekunle (2):
  staging: rtl8723bs: Remove duplicate NULL tests on pstat
  staging: rtl8723bs: Prevent duplicate NULL tests on psta pointer

 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c |  2 +-
 drivers/staging/rtl8723bs/core/rtw_xmit.c     | 62 +++++++++----------
 2 files changed, 31 insertions(+), 33 deletions(-)

-- 
2.34.1

[PATCH v6 1/2] staging: rtl8723bs: Remove duplicate NULL tests on pstat

[Abraham Samuel Adekunle]

The pstat pointer has been previously tested for NULL after being
initialized as seen from line 970-973 of the file.
The first NULL test is shown below:

		pstat = rtw_get_stainfo(pstapriv, GetAddr2Ptr(pframe));
		 if (!pstat) {
			 status = WLAN_REASON_CLASS2_FRAME_FROM_NONAUTH_STA;
			goto asoc_class2_error;
		}

Remove the second NULL test to avoid redundancy in code.

Found by Coccinelle

Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index e74fb7d5dc37..863c188a3cfb 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -1323,7 +1323,7 @@ unsigned int OnAssocReq(struct adapter *padapter, union recv_frame *precv_frame)
 	spin_unlock_bh(&pstapriv->asoc_list_lock);
 
 	/*  now the station is qualified to join our BSS... */
-	if (pstat && (pstat->state & WIFI_FW_ASSOC_SUCCESS) && (status == WLAN_STATUS_SUCCESS)) {
+	if ((pstat->state & WIFI_FW_ASSOC_SUCCESS) && (status == WLAN_STATUS_SUCCESS)) {
 		/* 1 bss_cap_update & sta_info_update */
 		bss_cap_update_on_sta_join(padapter, pstat);
 		sta_info_update(padapter, pstat);
-- 
2.34.1

[PATCH v6 2/2] staging: rtl8723bs: Prevent duplicate NULL tests on psta pointer

[Abraham Samuel Adekunle]

The psta pointer had previously been tested for NULL after initialization,
as seen in the code below:

		psta = rtw_get_stainfo(&padapter->stapriv, pattrib->ra);
		...
                if (!psta)
			return _FAIL;
This makes a second NULL test on psta unnecessary.

Remove the second NULL test on psta to avoid redundancy in code.

Found by Cocinelle

Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/staging/rtl8723bs/core/rtw_xmit.c | 62 +++++++++++------------
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c b/drivers/staging/rtl8723bs/core/rtw_xmit.c
index 026d58b4bd7f..e933ccda1b39 100644
--- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
@@ -941,38 +941,36 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr, struct pkt_attrib *pattr
 			if (!(psta->state & _FW_LINKED))
 				return _FAIL;
 
-			if (psta) {
-				psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
-				psta->sta_xmitpriv.txseq_tid[pattrib->priority] %= 4096u;
-				pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];
-
-				SetSeqNum(hdr, pattrib->seqnum);
-
-				/* check if enable ampdu */
-				if (pattrib->ht_en && psta->htpriv.ampdu_enable)
-					if (psta->htpriv.agg_enable_bitmap & BIT(pattrib->priority))
-						pattrib->ampdu_en = true;
-
-				/* re-check if enable ampdu by BA_starting_seqctrl */
-				if (pattrib->ampdu_en == true) {
-					u16 tx_seq;
-
-					tx_seq = psta->BA_starting_seqctrl[pattrib->priority & 0x0f];
-
-					/* check BA_starting_seqctrl */
-					if (SN_LESS(pattrib->seqnum, tx_seq)) {
-						pattrib->ampdu_en = false;/* AGG BK */
-					} else if (SN_EQUAL(pattrib->seqnum, tx_seq)) {
-						psta->BA_starting_seqctrl[pattrib->priority & 0x0f] =
-							(tx_seq + 1) % 4096u;
-
-						pattrib->ampdu_en = true;/* AGG EN */
-					} else {
-						psta->BA_starting_seqctrl[pattrib->priority & 0x0f] =
-							(pattrib->seqnum + 1) % 4096u;
-
-						pattrib->ampdu_en = true;/* AGG EN */
-					}
+			psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
+			psta->sta_xmitpriv.txseq_tid[pattrib->priority] %= 4096u;
+			pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];
+
+			SetSeqNum(hdr, pattrib->seqnum);
+
+			/* check if enable ampdu */
+			if (pattrib->ht_en && psta->htpriv.ampdu_enable)
+				if (psta->htpriv.agg_enable_bitmap & BIT(pattrib->priority))
+					pattrib->ampdu_en = true;
+
+			/* re-check if enable ampdu by BA_starting_seqctrl */
+			if (pattrib->ampdu_en == true) {
+				u16 tx_seq;
+
+				tx_seq = psta->BA_starting_seqctrl[pattrib->priority & 0x0f];
+
+				/* check BA_starting_seqctrl */
+				if (SN_LESS(pattrib->seqnum, tx_seq)) {
+					pattrib->ampdu_en = false;/* AGG BK */
+				} else if (SN_EQUAL(pattrib->seqnum, tx_seq)) {
+					psta->BA_starting_seqctrl[pattrib->priority & 0x0f] =
+						(tx_seq + 1) % 4096u;
+
+					pattrib->ampdu_en = true;/* AGG EN */
+				} else {
+					psta->BA_starting_seqctrl[pattrib->priority & 0x0f] =
+						(pattrib->seqnum + 1) % 4096u;
+
+					pattrib->ampdu_en = true;/* AGG EN */
 				}
 			}
 		}
-- 
2.34.1

Re: [PATCH v6 1/2] staging: rtl8723bs: Remove duplicate NULL tests on pstat

[Andy Shevchenko]

On Fri, Apr 18, 2025 at 03:41:56PM +0000, Abraham Samuel Adekunle wrote:
> The pstat pointer has been previously tested for NULL after being
> initialized as seen from line 970-973 of the file.
> The first NULL test is shown below:

> 		pstat = rtw_get_stainfo(pstapriv, GetAddr2Ptr(pframe));
> 		 if (!pstat) {
> 			 status = WLAN_REASON_CLASS2_FRAME_FROM_NONAUTH_STA;
> 			goto asoc_class2_error;
> 		}

It's too much for such a change. You can do that in the comment area
(after '---' line below), or drop it completely. You can rephrase that
the pointer is checked before the changed lines.

> Remove the second NULL test to avoid redundancy in code.
> 
> Found by Coccinelle

Missing period.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 2/2] staging: rtl8723bs: Prevent duplicate NULL tests on psta pointer

[Andy Shevchenko]

On Fri, Apr 18, 2025 at 03:41:57PM +0000, Abraham Samuel Adekunle wrote:
> The psta pointer had previously been tested for NULL after initialization,
> as seen in the code below:
> 
> 		psta = rtw_get_stainfo(&padapter->stapriv, pattrib->ra);
> 		...
>                 if (!psta)
> 			return _FAIL;
> This makes a second NULL test on psta unnecessary.
> 
> Remove the second NULL test on psta to avoid redundancy in code.
> 
> Found by Cocinelle

Missing period.

P.S. Here the commit message is fine, however misindented
(check the indentation).

-- 
With Best Regards,
Andy Shevchenko