From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail2-relais-roc.national.inria.fr (mail2-relais-roc.national.inria.fr [192.134.164.83]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8038CA45; Thu, 3 Nov 2022 07:55:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=inria.fr; s=dc; h=date:from:to:cc:subject:in-reply-to:message-id: references:mime-version; bh=vAJ1+DlJ5nK0vw5GJm0avG1Jz8IfJecYpgZXhuzPbec=; b=rsivWsM6QjPZPKQPqlmygtNZ5NPw5ziZ2pKhJfHjXYQFgnPQUL7tbYwX AuudAGQ0npI74Pa8q9/J6BA+qSOVdFM2XGdRZBQf99vdicaRy8a0MSQmd CFeLP1nfYqZufzx2lHsvqiIcPe47vVq1ZGjhkoDZjFU/YYQvsP2jUGcqG s=; Authentication-Results: mail2-relais-roc.national.inria.fr; dkim=none (message not signed) header.i=none; spf=SoftFail smtp.mailfrom=julia.lawall@inria.fr; dmarc=fail (p=none dis=none) d=inria.fr X-IronPort-AV: E=Sophos;i="5.95,235,1661810400"; d="scan'208";a="72753911" Received: from 51.123.68.85.rev.sfr.net (HELO hadrien) ([85.68.123.51]) by mail2-relais-roc.national.inria.fr with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Nov 2022 08:55:32 +0100 Date: Thu, 3 Nov 2022 08:55:32 +0100 (CET) From: Julia Lawall X-X-Sender: jll@hadrien To: Deepak R Varma cc: outreachy@lists.linux.dev, Greg Kroah-Hartman , linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging: rtl8723bs: Use min/max macros for variable comparison In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (DEB 394 2020-01-19) Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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 > --- > > 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. 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 > > > > >