[PATCH v4] staging: rtl8192u: Remove ternary operator

[PATCH v4] staging: rtl8192u: Remove ternary operator

[Gargi Sharma]

Relational and logical operators evaluate to either true or false.
Lines with ternary operators were found using coccinelle script. In a
few cases using logical && operator would suffice. Hence those were
changed to improve readability.

Coccinelle Script:
@r@
expression A,B;
symbol true,false;
binary operator b = {==,!=,&&,||,>=,<=,>,<};
@@
- (A b B) ? true : false
+ A b B

Signed-off-by: Gargi Sharma <gs051095@gmail.com>
---
Changes in v4:
        - Aligned operands for && and added a newline.
Changes in v3:
        - Converted ? operator to &&.
Changes in v2:
        - Converted ? operator to if else statements.

---
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index c27397b..b17bdf9 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -977,16 +977,18 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)
 	HTSetConnectBwMode(ieee, (HT_CHANNEL_WIDTH)(pPeerHTCap->ChlWidth), (HT_EXTCHNL_OFFSET)(pPeerHTInfo->ExtChlOffset));
 
 //	if (pHTInfo->bCurBW40MHz)
-		pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth == 1)?true:false);
+
+	pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1);
 
 	//
 	// Update short GI/ long GI setting
 	//
 	// TODO:
-	pHTInfo->bCurShortGI20MHz=
-		((pHTInfo->bRegShortGI20MHz)?((pPeerHTCap->ShortGI20Mhz==1)?true:false):false);
-	pHTInfo->bCurShortGI40MHz=
-		((pHTInfo->bRegShortGI40MHz)?((pPeerHTCap->ShortGI40Mhz==1)?true:false):false);
+	pHTInfo->bCurShortGI20MHz = pHTInfo->bRegShortGI20MHz &&
+				    (pPeerHTCap->ShortGI20Mhz == 1);
+
+	pHTInfo->bCurShortGI40MHz = pHTInfo->bRegShortGI40MHz &&
+				    (pPeerHTCap->ShortGI40Mhz == 1);
 
 	//
 	// Config TX STBC setting
@@ -997,9 +999,8 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)
 	// Config DSSS/CCK  mode in 40MHz mode
 	//
 	// TODO:
-	pHTInfo->bCurSuppCCK =
-		((pHTInfo->bRegSuppCCK)?((pPeerHTCap->DssCCk==1)?true:false):false);
-
+	pHTInfo->bCurSuppCCK = pHTInfo->bRegSuppCCK &&
+			       (pPeerHTCap->DssCCk == 1);
 
 	//
 	// Config and configure A-MSDU setting
-- 
2.7.4

Re: [PATCH v4] staging: rtl8192u: Remove ternary operator

[Greg KH]

On Mon, Feb 20, 2017 at 12:51:42PM +0530, Gargi Sharma wrote:
> Relational and logical operators evaluate to either true or false.
> Lines with ternary operators were found using coccinelle script. In a
> few cases using logical && operator would suffice. Hence those were
> changed to improve readability.
> 
> Coccinelle Script:
> @r@
> expression A,B;
> symbol true,false;
> binary operator b = {==,!=,&&,||,>=,<=,>,<};
> @@
> - (A b B) ? true : false
> + A b B
> 
> Signed-off-by: Gargi Sharma <gs051095@gmail.com>
> ---
> Changes in v4:
>         - Aligned operands for && and added a newline.
> Changes in v3:
>         - Converted ? operator to &&.
> Changes in v2:
>         - Converted ? operator to if else statements.
> 
> ---
> ---
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> index c27397b..b17bdf9 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> @@ -977,16 +977,18 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)
>  	HTSetConnectBwMode(ieee, (HT_CHANNEL_WIDTH)(pPeerHTCap->ChlWidth), (HT_EXTCHNL_OFFSET)(pPeerHTInfo->ExtChlOffset));
>  
>  //	if (pHTInfo->bCurBW40MHz)
> -		pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth == 1)?true:false);
> +
> +	pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1);

While the indentation does make sense, it does not look correct with the
if statement still there, right?  Please fix this up when you resend
this.

thanks,

greg k-h

Re: [PATCH v4] staging: rtl8192u: Remove ternary operator

[Gargi Sharma]

[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]

On Fri, Feb 24, 2017 at 10:53 PM, Greg KH <gregkh@linuxfoundation.org>
wrote:
>
> On Mon, Feb 20, 2017 at 12:51:42PM +0530, Gargi Sharma wrote:
> > Relational and logical operators evaluate to either true or false.
> > Lines with ternary operators were found using coccinelle script. In a
> > few cases using logical && operator would suffice. Hence those were
> > changed to improve readability.
> >
> > Coccinelle Script:
> > @r@
> > expression A,B;
> > symbol true,false;
> > binary operator b = {==,!=,&&,||,>=,<=,>,<};
> > @@
> > - (A b B) ? true : false
> > + A b B
> >
> > Signed-off-by: Gargi Sharma <gs051095@gmail.com>
> > ---
> > Changes in v4:
> >         - Aligned operands for && and added a newline.
> > Changes in v3:
> >         - Converted ? operator to &&.
> > Changes in v2:
> >         - Converted ? operator to if else statements.
> >
> > ---
> > ---
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 17
+++++++++--------
> >  1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> > index c27397b..b17bdf9 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> > +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> > @@ -977,16 +977,18 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)
> >       HTSetConnectBwMode(ieee, (HT_CHANNEL_WIDTH)(pPeerHTCap->ChlWidth),
(HT_EXTCHNL_OFFSET)(pPeerHTInfo->ExtChlOffset));
> >
if (pHTInfo->bCurBW40MHz)
Is this the if statement you are talking about? Should I remove this or fix
the alignment for the next statement?

Thanks,
Gargi

> > -             pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth
== 1)?true:false);
> > +
> > +     pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1);
>
> While the indentation does make sense, it does not look correct with the
> if statement still there, right?  Please fix this up when you resend
> this.
>
> thanks,
>
> greg k-h

[-- Attachment #2: Type: text/html, Size: 2766 bytes --]

Re: [Outreachy kernel] Re: [PATCH v4] staging: rtl8192u: Remove ternary operator

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 3136 bytes --]



On Sun, 26 Feb 2017, Gargi Sharma wrote:

> On Fri, Feb 24, 2017 at 10:53 PM, Greg KH <gregkh@linuxfoundation.org>
> wrote:
> >
> > On Mon, Feb 20, 2017 at 12:51:42PM +0530, Gargi Sharma wrote:
> > > Relational and logical operators evaluate to either true or false.
> > > Lines with ternary operators were found using coccinelle script. In a
> > > few cases using logical && operator would suffice. Hence those were
> > > changed to improve readability.
> > >
> > > Coccinelle Script:
> > > @r@
> > > expression A,B;
> > > symbol true,false;
> > > binary operator b = {==,!=,&&,||,>=,<=,>,<};
> > > @@
> > > - (A b B) ? true : false
> > > + A b B
> > >
> > > Signed-off-by: Gargi Sharma <gs051095@gmail.com>
> > > ---
> > > Changes in v4:
> > >         - Aligned operands for && and added a newline.
> > > Changes in v3:
> > >         - Converted ? operator to &&.
> > > Changes in v2:
> > >         - Converted ? operator to if else statements.
> > >
> > > ---
> > > ---
> > >  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 17
> +++++++++--------
> > >  1 file changed, 9 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> > > index c27397b..b17bdf9 100644
> > > --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> > > +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> > > @@ -977,16 +977,18 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)
> > >       HTSetConnectBwMode(ieee, (HT_CHANNEL_WIDTH)(pPeerHTCap->ChlWidth),
> (HT_EXTCHNL_OFFSET)(pPeerHTInfo->ExtChlOffset));
> > >if (pHTInfo->bCurBW40MHz)Is this the if statement you are talking about?
> Should I remove this or fix the alignment for the next statement?
>
> Thanks,
> Gargi
>
> > > -             pHTInfo->bCurTxBW40MHz = ((pPeerHTInfo->RecommemdedTxWidth
> == 1)?true:false);
> > > +
> > > +     pHTInfo->bCurTxBW40MHz = (pPeerHTInfo->RecommemdedTxWidth == 1);

The code just above was originally below an if that was preceeded by a //.
Ad you can see just above, you reduced the indentation, to reflect that
the code is not really under an if any more.  Greg thought it looked odd.
You can just get rid of the if under //.  In general, the policy in the
Linux kernel is that there should not be code under comments.  It should
just be removed.

julia

> >
> > While the indentation does make sense, it does not look correct with the
> > if statement still there, right?  Please fix this up when you resend
> > this.
> >
> > 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 visithttps://groups.google.com/d/msgid/outreachy-kernel/CAOCi2DE6hbgroFNfo5P8hDx
> gMxk2JCez1zADarnZVCL2uJrCCw%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>