staging: rtl8192u: use min/max macros
cover staging: rtl8192u: use min/max macros
These patches replace ternary and if else statements with an equivalent min/max macro. Found with minmax coccinelle script.
---
I noticed that ieee80211.h includes linux/kernel.h and linux/jiffies.h, which both include linux/minmax.h. Would it be a good idea to add #include <linux/minmax.h>
in ieee80211.h in case the content of linux/kernel.h or linux/jiffies.h changes?
---
Rebecca Mckeever (2):
staging: rtl8192u: use max macro instead of ternary operator
staging: rtl8192u: use min_t/max_t macros instead of if else
drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 2 +- drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 13 +++++-------- 2 files changed, 6 insertions(+), 9 deletions(-)
Signature · 1 line
2.32.0
PATCH 1/2 staging: rtl8192u: use max macro instead of ternary operator
Replace ternary operator with max macro to increase readability and conform to Linux kernel coding style.
Found with minmax coccinelle script.
--- drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c +1 -1
@@ -470,7 +470,7 @@ int ieee80211_wx_get_encode(struct ieee80211_device *ieee,return 0 ; } len = crypt->ops->get_key (keybuf,SCM_KEY_LEN ,NULL , crypt->priv); - erq->length = (len >=0 ? len :0 ); + erq->length =max (len,0 ); erq->flags |=IW_ENCODE_ENABLED ;
Signature · 1 line
2.32.0
Re: [PATCH 1/2] staging: rtl8192u: use max macro instead of ternary operator
Quoted 14 lines · Rebecca Mckeever
Replace ternary operator with max macro to increase readability and conform to Linux kernel coding style.
Found with minmax coccinelle script.Signed-off-by Rebecca Mckeever <[email protected]>--- drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c +0
-0
drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c int ieee80211_wx_get_encode(struct ieee80211_device *ieee, · line 473
Quoted 5 lines
drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c +1
-1@@ -470,7 +470,7 @@ int ieee80211_wx_get_encode(struct ieee80211_device *ieee,return 0 ; } len = crypt->ops->get_key (keybuf,SCM_KEY_LEN ,NULL , crypt->priv); - erq->length = (len >=0 ? len :0 ); + erq->length =max (len,0 );
Neither before nor after is really readable. It would be better to write it as:
len = crypt->ops->get_key(keybuf, SCM_KEY_LEN, NULL, crypt->priv); if (len < 0) len = 0; erq->length = len;
There are few rules/patterns that apply here:
1) Do error handling right away.
2) Do error handling not success handling.
3) Try to keep the error path and the success path separate.
4) Try to keep the success path indented one tab and the error path indented two tabs. It looks like this to the reader:
a = frob(); if (fail) cleanup; b = frob(); if (fail) cleanup; success check fail success check fail
So then if you want to understand what a function does at a high level you can just skim the code which is indented one tab and ignore the error handling code.
regards,
dan carpenter
PATCH 2/2 staging: rtl8192u: use min_t/max_t macros instead of if else
In reply to Rebecca MckeeverReplace if else statement with min_t or max_t macros to increase readability and conform to Linux kernel coding style. The _t versions of the macros must be used to avoid applying typeof to the bit fields pPeerHTCap->MaxRxAMPDUFactor, and pPeerHTCap->MPDUDensity. Found with minmax coccinelle script.
--- drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c +5 -8
@@ -940,10 +940,8 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)else pHTInfo->CurrentAMPDUFactor =HT_AGG_SIZE_64K ; }else { -if (pPeerHTCap->MaxRxAMPDUFactor <HT_AGG_SIZE_32K ) - pHTInfo->CurrentAMPDUFactor = pPeerHTCap->MaxRxAMPDUFactor; -else - pHTInfo->CurrentAMPDUFactor =HT_AGG_SIZE_32K ; + pHTInfo->CurrentAMPDUFactor =min_t (u8, pPeerHTCap->MaxRxAMPDUFactor, +HT_AGG_SIZE_32K ); } }
@@ -951,10 +949,9 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)* <2> Set AMPDU Minimum MPDU Start Spacing * 802.11n 3.0 section 9.7d.3 */ -if (pHTInfo->MPDU_Density > pPeerHTCap->MPDUDensity) - pHTInfo->CurrentMPDUDensity = pHTInfo->MPDU_Density; -else - pHTInfo->CurrentMPDUDensity = pPeerHTCap->MPDUDensity; + pHTInfo->CurrentMPDUDensity =max_t (u8, pHTInfo->MPDU_Density, + pPeerHTCap->MPDUDensity); +if (ieee->pairwise_key_type !=KEY_TYPE_NA ) pHTInfo->CurrentMPDUDensity =7 ;// 8us // Force TX AMSDU
Signature · 1 line
2.32.0
Re: [PATCH 2/2] staging: rtl8192u: use min_t/max_t macros instead of if else
Quoted 16 lines · Rebecca Mckeever
Replace if else statement with
min_tormax_tmacros to increase readability and conform to Linux kernel coding style. The _t versions of the macros must be used to avoid applying typeof to the bit fieldspPeerHTCap->MaxRxAMPDUFactor, andpPeerHTCap->MPDUDensity. Found with minmax coccinelle script.Signed-off-by Rebecca Mckeever <[email protected]>--- drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c +0
-0
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c void HTOnAssocRsp(struct ieee80211_device *ieee) · line 944
Quoted 9 lines
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c +2
-4@@ -940,10 +940,8 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)else pHTInfo->CurrentAMPDUFactor =HT_AGG_SIZE_64K ; }else { -if (pPeerHTCap->MaxRxAMPDUFactor <HT_AGG_SIZE_32K ) - pHTInfo->CurrentAMPDUFactor = pPeerHTCap->MaxRxAMPDUFactor; -else - pHTInfo->CurrentAMPDUFactor =HT_AGG_SIZE_32K ; + pHTInfo->CurrentAMPDUFactor =min_t (u8, pPeerHTCap->MaxRxAMPDUFactor, +HT_AGG_SIZE_32K );
The u8 cast doesn't cause a problem, but it makes me nervous so I have to look up the types involved.
pPeerHTCap->MaxRxAMPDUFactor is 2 bits of a u8. HT_AGG_SIZE_32K is an enum, which in real life terms of the compilers we use is going to be a uint.
With min_t() you want to do the casts to a large enough unsigned type so that it doesn't truncate. And so u8 works. But it would be less scary if you cast it to a u32 or uint because a read will assume that probably pPeerHTCap->MaxRxAMPDUFactor is a u32 and HT_AGG_SIZE_32K is an int. These are false assumptions, but probably that's what most people will assume.
The reader in this case will feel safe and not worry about truncation so they won't have to look up the types.
regards,
dan carpenter