On Thu, Nov 03, 2022 at 08:55:32AM +0100, Julia Lawall wrote:
>
>
> On Thu, 3 Nov 2022, Deepak R Varma wrote:
>
> > Simplify code by using min and max helper macros in place of lengthy
> > if/else block oriented logical evaluation and value assignment. This
> > issue is identified by coccicheck using the minmax.cocci file.
> >
> > Signed-off-by: Deepak R Varma <drv@xxxxxxxxx>
> > ---
> >
> > Please note:
> > 1. Using min for max_AMPDU_len computation warning was NOT auto generated by
> > the cocciecheck command. This was caught while surround code review and
> > was manually changed.
> > 2. Checkpatch script continues to complaint about min_MPDU_spacing
> > computation line being more than 100 character in length. I did not find a
> > better formatting that will address this checkpatch warning. Any
> > suggestions are most welcome.
> > 3. Proposed changes are compile tested only on my x86 based VM.
> >
> >
> > drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 12 ++++--------
> > drivers/staging/rtl8723bs/hal/odm_DIG.c | 5 +----
> > 2 files changed, 5 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
> > index 18ba846c0b7b..dcda587b84bc 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
> > @@ -986,15 +986,11 @@ void HT_caps_handler(struct adapter *padapter, struct ndis_80211_var_ie *pIE)
> > pmlmeinfo->HT_caps.u.HT_cap[i] &= (pIE->data[i]);
> > } else {
> > /* modify from fw by Thomas 2010/11/17 */
> > - if ((pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x3) > (pIE->data[i] & 0x3))
> > - max_AMPDU_len = (pIE->data[i] & 0x3);
> > - else
> > - max_AMPDU_len = (pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x3);
> > + max_AMPDU_len = min((pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x3),
> > + (pIE->data[i] & 0x3));
> >
> > - if ((pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x1c) > (pIE->data[i] & 0x1c))
> > - min_MPDU_spacing = (pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x1c);
> > - else
> > - min_MPDU_spacing = (pIE->data[i] & 0x1c);
> > + min_MPDU_spacing = max((pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x1c),
> > + (pIE->data[i] & 0x1c));
>
> There seem to be a lot of unnecessary parentheses. Admittedly they were
> there before, but since you are creating the max call from scratch in this
> patch, perhaps the parentheses around the arguments could be dropped at
> the same time.
Sounds good. I will improve further and send in a revision.
Also, do you have a comment on why coccicheck did not catch the min opportunity
for max_AMPDU_len computation? I am not seeing anything different with the
if/else blocks here.
Thank you,
./drv
>
> julia
>
> >
> > pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para = max_AMPDU_len | min_MPDU_spacing;
> > }
> > diff --git a/drivers/staging/rtl8723bs/hal/odm_DIG.c b/drivers/staging/rtl8723bs/hal/odm_DIG.c
> > index 07edf74ccfe5..97a51546463a 100644
> > --- a/drivers/staging/rtl8723bs/hal/odm_DIG.c
> > +++ b/drivers/staging/rtl8723bs/hal/odm_DIG.c
> > @@ -598,10 +598,7 @@ void odm_DIGbyRSSI_LPS(void *pDM_VOID)
> > /* Lower bound checking */
> >
> > /* RSSI Lower bound check */
> > - if ((pDM_Odm->RSSI_Min-10) > DM_DIG_MIN_NIC)
> > - RSSI_Lower = pDM_Odm->RSSI_Min-10;
> > - else
> > - RSSI_Lower = DM_DIG_MIN_NIC;
> > + RSSI_Lower = max(pDM_Odm->RSSI_Min - 10, DM_DIG_MIN_NIC);
> >
> > /* Upper and Lower Bound checking */
> > if (CurrentIGI > DM_DIG_MAX_NIC)
> > --
> > 2.34.1
> >
> >
> >
> >
> >
>
