drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 2 +- drivers/staging/rtl8723bs/core/rtw_xmit.c | 56 +++++++++---------- 2 files changed, 28 insertions(+), 30 deletions(-)
When a value has been tested for NULL in an expression, a
second NULL test on the same value in another expression
is unnecessary when the value has not been assigned to NULL.
Remove unnecessary duplicate NULL tests on the same value
that has previously been tested.
Found by Coccinelle.
Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
---
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.
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 2 +-
drivers/staging/rtl8723bs/core/rtw_xmit.c | 56 +++++++++----------
2 files changed, 28 insertions(+), 30 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 90966b7034ab..675226535cd1 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);
diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c b/drivers/staging/rtl8723bs/core/rtw_xmit.c
index 026061b464f7..f817cab2f831 100644
--- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
@@ -941,35 +941,33 @@ 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] &= 0xFFF;
- 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)&0xfff;
-
- pattrib->ampdu_en = true;/* AGG EN */
- } else {
- psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff;
- pattrib->ampdu_en = true;/* AGG EN */
- }
+ psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
+ psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
+ 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)&0xfff;
+
+ pattrib->ampdu_en = true;/* AGG EN */
+ } else {
+ psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff;
+ pattrib->ampdu_en = true;/* AGG EN */
}
}
}
--
2.34.1On Sat, Apr 05, 2025 at 02:21:42PM +0100, Abraham Samuel Adekunle wrote: > When a value has been tested for NULL in an expression, a > second NULL test on the same value in another expression > is unnecessary when the value has not been assigned to NULL. > > Remove unnecessary duplicate NULL tests on the same value > that has previously been tested. > > Found by Coccinelle. > > Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> > --- Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org> regards, dan carpenter
On Sat, 5 Apr 2025, Abraham Samuel Adekunle wrote:
> When a value has been tested for NULL in an expression, a
> second NULL test on the same value in another expression
> is unnecessary when the value has not been assigned to NULL.
>
> Remove unnecessary duplicate NULL tests on the same value
> that has previously been tested.
>
> Found by Coccinelle.
The changes are found in the same way, but the code patterns are overall
quite different. It could make sense to make separate patches for them.
Then you could make a log message that is really specialized to the code
in each patch and it would be easier for the reviewer to be convinced that
you have done the right thing.
julia
>
> Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
> ---
> 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.
>
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 2 +-
> drivers/staging/rtl8723bs/core/rtw_xmit.c | 56 +++++++++----------
> 2 files changed, 28 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index 90966b7034ab..675226535cd1 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);
> diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> index 026061b464f7..f817cab2f831 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> @@ -941,35 +941,33 @@ 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] &= 0xFFF;
> - 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)&0xfff;
> -
> - pattrib->ampdu_en = true;/* AGG EN */
> - } else {
> - psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff;
> - pattrib->ampdu_en = true;/* AGG EN */
> - }
> + psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
> + psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
> + 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)&0xfff;
> +
> + pattrib->ampdu_en = true;/* AGG EN */
> + } else {
> + psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff;
> + pattrib->ampdu_en = true;/* AGG EN */
> }
> }
> }
> --
> 2.34.1
>
>
On Sat, Apr 5, 2025 at 2:30 PM Julia Lawall <julia.lawall@inria.fr> wrote: > > > > On Sat, 5 Apr 2025, Abraham Samuel Adekunle wrote: > > > When a value has been tested for NULL in an expression, a > > second NULL test on the same value in another expression > > is unnecessary when the value has not been assigned to NULL. > > > > Remove unnecessary duplicate NULL tests on the same value > > that has previously been tested. > > > > Found by Coccinelle. > > The changes are found in the same way, but the code patterns are overall > quite different. It could make sense to make separate patches for them. > Then you could make a log message that is really specialized to the code > in each patch and it would be easier for the reviewer to be convinced that > you have done the right thing. Thank you for the feedback Julia. Okay how about the versioning. Should I make it like a Patchset? Like [PATCH v6] with the cover letter and two patches, one for each code pattern? Thanks Adekunle
On Sat, 5 Apr 2025, Samuel Abraham wrote: > On Sat, Apr 5, 2025 at 2:30 PM Julia Lawall <julia.lawall@inria.fr> wrote: > > > > > > > > On Sat, 5 Apr 2025, Abraham Samuel Adekunle wrote: > > > > > When a value has been tested for NULL in an expression, a > > > second NULL test on the same value in another expression > > > is unnecessary when the value has not been assigned to NULL. > > > > > > Remove unnecessary duplicate NULL tests on the same value > > > that has previously been tested. > > > > > > Found by Coccinelle. > > > > The changes are found in the same way, but the code patterns are overall > > quite different. It could make sense to make separate patches for them. > > Then you could make a log message that is really specialized to the code > > in each patch and it would be easier for the reviewer to be convinced that > > you have done the right thing. > > Thank you for the feedback Julia. > Okay how about the versioning. Should I make it like a Patchset? > > Like [PATCH v6] with the cover letter and two patches, one for each code > pattern? Yes. Just explain what you are doing. julia
© 2016 - 2025 Red Hat, Inc.