staging: rtl8192u: fix line over 80 characters
Subsystems (2)
PATCH 1/1 staging: rtl8192u: fix line over 80 characters
*fix checkpatch.pl warnings:
WARNING: line over 80 characters
--- drivers/staging/rtl8192u/r8192U_core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/staging/rtl8192u/r8192U_core.c +4 -1
@@ -2044,7 +2044,10 @@ static bool GetNmodeSupportBySecCfg8192(struct net_device *dev)return false; }else if ((wpa_ie_len !=0 )) {/* parse pairwise key type */ -if (((ieee->wpa_ie[0 ] =0xdd ) && (!memcmp (&(ieee->wpa_ie[14 ]), ccmp_ie,4 ))) || ((ieee->wpa_ie[0 ] =0x30 ) && (!memcmp (&ieee->wpa_ie[10 ], ccmp_rsn_ie,4 )))) +if (((ieee->wpa_ie[0 ] =0xdd ) && + (!memcmp (&(ieee->wpa_ie[14 ]), ccmp_ie,4 ))) || + ((ieee->wpa_ie[0 ] =0x30 ) && + (!memcmp (&ieee->wpa_ie[10 ], ccmp_rsn_ie,4 ))))return true;else return false;
Signature · 1 line
2.7.4
Quoted 2 lines · Hyunil Kim
*fix checkpatch.pl warnings:
WARNING: line over 80 characters
Describe what you do and why, rather than just saying "Fix". Often something can be fixed in multiple ways.
julia
Quoted 31 lines
Signed-off-by Hyunil Kim <[email protected]>--- drivers/staging/rtl8192u/r8192U_core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)drivers/staging/rtl8192u/r8192U_core.c +4
-2@@ -2044,7 +2044,10 @@ static bool GetNmodeSupportBySecCfg8192(struct net_device *dev)return false; }else if ((wpa_ie_len !=0 )) {/* parse pairwise key type */ -if (((ieee->wpa_ie[0 ] =0xdd ) && (!memcmp (&(ieee->wpa_ie[14 ]), ccmp_ie,4 ))) || ((ieee->wpa_ie[0 ] =0x30 ) && (!memcmp (&ieee->wpa_ie[10 ], ccmp_rsn_ie,4 )))) +if (((ieee->wpa_ie[0 ] =0xdd ) && + (!memcmp (&(ieee->wpa_ie[14 ]), ccmp_ie,4 ))) || + ((ieee->wpa_ie[0 ] =0x30 ) && + (!memcmp (&ieee->wpa_ie[10 ], ccmp_rsn_ie,4 ))))return true;else return false; --2.7.4
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
I would suggest giving the reader a chance to understand the code.
return false;
} else if ((wpa_ie_len != 0)) {
/* parse pairwise key type */
- if (((ieee->wpa_ie[0] = 0xdd) && (!memcmp(&(ieee->wpa_ie[14]), ccmp_ie, 4)))
|| ((ieee->wpa_ie[0] = 0x30) && (!memcmp(&ieee->wpa_ie[10], ccmp_rsn_ie, 4))))
+ if (((ieee->wpa_ie[0] = 0xdd) &&
+ (!memcmp(&(ieee->wpa_ie[14]), ccmp_ie, 4))) ||
+ ((ieee->wpa_ie[0] = 0x30) &&
+ (!memcmp(&ieee->wpa_ie[10], ccmp_rsn_ie, 4))))
return true;
else
return false;
my interpreation (read with out testing) of this code is:
if (ieee->wpa_ie[0] = 0xdd && (!memcmp(&(ieee->wpa_ie[14]), ccmp_ie, 4))
return true;
if (ieee->wpa_ie[0] = 0x30 && (!memcmp(&ieee->wpa_ie[10], ccmp_rsn_ie, 4) )
return true;
return false;
For this its much more clear what it intended but braces are still wired.
just my 2 cents
re,
wh
drivers/staging/rtl8192u/r8192U_core.c static bool GetNmodeSupportBySecCfg8192(struct net_device *dev) · line 2050
Quoted 7 lines · Hyunil Kim
*fix checkpatch.pl warnings:
WARNING: line over 80 charactersdrivers/staging/rtl8192u/r8192U_core.c +4
-0+if (((ieee->wpa_ie[0 ] ==0xdd ) && + (!memcmp (&(ieee->wpa_ie[14 ]), ccmp_ie,4 ))) || + ((ieee->wpa_ie[0 ] ==0x30 ) && + (!memcmp (&ieee->wpa_ie[10 ], ccmp_rsn_ie,4 ))))
Ouch. The && and || at ends of line mean a lot of parenthesis counting to figure out which goes with which. Use additional indentation to help understanding.
+ if (((ieee->wpa_ie[0] == 0xdd) &&
+ (!memcmp(&(ieee->wpa_ie[14]), ccmp_ie, 4))) ||
+ ((ieee->wpa_ie[0] == 0x30) &&
+ (!memcmp(&ieee->wpa_ie[10], ccmp_rsn_ie, 4))))
Or perhaps
+ if (((ieee->wpa_ie[0] == 0xdd) &&
+ (!memcmp(&(ieee->wpa_ie[14]), ccmp_ie, 4)))
+ || ((ieee->wpa_ie[0] == 0x30) &&
+ (!memcmp(&ieee->wpa_ie[10], ccmp_rsn_ie, 4))))
Quoted 19 lines · On Sat, 2018-06-16 at 15:22 -0400, [email protected] wrote:
Review
Quoted 6 lines · Hyunil Kim
*fix checkpatch.pl warnings:
WARNING: line over 80 characters(quoted diff) +4
-0+ if (((ieee->wpa_ie[0] = 0xdd) && + (!memcmp(&(ieee->wpa_ie[14]), ccmp_ie, 4))) || + ((ieee->wpa_ie[0] = 0x30) && + (!memcmp(&ieee->wpa_ie[10], ccmp_rsn_ie, 4))))Ouch. The && and || at ends of line mean a lot of parenthesis counting to figure out which goes with which. Use additional indentation to help understanding.
(quoted diff) +4
-0+ if (((ieee->wpa_ie[0] = 0xdd) && + (!memcmp(&(ieee->wpa_ie[14]), ccmp_ie, 4))) || + ((ieee->wpa_ie[0] = 0x30) && + (!memcmp(&ieee->wpa_ie[10], ccmp_rsn_ie, 4))))Or perhaps
Review
Quoted 4 lines
(quoted diff) +4
-0+ if (((ieee->wpa_ie[0] = 0xdd) && + (!memcmp(&(ieee->wpa_ie[14]), ccmp_ie, 4))) + || ((ieee->wpa_ie[0] = 0x30) && + (!memcmp(&ieee->wpa_ie[10], ccmp_rsn_ie, 4))))
Or perhaps refactor to something like:
--- drivers/staging/rtl8192u/r8192U_core.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
drivers/staging/rtl8192u/r8192U_core.c +9 -11
@@ -2039,20 +2039,18 @@ static bool GetNmodeSupportBySecCfg8192(struct net_device *dev) (strcmp(crypt->ops->name, "WEP") = 0));/* simply judge */ -if (encrypt && (wpa_ie_len =0 )) { -/* wep encryption, no N mode setting */ +/* wep encryption, no N mode setting */ +if (encrypt && wpa_ie_len =0 )return false; - }else if ((wpa_ie_len !=0 )) { -/* parse pairwise key type */ -if (((ieee->wpa_ie[0 ] =0xdd ) && (!memcmp (&(ieee->wpa_ie[14 ]), ccmp_ie,4 ))) || ((ieee->wpa_ie[0 ] =0x30 ) && (!memcmp (&ieee->wpa_ie[10 ], ccmp_rsn_ie,4 )))) -return true; -else -return false; - }else { + +if (wpa_ie_len =0 )return true; - } -return true; +/* parse pairwise key type */ +return (ieee->wpa_ie[0 ] =0xdd && + !memcmp (&ieee->wpa_ie[14 ], ccmp_ie,4 )) || + (ieee->wpa_ie[0 ] =0x30 && + !memcmp (&ieee->wpa_ie[10 ], ccmp_rsn_ie,4 )); }static bool GetHalfNmodeSupportByAPs819xUsb (struct net_device *dev)