* [PATCH v2 0/2] staging: rtl8192u: cleanup of ternary and if else statements
@ 2022-04-08 1:12 Rebecca Mckeever
2022-04-08 1:12 ` [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment Rebecca Mckeever
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Rebecca Mckeever @ 2022-04-08 1:12 UTC (permalink / raw)
To: outreachy
Cc: Greg Kroah-Hartman, linux-staging, linux-kernel, Rebecca Mckeever
These patches replace ternary and if else statements with
more readable statements. Found with minmax coccinelle script.
---
v1 -> v2:
* Patch 1: "staging: rtl8192u: replace ternary statement with if and
* assignment"
* replaced max macro with an if statement followed by an assignment
* Patch 2: "staging: rtl8192u: use min_t/max_t macros instead of if
* else"
* changed the type argument in min_t and max_t from u8 to u32
---
Rebecca Mckeever (2):
staging: rtl8192u: replace ternary statement with if and assignment
staging: rtl8192u: use min_t/max_t macros instead of if else
drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 4 +++-
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 13 +++++--------
2 files changed, 8 insertions(+), 9 deletions(-)
--
2.32.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment
2022-04-08 1:12 [PATCH v2 0/2] staging: rtl8192u: cleanup of ternary and if else statements Rebecca Mckeever
@ 2022-04-08 1:12 ` Rebecca Mckeever
2022-04-08 4:15 ` Julia Lawall
2022-04-08 1:12 ` [PATCH v2 2/2] staging: rtl8192u: use min_t/max_t macros instead of if else Rebecca Mckeever
2022-04-08 6:39 ` [PATCH v2 0/2] staging: rtl8192u: cleanup of ternary and if else statements Dan Carpenter
2 siblings, 1 reply; 12+ messages in thread
From: Rebecca Mckeever @ 2022-04-08 1:12 UTC (permalink / raw)
To: outreachy
Cc: Greg Kroah-Hartman, linux-staging, linux-kernel, Rebecca Mckeever
Replace ternary statement with an if statement followed by an assignment
to increase readability and make error handling more obvious.
Found with minmax coccinelle script.
Signed-off-by: Rebecca Mckeever <remckee0@gmail.com>
---
drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
index 78cc8f357bbc..9885917b9199 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
@@ -470,7 +470,9 @@ 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);
+ if (len < 0)
+ len = 0;
+ erq->length = len;
erq->flags |= IW_ENCODE_ENABLED;
--
2.32.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] staging: rtl8192u: use min_t/max_t macros instead of if else
2022-04-08 1:12 [PATCH v2 0/2] staging: rtl8192u: cleanup of ternary and if else statements Rebecca Mckeever
2022-04-08 1:12 ` [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment Rebecca Mckeever
@ 2022-04-08 1:12 ` Rebecca Mckeever
2022-04-08 6:39 ` [PATCH v2 0/2] staging: rtl8192u: cleanup of ternary and if else statements Dan Carpenter
2 siblings, 0 replies; 12+ messages in thread
From: Rebecca Mckeever @ 2022-04-08 1:12 UTC (permalink / raw)
To: outreachy
Cc: Greg Kroah-Hartman, linux-staging, linux-kernel, Rebecca Mckeever
Replace 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.
Using u32 assures the reader that the value with not be truncated
without having to look up the types of the variables involved.
Found with minmax coccinelle script.
Signed-off-by: Rebecca Mckeever <remckee0@gmail.com>
---
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index dba3f2db9f48..358c35d9589c 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -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(u32, 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(u32, pHTInfo->MPDU_Density,
+ pPeerHTCap->MPDUDensity);
+
if (ieee->pairwise_key_type != KEY_TYPE_NA)
pHTInfo->CurrentMPDUDensity = 7; // 8us
// Force TX AMSDU
--
2.32.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment
2022-04-08 1:12 ` [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment Rebecca Mckeever
@ 2022-04-08 4:15 ` Julia Lawall
2022-04-08 5:09 ` Greg Kroah-Hartman
2022-04-08 5:57 ` Dan Carpenter
0 siblings, 2 replies; 12+ messages in thread
From: Julia Lawall @ 2022-04-08 4:15 UTC (permalink / raw)
To: Rebecca Mckeever
Cc: outreachy, Greg Kroah-Hartman, linux-staging, linux-kernel
On Thu, 7 Apr 2022, Rebecca Mckeever wrote:
> Replace ternary statement with an if statement followed by an assignment
> to increase readability and make error handling more obvious.
> Found with minmax coccinelle script.
>
> Signed-off-by: Rebecca Mckeever <remckee0@gmail.com>
> ---
> drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> index 78cc8f357bbc..9885917b9199 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> @@ -470,7 +470,9 @@ 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);
> + if (len < 0)
> + len = 0;
> + erq->length = len;
Maybe you could use max here?
julia
>
> erq->flags |= IW_ENCODE_ENABLED;
>
> --
> 2.32.0
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment
2022-04-08 4:15 ` Julia Lawall
@ 2022-04-08 5:09 ` Greg Kroah-Hartman
2022-04-08 5:57 ` Dan Carpenter
1 sibling, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2022-04-08 5:09 UTC (permalink / raw)
To: Julia Lawall; +Cc: Rebecca Mckeever, outreachy, linux-staging, linux-kernel
On Fri, Apr 08, 2022 at 06:15:14AM +0200, Julia Lawall wrote:
>
>
> On Thu, 7 Apr 2022, Rebecca Mckeever wrote:
>
> > Replace ternary statement with an if statement followed by an assignment
> > to increase readability and make error handling more obvious.
> > Found with minmax coccinelle script.
> >
> > Signed-off-by: Rebecca Mckeever <remckee0@gmail.com>
> > ---
> > drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > index 78cc8f357bbc..9885917b9199 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > @@ -470,7 +470,9 @@ 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);
> > + if (len < 0)
> > + len = 0;
> > + erq->length = len;
>
> Maybe you could use max here?
Dan said to not do this on based on the first submission :)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment
2022-04-08 4:15 ` Julia Lawall
2022-04-08 5:09 ` Greg Kroah-Hartman
@ 2022-04-08 5:57 ` Dan Carpenter
2022-04-08 6:14 ` Joe Perches
2022-04-08 8:50 ` Julia Lawall
1 sibling, 2 replies; 12+ messages in thread
From: Dan Carpenter @ 2022-04-08 5:57 UTC (permalink / raw)
To: Julia Lawall
Cc: Rebecca Mckeever, outreachy, Greg Kroah-Hartman, linux-staging,
linux-kernel
On Fri, Apr 08, 2022 at 06:15:14AM +0200, Julia Lawall wrote:
> On Thu, 7 Apr 2022, Rebecca Mckeever wrote:
>
> > Replace ternary statement with an if statement followed by an assignment
> > to increase readability and make error handling more obvious.
> > Found with minmax coccinelle script.
> >
> > Signed-off-by: Rebecca Mckeever <remckee0@gmail.com>
> > ---
> > drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > index 78cc8f357bbc..9885917b9199 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > @@ -470,7 +470,9 @@ 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);
> > + if (len < 0)
> > + len = 0;
> > + erq->length = len;
>
> Maybe you could use max here?
Initially Rebecca did use max() but I NAKed it. It's really not less
readable. Better to handle the error explicitly. Keep the error path
indented two tabs. Separate from the success path.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment
2022-04-08 5:57 ` Dan Carpenter
@ 2022-04-08 6:14 ` Joe Perches
2022-04-08 6:31 ` Dan Carpenter
2022-04-08 8:50 ` Julia Lawall
1 sibling, 1 reply; 12+ messages in thread
From: Joe Perches @ 2022-04-08 6:14 UTC (permalink / raw)
To: Dan Carpenter, Julia Lawall
Cc: Rebecca Mckeever, outreachy, Greg Kroah-Hartman, linux-staging,
linux-kernel
On Fri, 2022-04-08 at 08:57 +0300, Dan Carpenter wrote:
> On Fri, Apr 08, 2022 at 06:15:14AM +0200, Julia Lawall wrote:
> > On Thu, 7 Apr 2022, Rebecca Mckeever wrote:
> >
> > > Replace ternary statement with an if statement followed by an assignment
> > > to increase readability and make error handling more obvious.
> > > Found with minmax coccinelle script.
[]
> > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
[]
> > > @@ -470,7 +470,9 @@ 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);
> > > + if (len < 0)
> > > + len = 0;
> > > + erq->length = len;
> >
> > Maybe you could use max here?
>
> Initially Rebecca did use max() but I NAKed it. It's really not less
> readable. Better to handle the error explicitly. Keep the error path
> indented two tabs. Separate from the success path.
A comment would be useful as it's not obvious it's an 'error' path.
One has to read all 3 get_key functions to determine that.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment
2022-04-08 6:14 ` Joe Perches
@ 2022-04-08 6:31 ` Dan Carpenter
2022-04-08 7:19 ` Joe Perches
0 siblings, 1 reply; 12+ messages in thread
From: Dan Carpenter @ 2022-04-08 6:31 UTC (permalink / raw)
To: Joe Perches
Cc: Julia Lawall, Rebecca Mckeever, outreachy, Greg Kroah-Hartman,
linux-staging, linux-kernel
On Thu, Apr 07, 2022 at 11:14:51PM -0700, Joe Perches wrote:
> On Fri, 2022-04-08 at 08:57 +0300, Dan Carpenter wrote:
> > On Fri, Apr 08, 2022 at 06:15:14AM +0200, Julia Lawall wrote:
> > > On Thu, 7 Apr 2022, Rebecca Mckeever wrote:
> > >
> > > > Replace ternary statement with an if statement followed by an assignment
> > > > to increase readability and make error handling more obvious.
> > > > Found with minmax coccinelle script.
> []
> > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> []
> > > > @@ -470,7 +470,9 @@ 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);
> > > > + if (len < 0)
> > > > + len = 0;
> > > > + erq->length = len;
> > >
> > > Maybe you could use max here?
> >
> > Initially Rebecca did use max() but I NAKed it. It's really not less
> > readable. Better to handle the error explicitly. Keep the error path
> > indented two tabs. Separate from the success path.
>
> A comment would be useful as it's not obvious it's an 'error' path.
> One has to read all 3 get_key functions to determine that.
>
I'm so confused. Negative error codes are the common case in the
kernel. We don't need to comment it.
Obviously in an ideal world the get_key() functions would return -EINVAL
instead of -1...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/2] staging: rtl8192u: cleanup of ternary and if else statements
2022-04-08 1:12 [PATCH v2 0/2] staging: rtl8192u: cleanup of ternary and if else statements Rebecca Mckeever
2022-04-08 1:12 ` [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment Rebecca Mckeever
2022-04-08 1:12 ` [PATCH v2 2/2] staging: rtl8192u: use min_t/max_t macros instead of if else Rebecca Mckeever
@ 2022-04-08 6:39 ` Dan Carpenter
2 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2022-04-08 6:39 UTC (permalink / raw)
To: Rebecca Mckeever
Cc: outreachy, Greg Kroah-Hartman, linux-staging, linux-kernel
On Thu, Apr 07, 2022 at 08:12:49PM -0500, Rebecca Mckeever wrote:
> These patches replace ternary and if else statements with
> more readable statements. Found with minmax coccinelle script.
>
> ---
> v1 -> v2:
> * Patch 1: "staging: rtl8192u: replace ternary statement with if and
> * assignment"
> * replaced max macro with an if statement followed by an assignment
>
> * Patch 2: "staging: rtl8192u: use min_t/max_t macros instead of if
> * else"
> * changed the type argument in min_t and max_t from u8 to u32
> ---
Looks great. Thanks!
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment
2022-04-08 6:31 ` Dan Carpenter
@ 2022-04-08 7:19 ` Joe Perches
2022-04-08 8:35 ` Dan Carpenter
0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2022-04-08 7:19 UTC (permalink / raw)
To: Dan Carpenter
Cc: Julia Lawall, Rebecca Mckeever, outreachy, Greg Kroah-Hartman,
linux-staging, linux-kernel
On Fri, 2022-04-08 at 09:31 +0300, Dan Carpenter wrote:
> On Thu, Apr 07, 2022 at 11:14:51PM -0700, Joe Perches wrote:
> > On Fri, 2022-04-08 at 08:57 +0300, Dan Carpenter wrote:
> > > On Fri, Apr 08, 2022 at 06:15:14AM +0200, Julia Lawall wrote:
> > > > On Thu, 7 Apr 2022, Rebecca Mckeever wrote:
> > > >
> > > > > Replace ternary statement with an if statement followed by an assignment
> > > > > to increase readability and make error handling more obvious.
> > > > > Found with minmax coccinelle script.
> > []
> > > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > []
> > > > > @@ -470,7 +470,9 @@ 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);
> > > > > + if (len < 0)
> > > > > + len = 0;
> > > > > + erq->length = len;
> > > >
> > > > Maybe you could use max here?
> > >
> > > Initially Rebecca did use max() but I NAKed it. It's really not less
> > > readable. Better to handle the error explicitly. Keep the error path
> > > indented two tabs. Separate from the success path.
> >
> > A comment would be useful as it's not obvious it's an 'error' path.
> > One has to read all 3 get_key functions to determine that.
> >
>
> I'm so confused. Negative error codes are the common case in the
> kernel. We don't need to comment it.
If it was an error, it would handle it as an error not set
len to 0 and continue. That's why IMO a comment is useful.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment
2022-04-08 7:19 ` Joe Perches
@ 2022-04-08 8:35 ` Dan Carpenter
0 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2022-04-08 8:35 UTC (permalink / raw)
To: Joe Perches
Cc: Julia Lawall, Rebecca Mckeever, outreachy, Greg Kroah-Hartman,
linux-staging, linux-kernel
On Fri, Apr 08, 2022 at 12:19:01AM -0700, Joe Perches wrote:
> On Fri, 2022-04-08 at 09:31 +0300, Dan Carpenter wrote:
> > On Thu, Apr 07, 2022 at 11:14:51PM -0700, Joe Perches wrote:
> > > On Fri, 2022-04-08 at 08:57 +0300, Dan Carpenter wrote:
> > > > On Fri, Apr 08, 2022 at 06:15:14AM +0200, Julia Lawall wrote:
> > > > > On Thu, 7 Apr 2022, Rebecca Mckeever wrote:
> > > > >
> > > > > > Replace ternary statement with an if statement followed by an assignment
> > > > > > to increase readability and make error handling more obvious.
> > > > > > Found with minmax coccinelle script.
> > > []
> > > > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > > []
> > > > > > @@ -470,7 +470,9 @@ 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);
> > > > > > + if (len < 0)
> > > > > > + len = 0;
> > > > > > + erq->length = len;
> > > > >
> > > > > Maybe you could use max here?
> > > >
> > > > Initially Rebecca did use max() but I NAKed it. It's really not less
> > > > readable. Better to handle the error explicitly. Keep the error path
> > > > indented two tabs. Separate from the success path.
> > >
> > > A comment would be useful as it's not obvious it's an 'error' path.
> > > One has to read all 3 get_key functions to determine that.
> > >
> >
> > I'm so confused. Negative error codes are the common case in the
> > kernel. We don't need to comment it.
>
> If it was an error, it would handle it as an error not set
> len to 0 and continue. That's why IMO a comment is useful.
Yeah. You're probably right. My understanding is that a zero length
key is a special case where it uses the default key? Which I guess is
all zeroes here.
if (len < 0) {
/* No key data. Use the default key. */
len = 0;
}
But when I look at this some more then there are three ->get_key()
callers in this file and only this one checks for -1 returns. For the
one caller that does this:
ext->key_len = crypt->ops->get_key(ext->key, SCM_KEY_LEN, NULL, crypt->priv);
then a negative return would result in a buffer overflow.
So another option would be to just return 0 instead of -1 from the
get_key() functions.
File | Pointer | Function | Static
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | (struct ieee80211_crypto_ops)->get_key | ieee80211_tkip_get_key | 1
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c | (struct ieee80211_crypto_ops)->get_key | prism2_wep_get_key | 1
drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_ccmp.c | (struct ieee80211_crypto_ops)->get_key | ieee80211_ccmp_get_key | 1
Changing it to return zero would leave ieee80211_wx_get_encode() behavior
as-is. It would fix a buffer overflow in ieee80211_wx_get_encode_ext().
It is a behavior change in ieee80211_wx_set_encode() and I think that's
a bug fix as well.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment
2022-04-08 5:57 ` Dan Carpenter
2022-04-08 6:14 ` Joe Perches
@ 2022-04-08 8:50 ` Julia Lawall
1 sibling, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2022-04-08 8:50 UTC (permalink / raw)
To: Dan Carpenter
Cc: Rebecca Mckeever, outreachy, Greg Kroah-Hartman, linux-staging,
linux-kernel
On Fri, 8 Apr 2022, Dan Carpenter wrote:
> On Fri, Apr 08, 2022 at 06:15:14AM +0200, Julia Lawall wrote:
> > On Thu, 7 Apr 2022, Rebecca Mckeever wrote:
> >
> > > Replace ternary statement with an if statement followed by an assignment
> > > to increase readability and make error handling more obvious.
> > > Found with minmax coccinelle script.
> > >
> > > Signed-off-by: Rebecca Mckeever <remckee0@gmail.com>
> > > ---
> > > drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > > index 78cc8f357bbc..9885917b9199 100644
> > > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> > > @@ -470,7 +470,9 @@ 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);
> > > + if (len < 0)
> > > + len = 0;
> > > + erq->length = len;
> >
> > Maybe you could use max here?
>
> Initially Rebecca did use max() but I NAKed it. It's really not less
> readable. Better to handle the error explicitly. Keep the error path
> indented two tabs. Separate from the success path.
OK. I have a hard time seeing that as an error path, though, with no
return or goto. I guess it's error and recover. OK either way.
julia
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-04-08 8:51 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-08 1:12 [PATCH v2 0/2] staging: rtl8192u: cleanup of ternary and if else statements Rebecca Mckeever
2022-04-08 1:12 ` [PATCH v2 1/2] staging: rtl8192u: replace ternary statement with if and assignment Rebecca Mckeever
2022-04-08 4:15 ` Julia Lawall
2022-04-08 5:09 ` Greg Kroah-Hartman
2022-04-08 5:57 ` Dan Carpenter
2022-04-08 6:14 ` Joe Perches
2022-04-08 6:31 ` Dan Carpenter
2022-04-08 7:19 ` Joe Perches
2022-04-08 8:35 ` Dan Carpenter
2022-04-08 8:50 ` Julia Lawall
2022-04-08 1:12 ` [PATCH v2 2/2] staging: rtl8192u: use min_t/max_t macros instead of if else Rebecca Mckeever
2022-04-08 6:39 ` [PATCH v2 0/2] staging: rtl8192u: cleanup of ternary and if else statements Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).