* [PATCH] Staging: rtl8192e: Remove ternary operator
@ 2016-10-08 18:28 Bhumika Goyal
2016-10-09 6:00 ` [Outreachy kernel] " Julia Lawall
2016-10-09 14:26 ` Greg KH
0 siblings, 2 replies; 8+ messages in thread
From: Bhumika Goyal @ 2016-10-08 18:28 UTC (permalink / raw)
To: outreachy-kernel, gregkh; +Cc: Bhumika Goyal
Relational and logical operators evaluate to either true or false.
Explicit conversion is not needed so remove the ternary operator.
Done using coccinelle:
@r@
expression A,B;
symbol true,false;
binary operator b = {==,!=,&&,||,>=,<=,>,<};
@@
- (A b B) ? true : false
+ A b B
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
drivers/staging/rtl8192e/rtl819x_HTProc.c | 12 ++++--------
drivers/staging/rtl8192e/rtllib.h | 2 +-
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c
index dd9c0c8..8cd51a9 100644
--- a/drivers/staging/rtl8192e/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c
@@ -558,19 +558,15 @@ void HTOnAssocRsp(struct rtllib_device *ieee)
#endif
HTSetConnectBwMode(ieee, (enum ht_channel_width)(pPeerHTCap->ChlWidth),
(enum ht_extchnl_offset)(pPeerHTInfo->ExtChlOffset));
- pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth == 1) ?
- true : false);
+ pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1);
pHTInfo->bCurShortGI20MHz = ((pHTInfo->bRegShortGI20MHz) ?
- ((pPeerHTCap->ShortGI20Mhz == 1) ?
- true : false) : false);
+ (pPeerHTCap->ShortGI20Mhz == 1) : false);
pHTInfo->bCurShortGI40MHz = ((pHTInfo->bRegShortGI40MHz) ?
- ((pPeerHTCap->ShortGI40Mhz == 1) ?
- true : false) : false);
+ (pPeerHTCap->ShortGI40Mhz == 1) : false);
pHTInfo->bCurSuppCCK = ((pHTInfo->bRegSuppCCK) ?
- ((pPeerHTCap->DssCCk == 1) ? true :
- false) : false);
+ (pPeerHTCap->DssCCk == 1) : false);
pHTInfo->bCurrent_AMSDU_Support = pHTInfo->bAMSDU_Support;
diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index b895a53..f2c28f3 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -464,7 +464,7 @@ struct ieee_param {
#define RTLLIB_QCTL_TID 0x000F
#define FC_QOS_BIT BIT7
-#define IsDataFrame(pdu) (((pdu[0] & 0x0C) == 0x08) ? true : false)
+#define IsDataFrame(pdu) ((pdu[0] & 0x0C) == 0x08)
#define IsLegacyDataFrame(pdu) (IsDataFrame(pdu) && (!(pdu[0]&FC_QOS_BIT)))
#define IsQoSDataFrame(pframe) \
((*(u16 *)pframe&(RTLLIB_STYPE_QOS_DATA|RTLLIB_FTYPE_DATA)) == \
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Outreachy kernel] [PATCH] Staging: rtl8192e: Remove ternary operator 2016-10-08 18:28 [PATCH] Staging: rtl8192e: Remove ternary operator Bhumika Goyal @ 2016-10-09 6:00 ` Julia Lawall 2016-10-09 6:38 ` Bhumika Goyal 2016-10-09 14:26 ` Greg KH 1 sibling, 1 reply; 8+ messages in thread From: Julia Lawall @ 2016-10-09 6:00 UTC (permalink / raw) To: Bhumika Goyal; +Cc: outreachy-kernel, gregkh On Sat, 8 Oct 2016, Bhumika Goyal wrote: > Relational and logical operators evaluate to either true or false. > Explicit conversion is not needed so remove the ternary operator. > Done using coccinelle: > > @r@ > expression A,B; > symbol true,false; > binary operator b = {==,!=,&&,||,>=,<=,>,<}; > @@ > - (A b B) ? true : false > + A b B > > Signed-off-by: Bhumika Goyal <bhumirks@gmail.com> > --- > drivers/staging/rtl8192e/rtl819x_HTProc.c | 12 ++++-------- > drivers/staging/rtl8192e/rtllib.h | 2 +- > 2 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c > index dd9c0c8..8cd51a9 100644 > --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c > +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c > @@ -558,19 +558,15 @@ void HTOnAssocRsp(struct rtllib_device *ieee) > #endif > HTSetConnectBwMode(ieee, (enum ht_channel_width)(pPeerHTCap->ChlWidth), > (enum ht_extchnl_offset)(pPeerHTInfo->ExtChlOffset)); > - pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth == 1) ? > - true : false); > + pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1); > > pHTInfo->bCurShortGI20MHz = ((pHTInfo->bRegShortGI20MHz) ? > - ((pPeerHTCap->ShortGI20Mhz == 1) ? > - true : false) : false); > + (pPeerHTCap->ShortGI20Mhz == 1) : false); > pHTInfo->bCurShortGI40MHz = ((pHTInfo->bRegShortGI40MHz) ? > - ((pPeerHTCap->ShortGI40Mhz == 1) ? > - true : false) : false); > + (pPeerHTCap->ShortGI40Mhz == 1) : false); > > pHTInfo->bCurSuppCCK = ((pHTInfo->bRegSuppCCK) ? > - ((pPeerHTCap->DssCCk == 1) ? true : > - false) : false); > + (pPeerHTCap->DssCCk == 1) : false); Even with the simplification, these look like a mess. Could && and || be used here? julia > > > pHTInfo->bCurrent_AMSDU_Support = pHTInfo->bAMSDU_Support; > diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h > index b895a53..f2c28f3 100644 > --- a/drivers/staging/rtl8192e/rtllib.h > +++ b/drivers/staging/rtl8192e/rtllib.h > @@ -464,7 +464,7 @@ struct ieee_param { > #define RTLLIB_QCTL_TID 0x000F > > #define FC_QOS_BIT BIT7 > -#define IsDataFrame(pdu) (((pdu[0] & 0x0C) == 0x08) ? true : false) > +#define IsDataFrame(pdu) ((pdu[0] & 0x0C) == 0x08) > #define IsLegacyDataFrame(pdu) (IsDataFrame(pdu) && (!(pdu[0]&FC_QOS_BIT))) > #define IsQoSDataFrame(pframe) \ > ((*(u16 *)pframe&(RTLLIB_STYPE_QOS_DATA|RTLLIB_FTYPE_DATA)) == \ > -- > 1.9.1 > > -- > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1475951297-10364-1-git-send-email-bhumirks%40gmail.com. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192e: Remove ternary operator 2016-10-09 6:00 ` [Outreachy kernel] " Julia Lawall @ 2016-10-09 6:38 ` Bhumika Goyal 2016-10-09 10:51 ` Julia Lawall 0 siblings, 1 reply; 8+ messages in thread From: Bhumika Goyal @ 2016-10-09 6:38 UTC (permalink / raw) To: Julia Lawall; +Cc: outreachy-kernel, gregkh@linuxfoundation.org On Sun, Oct 9, 2016 at 11:30 AM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > On Sat, 8 Oct 2016, Bhumika Goyal wrote: > >> Relational and logical operators evaluate to either true or false. >> Explicit conversion is not needed so remove the ternary operator. >> Done using coccinelle: >> >> @r@ >> expression A,B; >> symbol true,false; >> binary operator b = {==,!=,&&,||,>=,<=,>,<}; >> @@ >> - (A b B) ? true : false >> + A b B >> >> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com> >> --- >> drivers/staging/rtl8192e/rtl819x_HTProc.c | 12 ++++-------- >> drivers/staging/rtl8192e/rtllib.h | 2 +- >> 2 files changed, 5 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c >> index dd9c0c8..8cd51a9 100644 >> --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c >> +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c >> @@ -558,19 +558,15 @@ void HTOnAssocRsp(struct rtllib_device *ieee) >> #endif >> HTSetConnectBwMode(ieee, (enum ht_channel_width)(pPeerHTCap->ChlWidth), >> (enum ht_extchnl_offset)(pPeerHTInfo->ExtChlOffset)); >> - pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth == 1) ? >> - true : false); >> + pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1); >> >> pHTInfo->bCurShortGI20MHz = ((pHTInfo->bRegShortGI20MHz) ? >> - ((pPeerHTCap->ShortGI20Mhz == 1) ? >> - true : false) : false); >> + (pPeerHTCap->ShortGI20Mhz == 1) : false); >> pHTInfo->bCurShortGI40MHz = ((pHTInfo->bRegShortGI40MHz) ? >> - ((pPeerHTCap->ShortGI40Mhz == 1) ? >> - true : false) : false); >> + (pPeerHTCap->ShortGI40Mhz == 1) : false); >> >> pHTInfo->bCurSuppCCK = ((pHTInfo->bRegSuppCCK) ? >> - ((pPeerHTCap->DssCCk == 1) ? true : >> - false) : false); >> + (pPeerHTCap->DssCCk == 1) : false); > > Even with the simplification, these look like a mess. Could && and || be > used here? > Yes, I think these can be further reduced to something like: pHTInfo->bCurSuppCCK= (pHTInfo->bRegSuppCCK && pPeerHTCap->DssCCk); as the left hand side is true only if both the RHS values are true/1. Same reduction could be done for pHTInfo->bCurShortGI40MHz and pHTInfo->bCurShortGI20MHz. Thanks, Bhumika > julia > >> >> >> pHTInfo->bCurrent_AMSDU_Support = pHTInfo->bAMSDU_Support; >> diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h >> index b895a53..f2c28f3 100644 >> --- a/drivers/staging/rtl8192e/rtllib.h >> +++ b/drivers/staging/rtl8192e/rtllib.h >> @@ -464,7 +464,7 @@ struct ieee_param { >> #define RTLLIB_QCTL_TID 0x000F >> >> #define FC_QOS_BIT BIT7 >> -#define IsDataFrame(pdu) (((pdu[0] & 0x0C) == 0x08) ? true : false) >> +#define IsDataFrame(pdu) ((pdu[0] & 0x0C) == 0x08) >> #define IsLegacyDataFrame(pdu) (IsDataFrame(pdu) && (!(pdu[0]&FC_QOS_BIT))) >> #define IsQoSDataFrame(pframe) \ >> ((*(u16 *)pframe&(RTLLIB_STYPE_QOS_DATA|RTLLIB_FTYPE_DATA)) == \ >> -- >> 1.9.1 >> >> -- >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. >> To post to this group, send email to outreachy-kernel@googlegroups.com. >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1475951297-10364-1-git-send-email-bhumirks%40gmail.com. >> For more options, visit https://groups.google.com/d/optout. >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192e: Remove ternary operator 2016-10-09 6:38 ` Bhumika Goyal @ 2016-10-09 10:51 ` Julia Lawall 0 siblings, 0 replies; 8+ messages in thread From: Julia Lawall @ 2016-10-09 10:51 UTC (permalink / raw) To: Bhumika Goyal; +Cc: Julia Lawall, outreachy-kernel, gregkh@linuxfoundation.org On Sun, 9 Oct 2016, Bhumika Goyal wrote: > On Sun, Oct 9, 2016 at 11:30 AM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > > > > On Sat, 8 Oct 2016, Bhumika Goyal wrote: > > > >> Relational and logical operators evaluate to either true or false. > >> Explicit conversion is not needed so remove the ternary operator. > >> Done using coccinelle: > >> > >> @r@ > >> expression A,B; > >> symbol true,false; > >> binary operator b = {==,!=,&&,||,>=,<=,>,<}; > >> @@ > >> - (A b B) ? true : false > >> + A b B > >> > >> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com> > >> --- > >> drivers/staging/rtl8192e/rtl819x_HTProc.c | 12 ++++-------- > >> drivers/staging/rtl8192e/rtllib.h | 2 +- > >> 2 files changed, 5 insertions(+), 9 deletions(-) > >> > >> diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c > >> index dd9c0c8..8cd51a9 100644 > >> --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c > >> +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c > >> @@ -558,19 +558,15 @@ void HTOnAssocRsp(struct rtllib_device *ieee) > >> #endif > >> HTSetConnectBwMode(ieee, (enum ht_channel_width)(pPeerHTCap->ChlWidth), > >> (enum ht_extchnl_offset)(pPeerHTInfo->ExtChlOffset)); > >> - pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth == 1) ? > >> - true : false); > >> + pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1); > >> > >> pHTInfo->bCurShortGI20MHz = ((pHTInfo->bRegShortGI20MHz) ? > >> - ((pPeerHTCap->ShortGI20Mhz == 1) ? > >> - true : false) : false); > >> + (pPeerHTCap->ShortGI20Mhz == 1) : false); > >> pHTInfo->bCurShortGI40MHz = ((pHTInfo->bRegShortGI40MHz) ? > >> - ((pPeerHTCap->ShortGI40Mhz == 1) ? > >> - true : false) : false); > >> + (pPeerHTCap->ShortGI40Mhz == 1) : false); > >> > >> pHTInfo->bCurSuppCCK = ((pHTInfo->bRegSuppCCK) ? > >> - ((pPeerHTCap->DssCCk == 1) ? true : > >> - false) : false); > >> + (pPeerHTCap->DssCCk == 1) : false); > > > > Even with the simplification, these look like a mess. Could && and || be > > used here? > > > Yes, I think these can be further reduced to something like: > pHTInfo->bCurSuppCCK= (pHTInfo->bRegSuppCCK && pPeerHTCap->DssCCk); > as the left hand side is true only if both the RHS values are true/1. > Same reduction could be done for pHTInfo->bCurShortGI40MHz and > pHTInfo->bCurShortGI20MHz. Is DssCCk boolean? If not you may need to keep the == 1. julia > > Thanks, > Bhumika > > > julia > > > >> > >> > >> pHTInfo->bCurrent_AMSDU_Support = pHTInfo->bAMSDU_Support; > >> diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h > >> index b895a53..f2c28f3 100644 > >> --- a/drivers/staging/rtl8192e/rtllib.h > >> +++ b/drivers/staging/rtl8192e/rtllib.h > >> @@ -464,7 +464,7 @@ struct ieee_param { > >> #define RTLLIB_QCTL_TID 0x000F > >> > >> #define FC_QOS_BIT BIT7 > >> -#define IsDataFrame(pdu) (((pdu[0] & 0x0C) == 0x08) ? true : false) > >> +#define IsDataFrame(pdu) ((pdu[0] & 0x0C) == 0x08) > >> #define IsLegacyDataFrame(pdu) (IsDataFrame(pdu) && (!(pdu[0]&FC_QOS_BIT))) > >> #define IsQoSDataFrame(pframe) \ > >> ((*(u16 *)pframe&(RTLLIB_STYPE_QOS_DATA|RTLLIB_FTYPE_DATA)) == \ > >> -- > >> 1.9.1 > >> > >> -- > >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > >> To post to this group, send email to outreachy-kernel@googlegroups.com. > >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1475951297-10364-1-git-send-email-bhumirks%40gmail.com. > >> For more options, visit https://groups.google.com/d/optout. > >> > > -- > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAOH%2B1jGUWFyq5gJJq%3D1MhrjZjXyW0zeF4%2Bm8g%2BjvVPurerLg0w%40mail.gmail.com. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Staging: rtl8192e: Remove ternary operator 2016-10-08 18:28 [PATCH] Staging: rtl8192e: Remove ternary operator Bhumika Goyal 2016-10-09 6:00 ` [Outreachy kernel] " Julia Lawall @ 2016-10-09 14:26 ` Greg KH 2016-10-09 14:36 ` [Outreachy kernel] " Julia Lawall 1 sibling, 1 reply; 8+ messages in thread From: Greg KH @ 2016-10-09 14:26 UTC (permalink / raw) To: Bhumika Goyal; +Cc: outreachy-kernel On Sat, Oct 08, 2016 at 11:58:17PM +0530, Bhumika Goyal wrote: > Relational and logical operators evaluate to either true or false. > Explicit conversion is not needed so remove the ternary operator. > Done using coccinelle: > > @r@ > expression A,B; > symbol true,false; > binary operator b = {==,!=,&&,||,>=,<=,>,<}; > @@ > - (A b B) ? true : false > + A b B > > Signed-off-by: Bhumika Goyal <bhumirks@gmail.com> > --- > drivers/staging/rtl8192e/rtl819x_HTProc.c | 12 ++++-------- > drivers/staging/rtl8192e/rtllib.h | 2 +- > 2 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c > index dd9c0c8..8cd51a9 100644 > --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c > +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c > @@ -558,19 +558,15 @@ void HTOnAssocRsp(struct rtllib_device *ieee) > #endif > HTSetConnectBwMode(ieee, (enum ht_channel_width)(pPeerHTCap->ChlWidth), > (enum ht_extchnl_offset)(pPeerHTInfo->ExtChlOffset)); > - pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth == 1) ? > - true : false); > + pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1); Ugh, I _hate_ ? : statements. Just write the thing out, is this easy to read as is? > pHTInfo->bCurShortGI20MHz = ((pHTInfo->bRegShortGI20MHz) ? > - ((pPeerHTCap->ShortGI20Mhz == 1) ? > - true : false) : false); > + (pPeerHTCap->ShortGI20Mhz == 1) : false); > pHTInfo->bCurShortGI40MHz = ((pHTInfo->bRegShortGI40MHz) ? > - ((pPeerHTCap->ShortGI40Mhz == 1) ? > - true : false) : false); > + (pPeerHTCap->ShortGI40Mhz == 1) : false); Is this? Kernel code is meant to be read by humans first, and the compiler second. None of this will be any different if you write out a if () statement. Please just do that instead. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH] Staging: rtl8192e: Remove ternary operator 2016-10-09 14:26 ` Greg KH @ 2016-10-09 14:36 ` Julia Lawall 2016-10-09 14:46 ` Greg KH 0 siblings, 1 reply; 8+ messages in thread From: Julia Lawall @ 2016-10-09 14:36 UTC (permalink / raw) To: Greg KH; +Cc: Bhumika Goyal, outreachy-kernel On Sun, 9 Oct 2016, Greg KH wrote: > On Sat, Oct 08, 2016 at 11:58:17PM +0530, Bhumika Goyal wrote: > > Relational and logical operators evaluate to either true or false. > > Explicit conversion is not needed so remove the ternary operator. > > Done using coccinelle: > > > > @r@ > > expression A,B; > > symbol true,false; > > binary operator b = {==,!=,&&,||,>=,<=,>,<}; > > @@ > > - (A b B) ? true : false > > + A b B > > > > Signed-off-by: Bhumika Goyal <bhumirks@gmail.com> > > --- > > drivers/staging/rtl8192e/rtl819x_HTProc.c | 12 ++++-------- > > drivers/staging/rtl8192e/rtllib.h | 2 +- > > 2 files changed, 5 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c > > index dd9c0c8..8cd51a9 100644 > > --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c > > +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c > > @@ -558,19 +558,15 @@ void HTOnAssocRsp(struct rtllib_device *ieee) > > #endif > > HTSetConnectBwMode(ieee, (enum ht_channel_width)(pPeerHTCap->ChlWidth), > > (enum ht_extchnl_offset)(pPeerHTInfo->ExtChlOffset)); > > - pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth == 1) ? > > - true : false); > > + pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1); > > Ugh, I _hate_ ? : statements. > > Just write the thing out, is this easy to read as is? > > > pHTInfo->bCurShortGI20MHz = ((pHTInfo->bRegShortGI20MHz) ? > > - ((pPeerHTCap->ShortGI20Mhz == 1) ? > > - true : false) : false); > > + (pPeerHTCap->ShortGI20Mhz == 1) : false); > > pHTInfo->bCurShortGI40MHz = ((pHTInfo->bRegShortGI40MHz) ? > > - ((pPeerHTCap->ShortGI40Mhz == 1) ? > > - true : false) : false); > > + (pPeerHTCap->ShortGI40Mhz == 1) : false); > > Is this? Kernel code is meant to be read by humans first, and the > compiler second. None of this will be any different if you write out a > if () statement. Please just do that instead. Wouldn't && or || be nicer if possible? If the == 1 are actually testing booleans, it could be quite concise. julia > > thanks, > > greg k-h > > -- > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20161009142656.GA27065%40kroah.com. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH] Staging: rtl8192e: Remove ternary operator 2016-10-09 14:36 ` [Outreachy kernel] " Julia Lawall @ 2016-10-09 14:46 ` Greg KH 2016-10-09 16:21 ` Bhumika Goyal 0 siblings, 1 reply; 8+ messages in thread From: Greg KH @ 2016-10-09 14:46 UTC (permalink / raw) To: Julia Lawall; +Cc: Bhumika Goyal, outreachy-kernel On Sun, Oct 09, 2016 at 04:36:34PM +0200, Julia Lawall wrote: > > > On Sun, 9 Oct 2016, Greg KH wrote: > > > On Sat, Oct 08, 2016 at 11:58:17PM +0530, Bhumika Goyal wrote: > > > Relational and logical operators evaluate to either true or false. > > > Explicit conversion is not needed so remove the ternary operator. > > > Done using coccinelle: > > > > > > @r@ > > > expression A,B; > > > symbol true,false; > > > binary operator b = {==,!=,&&,||,>=,<=,>,<}; > > > @@ > > > - (A b B) ? true : false > > > + A b B > > > > > > Signed-off-by: Bhumika Goyal <bhumirks@gmail.com> > > > --- > > > drivers/staging/rtl8192e/rtl819x_HTProc.c | 12 ++++-------- > > > drivers/staging/rtl8192e/rtllib.h | 2 +- > > > 2 files changed, 5 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c > > > index dd9c0c8..8cd51a9 100644 > > > --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c > > > +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c > > > @@ -558,19 +558,15 @@ void HTOnAssocRsp(struct rtllib_device *ieee) > > > #endif > > > HTSetConnectBwMode(ieee, (enum ht_channel_width)(pPeerHTCap->ChlWidth), > > > (enum ht_extchnl_offset)(pPeerHTInfo->ExtChlOffset)); > > > - pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth == 1) ? > > > - true : false); > > > + pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1); > > > > Ugh, I _hate_ ? : statements. > > > > Just write the thing out, is this easy to read as is? > > > > > pHTInfo->bCurShortGI20MHz = ((pHTInfo->bRegShortGI20MHz) ? > > > - ((pPeerHTCap->ShortGI20Mhz == 1) ? > > > - true : false) : false); > > > + (pPeerHTCap->ShortGI20Mhz == 1) : false); > > > pHTInfo->bCurShortGI40MHz = ((pHTInfo->bRegShortGI40MHz) ? > > > - ((pPeerHTCap->ShortGI40Mhz == 1) ? > > > - true : false) : false); > > > + (pPeerHTCap->ShortGI40Mhz == 1) : false); > > > > Is this? Kernel code is meant to be read by humans first, and the > > compiler second. None of this will be any different if you write out a > > if () statement. Please just do that instead. > > Wouldn't && or || be nicer if possible? If the == 1 are actually testing > booleans, it could be quite concise. Yes, maybe, but never ? : if at all possible (as function arguments it makes sense.) thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH] Staging: rtl8192e: Remove ternary operator 2016-10-09 14:46 ` Greg KH @ 2016-10-09 16:21 ` Bhumika Goyal 0 siblings, 0 replies; 8+ messages in thread From: Bhumika Goyal @ 2016-10-09 16:21 UTC (permalink / raw) To: Greg KH; +Cc: Julia Lawall, outreachy-kernel On Sun, Oct 9, 2016 at 8:16 PM, Greg KH <gregkh@linuxfoundation.org> wrote: > On Sun, Oct 09, 2016 at 04:36:34PM +0200, Julia Lawall wrote: >> >> >> On Sun, 9 Oct 2016, Greg KH wrote: >> >> > On Sat, Oct 08, 2016 at 11:58:17PM +0530, Bhumika Goyal wrote: >> > > Relational and logical operators evaluate to either true or false. >> > > Explicit conversion is not needed so remove the ternary operator. >> > > Done using coccinelle: >> > > >> > > @r@ >> > > expression A,B; >> > > symbol true,false; >> > > binary operator b = {==,!=,&&,||,>=,<=,>,<}; >> > > @@ >> > > - (A b B) ? true : false >> > > + A b B >> > > >> > > Signed-off-by: Bhumika Goyal <bhumirks@gmail.com> >> > > --- >> > > drivers/staging/rtl8192e/rtl819x_HTProc.c | 12 ++++-------- >> > > drivers/staging/rtl8192e/rtllib.h | 2 +- >> > > 2 files changed, 5 insertions(+), 9 deletions(-) >> > > >> > > diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c >> > > index dd9c0c8..8cd51a9 100644 >> > > --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c >> > > +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c >> > > @@ -558,19 +558,15 @@ void HTOnAssocRsp(struct rtllib_device *ieee) >> > > #endif >> > > HTSetConnectBwMode(ieee, (enum ht_channel_width)(pPeerHTCap->ChlWidth), >> > > (enum ht_extchnl_offset)(pPeerHTInfo->ExtChlOffset)); >> > > - pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth == 1) ? >> > > - true : false); >> > > + pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1); >> > >> > Ugh, I _hate_ ? : statements. >> > >> > Just write the thing out, is this easy to read as is? >> > >> > > pHTInfo->bCurShortGI20MHz = ((pHTInfo->bRegShortGI20MHz) ? >> > > - ((pPeerHTCap->ShortGI20Mhz == 1) ? >> > > - true : false) : false); >> > > + (pPeerHTCap->ShortGI20Mhz == 1) : false); >> > > pHTInfo->bCurShortGI40MHz = ((pHTInfo->bRegShortGI40MHz) ? >> > > - ((pPeerHTCap->ShortGI40Mhz == 1) ? >> > > - true : false) : false); >> > > + (pPeerHTCap->ShortGI40Mhz == 1) : false); >> > >> > Is this? Kernel code is meant to be read by humans first, and the >> > compiler second. None of this will be any different if you write out a >> > if () statement. Please just do that instead. >> >> Wouldn't && or || be nicer if possible? If the == 1 are actually testing >> booleans, it could be quite concise. > > Yes, maybe, but never ? : if at all possible (as function arguments it > makes sense.) > Okay, I will convert all these ?: operators into if() statements. Thanks for the feedback Julia and Greg. Thanks, Bhumika > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-10-09 16:21 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-08 18:28 [PATCH] Staging: rtl8192e: Remove ternary operator Bhumika Goyal 2016-10-09 6:00 ` [Outreachy kernel] " Julia Lawall 2016-10-09 6:38 ` Bhumika Goyal 2016-10-09 10:51 ` Julia Lawall 2016-10-09 14:26 ` Greg KH 2016-10-09 14:36 ` [Outreachy kernel] " Julia Lawall 2016-10-09 14:46 ` Greg KH 2016-10-09 16:21 ` Bhumika Goyal
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.