[PATCH] staging: rtl8723bs: core: fix line over 80 characters warning

[PATCH] staging: rtl8723bs: core: fix line over 80 characters warning

[Anirudh Rayabharam]

Shorten the expression by re-using the part that was already computed to
fix the line over 80 characters warning reported by checkpatch.pl.

Signed-off-by: Anirudh Rayabharam <anirudh.rayabharam@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_ap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
index 18fabf5ff44b..bc0230672457 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ap.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
@@ -2336,8 +2336,8 @@ void rtw_ap_restore_network(struct adapter *padapter)
 			Update_RA_Entry(padapter, psta);
 			/* pairwise key */
 			/* per sta pairwise key and settings */
-			if ((padapter->securitypriv.dot11PrivacyAlgrthm == _TKIP_) ||
-				(padapter->securitypriv.dot11PrivacyAlgrthm == _AES_)) {
+			if ((psecuritypriv->dot11PrivacyAlgrthm == _TKIP_) ||
+				(psecuritypriv->dot11PrivacyAlgrthm == _AES_)) {
 				rtw_setstakey_cmd(padapter, psta, true, false);
 			}
 		}
-- 
2.17.1

Re: [PATCH] staging: rtl8723bs: core: fix line over 80 characters warning

[Dan Carpenter]

On Tue, Mar 26, 2019 at 11:55:07PM +0530, Anirudh Rayabharam wrote:
> Shorten the expression by re-using the part that was already computed to

This confused me.  Better to phrase it like:

Shorten the expression by using the "psecuritypriv" pointer.

> fix the line over 80 characters warning reported by checkpatch.pl.
> 
> Signed-off-by: Anirudh Rayabharam <anirudh.rayabharam@gmail.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_ap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
> index 18fabf5ff44b..bc0230672457 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_ap.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
> @@ -2336,8 +2336,8 @@ void rtw_ap_restore_network(struct adapter *padapter)
>  			Update_RA_Entry(padapter, psta);
>  			/* pairwise key */
>  			/* per sta pairwise key and settings */
> -			if ((padapter->securitypriv.dot11PrivacyAlgrthm == _TKIP_) ||
> -				(padapter->securitypriv.dot11PrivacyAlgrthm == _AES_)) {
> +			if ((psecuritypriv->dot11PrivacyAlgrthm == _TKIP_) ||
> +				(psecuritypriv->dot11PrivacyAlgrthm == _AES_)) {

It's better to align it slightly different as well.  In the kernel we
would normally align the second condition to match the first one.

I probably would have gotten rid of the parenthesis as well.  I don't
like double parenthesis around == because I reserve that for =
assignment conditions.

			if (psecuritypriv->dot11PrivacyAlgrthm == _TKIP_ ||
			    psecuritypriv->dot11PrivacyAlgrthm == _AES_) {
				rtw_setstakey_cmd(padapter, psta, true, false);

When you're changing just a couple lines like this you can get away with
making multiple white space changes at the same time because the One
Thing that the patch does is "Clean up a Condition".  There is some
flexibility in the One thing Per Patch rule, but you have to sell it in
the right way.  The patch description would be:

  Checkpatch.pl complains that this line is over 80 characters.  We
  should use the "psecuritypriv" for consistency.  It's not aligned
  properly and there are too many parenthesis.

  This patch just cleans up a condition, it doesn't affect runtime.

regards,
dan carpenter

[PATCH v2] staging: rtl8723bs: core: fix line over 80 characters warning

[Anirudh Rayabharam]

Checkpatch.pl complains that these lines are over 80 characters. Use the
"psecuritypriv" pointer for consistency, remove unnecessary parantheses
and fix the alignment.

This patch just cleans up a condition, it doesn't affect runtime.

Signed-off-by: Anirudh Rayabharam <anirudh.rayabharam@gmail.com>
---
v2: Made the commit message clearer, removed unnecessary parantheses and fixed
    the alignment as suggested by Dan Carpenter <dan.carpenter@oracle.com>

 drivers/staging/rtl8723bs/core/rtw_ap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
index 18fabf5ff44b..8062b7f36de2 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ap.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
@@ -2336,8 +2336,8 @@ void rtw_ap_restore_network(struct adapter *padapter)
 			Update_RA_Entry(padapter, psta);
 			/* pairwise key */
 			/* per sta pairwise key and settings */
-			if ((padapter->securitypriv.dot11PrivacyAlgrthm == _TKIP_) ||
-				(padapter->securitypriv.dot11PrivacyAlgrthm == _AES_)) {
+			if (psecuritypriv->dot11PrivacyAlgrthm == _TKIP_ ||
+			    psecuritypriv->dot11PrivacyAlgrthm == _AES_) {
 				rtw_setstakey_cmd(padapter, psta, true, false);
 			}
 		}
-- 
2.17.1

Re: [PATCH v2] staging: rtl8723bs: core: fix line over 80 characters warning

[Dan Carpenter]

Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

Re: [PATCH v2] staging: rtl8723bs: core: fix line over 80 characters warning

[Joe Perches]

On Wed, 2019-03-27 at 23:49 +0530, Anirudh Rayabharam wrote:
> Checkpatch.pl complains that these lines are over 80 characters. Use the
> "psecuritypriv" pointer for consistency, remove unnecessary parantheses
> and fix the alignment.
> 
> This patch just cleans up a condition, it doesn't affect runtime.
[]
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
[]
> @@ -2336,8 +2336,8 @@ void rtw_ap_restore_network(struct adapter *padapter)
>  			Update_RA_Entry(padapter, psta);
>  			/* pairwise key */
>  			/* per sta pairwise key and settings */
> -			if ((padapter->securitypriv.dot11PrivacyAlgrthm == _TKIP_) ||
> -				(padapter->securitypriv.dot11PrivacyAlgrthm == _AES_)) {
> +			if (psecuritypriv->dot11PrivacyAlgrthm == _TKIP_ ||
> +			    psecuritypriv->dot11PrivacyAlgrthm == _AES_) {
>  				rtw_setstakey_cmd(padapter, psta, true, false);
>  			}

You could remove the unnecessary {} braces here too

Re: [PATCH v2] staging: rtl8723bs: core: fix line over 80 characters warning

[Greg KH]

On Wed, Mar 27, 2019 at 11:49:07PM +0530, Anirudh Rayabharam wrote:
> Checkpatch.pl complains that these lines are over 80 characters. Use the
> "psecuritypriv" pointer for consistency, remove unnecessary parantheses
> and fix the alignment.
> 
> This patch just cleans up a condition, it doesn't affect runtime.
> 
> Signed-off-by: Anirudh Rayabharam <anirudh.rayabharam@gmail.com>
> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Made the commit message clearer, removed unnecessary parantheses and fixed
>     the alignment as suggested by Dan Carpenter <dan.carpenter@oracle.com>
> 
>  drivers/staging/rtl8723bs/core/rtw_ap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
> index 18fabf5ff44b..8062b7f36de2 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_ap.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
> @@ -2336,8 +2336,8 @@ void rtw_ap_restore_network(struct adapter *padapter)
>  			Update_RA_Entry(padapter, psta);
>  			/* pairwise key */
>  			/* per sta pairwise key and settings */
> -			if ((padapter->securitypriv.dot11PrivacyAlgrthm == _TKIP_) ||
> -				(padapter->securitypriv.dot11PrivacyAlgrthm == _AES_)) {
> +			if (psecuritypriv->dot11PrivacyAlgrthm == _TKIP_ ||
> +			    psecuritypriv->dot11PrivacyAlgrthm == _AES_) {
>  				rtw_setstakey_cmd(padapter, psta, true, false);
>  			}
>  		}

Patch does not apply to my staging-next branch :(

Please rebase and resend.

thanks,

greg k-h

Re: [PATCH v2] staging: rtl8723bs: core: fix line over 80 characters warning

[Anirudh Rayabharam]

On Wed, Mar 27, 2019 at 11:49:07PM +0530, Anirudh Rayabharam wrote:
> Checkpatch.pl complains that these lines are over 80 characters. Use the
> "psecuritypriv" pointer for consistency, remove unnecessary parantheses
> and fix the alignment.
> 
> This patch just cleans up a condition, it doesn't affect runtime.
> 
> Signed-off-by: Anirudh Rayabharam <anirudh.rayabharam@gmail.com>
> ---
> v2: Made the commit message clearer, removed unnecessary parantheses and fixed
>     the alignment as suggested by Dan Carpenter <dan.carpenter@oracle.com>
> 
>  drivers/staging/rtl8723bs/core/rtw_ap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
> index 18fabf5ff44b..8062b7f36de2 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_ap.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
> @@ -2336,8 +2336,8 @@ void rtw_ap_restore_network(struct adapter *padapter)
>  			Update_RA_Entry(padapter, psta);
>  			/* pairwise key */
>  			/* per sta pairwise key and settings */
> -			if ((padapter->securitypriv.dot11PrivacyAlgrthm == _TKIP_) ||
> -				(padapter->securitypriv.dot11PrivacyAlgrthm == _AES_)) {
> +			if (psecuritypriv->dot11PrivacyAlgrthm == _TKIP_ ||
> +			    psecuritypriv->dot11PrivacyAlgrthm == _AES_) {
>  				rtw_setstakey_cmd(padapter, psta, true, false);
>  			}
>  		}
> -- 
> 2.17.1
> 

Unfortunately, the v2 of this patch was missed and v1 was applied to Greg's
staging tree. So, I am abandoning v2 of this patch. I will fix the alignment
and braces issue in a new patch.

Re: [PATCH] staging: rtl8723bs: core: fix line over 80 characters warning

[Mukesh Ojha]

On 3/26/2019 11:55 PM, Anirudh Rayabharam wrote:
> Shorten the expression by re-using the part that was already computed to
> fix the line over 80 characters warning reported by checkpatch.pl.
>
> Signed-off-by: Anirudh Rayabharam <anirudh.rayabharam@gmail.com>
> ---
>   drivers/staging/rtl8723bs/core/rtw_ap.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
> index 18fabf5ff44b..bc0230672457 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_ap.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
> @@ -2336,8 +2336,8 @@ void rtw_ap_restore_network(struct adapter *padapter)
>   			Update_RA_Entry(padapter, psta);
>   			/* pairwise key */
>   			/* per sta pairwise key and settings */
> -			if ((padapter->securitypriv.dot11PrivacyAlgrthm == _TKIP_) ||
> -				(padapter->securitypriv.dot11PrivacyAlgrthm == _AES_)) {
> +			if ((psecuritypriv->dot11PrivacyAlgrthm == _TKIP_) ||
> +				(psecuritypriv->dot11PrivacyAlgrthm == _AES_)) {
>   				rtw_setstakey_cmd(padapter, psta, true, false);
>   			}


Get rid of this {}.fix this .

Now patch looks good after Dan comment.
Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>

Cheers,
-Mukesh

>   		}