staging: r8188eu: trivial code cleanup patches
cover staging: r8188eu: trivial code cleanup patches
Address different kinds of checkpatch complains for the staging/r8188eu module. The patches are required to be applied in sequence.
Changes in v1:
1. Improve language / grammar for the patch descriptions
2. Further improve code reformatting
Deepak R Varma (4):
staging: r8188eu: use Linux kernel variable naming convention
staging: r8188eu: reformat long computation lines
staging: r8188eu: remove {} for single statement blocks
staging: r8188eu: use htons macro instead of __constant_htons
drivers/staging/r8188eu/core/rtw_br_ext.c | 122 +++++++++++----------- 1 file changed, 62 insertions(+), 60 deletions(-)
--
2.30.2
PATCH 1/4 staging: r8188eu: use Linux kernel variable naming convention
Follow the Linux Kernel coding style variable naming convention instead of using camelCase style. Issue reported by checkpatch script for these variables:
tagLen, tagType, networkAddr, ipAddr, macAddr
---
Changes in v1:
1. Improve patch description per feedback from [email protected]
drivers/staging/r8188eu/core/rtw_br_ext.c | 112 +++++++++++----------- 1 file changed, 56 insertions(+), 56 deletions(-)
drivers/staging/r8188eu/core/rtw_br_ext.c +56 -56
@@ -50,17 +50,17 @@static unsigned char *__nat25_find_pppoe_tag (struct pppoe_hdr *ph,unsigned short type) {unsigned char *cur_ptr, *start_ptr; -unsigned short tagLen, tagType; +unsigned short tag_len, tag_type; start_ptr = (unsigned char *)ph->tag; cur_ptr = (unsigned char *)ph->tag;while ((cur_ptr - start_ptr) <ntohs (ph->length)) {/* prevent un-alignment access */ - tagType = (unsigned short )((cur_ptr[0 ] <<8 ) + cur_ptr[1 ]); - tagLen = (unsigned short )((cur_ptr[2 ] <<8 ) + cur_ptr[3 ]); -if (tagType == type) + tag_type = (unsigned short )((cur_ptr[0 ] <<8 ) + cur_ptr[1 ]); + tag_len = (unsigned short )((cur_ptr[2 ] <<8 ) + cur_ptr[3 ]); +if (tag_type == type)return cur_ptr; - cur_ptr = cur_ptr +TAG_HDR_LEN + tagLen; + cur_ptr = cur_ptr +TAG_HDR_LEN + tag_len; }return NULL ; }
@@ -111,32 +111,32 @@ static int __nat25_has_expired(struct nat25_network_db_entry *fdb)return 0 ; } -static void __nat25_generate_ipv4_network_addr (unsigned char *networkAddr, -unsigned int *ipAddr) +static void __nat25_generate_ipv4_network_addr (unsigned char *network_addr, +unsigned int *ip_addr) { -memset (networkAddr,0 ,MAX_NETWORK_ADDR_LEN ); +memset (network_addr,0 ,MAX_NETWORK_ADDR_LEN ); - networkAddr[0 ] =NAT25_IPV4 ; -memcpy (networkAddr +7 , (unsigned char *)ipAddr,4 ); + network_addr[0 ] =NAT25_IPV4 ; +memcpy (network_addr +7 , (unsigned char *)ip_addr,4 ); } -static void __nat25_generate_pppoe_network_addr (unsigned char *networkAddr, +static void __nat25_generate_pppoe_network_addr (unsigned char *network_addr,unsigned char *ac_mac,__be16 *sid) { -memset (networkAddr,0 ,MAX_NETWORK_ADDR_LEN ); +memset (network_addr,0 ,MAX_NETWORK_ADDR_LEN ); - networkAddr[0 ] =NAT25_PPPOE ; -memcpy (networkAddr +1 , (unsigned char *)sid,2 ); -memcpy (networkAddr +3 , (unsigned char *)ac_mac,6 ); + network_addr[0 ] =NAT25_PPPOE ; +memcpy (network_addr +1 , (unsigned char *)sid,2 ); +memcpy (network_addr +3 , (unsigned char *)ac_mac,6 ); } -static void __nat25_generate_ipv6_network_addr (unsigned char *networkAddr, -unsigned int *ipAddr) +static void __nat25_generate_ipv6_network_addr (unsigned char *network_addr, +unsigned int *ip_addr) { -memset (networkAddr,0 ,MAX_NETWORK_ADDR_LEN ); +memset (network_addr,0 ,MAX_NETWORK_ADDR_LEN ); - networkAddr[0 ] =NAT25_IPV6 ; -memcpy (networkAddr +1 , (unsigned char *)ipAddr,16 ); + network_addr[0 ] =NAT25_IPV6 ; +memcpy (network_addr +1 , (unsigned char *)ip_addr,16 ); }static unsigned char *scan_tlv (unsigned char *data,int len,unsigned char tag,unsigned char len8b)
@@ -200,40 +200,40 @@ static int update_nd_link_layer_addr(unsigned char *data, int len, unsigned charreturn 0 ; } -static int __nat25_network_hash (unsigned char *networkAddr) +static int __nat25_network_hash (unsigned char *network_addr) { -if (networkAddr[0 ] ==NAT25_IPV4 ) { +if (network_addr[0 ] ==NAT25_IPV4 ) {unsigned long x; - x = networkAddr[7 ] ^ networkAddr[8 ] ^ networkAddr[9 ] ^ networkAddr[10 ]; + x = network_addr[7 ] ^ network_addr[8 ] ^ network_addr[9 ] ^ network_addr[10 ];return x & (NAT25_HASH_SIZE -1 ); - }else if (networkAddr[0 ] ==NAT25_IPX ) { + }else if (network_addr[0 ] ==NAT25_IPX ) {unsigned long x; - x = networkAddr[1 ] ^ networkAddr[2 ] ^ networkAddr[3 ] ^ networkAddr[4 ] ^ networkAddr[5 ] ^ - networkAddr[6 ] ^ networkAddr[7 ] ^ networkAddr[8 ] ^ networkAddr[9 ] ^ networkAddr[10 ]; + x = network_addr[1 ] ^ network_addr[2 ] ^ network_addr[3 ] ^ network_addr[4 ] ^ network_addr[5 ] ^ + network_addr[6 ] ^ network_addr[7 ] ^ network_addr[8 ] ^ network_addr[9 ] ^ network_addr[10 ];return x & (NAT25_HASH_SIZE -1 ); - }else if (networkAddr[0 ] ==NAT25_APPLE ) { + }else if (network_addr[0 ] ==NAT25_APPLE ) {unsigned long x; - x = networkAddr[1 ] ^ networkAddr[2 ] ^ networkAddr[3 ]; + x = network_addr[1 ] ^ network_addr[2 ] ^ network_addr[3 ];return x & (NAT25_HASH_SIZE -1 ); - }else if (networkAddr[0 ] ==NAT25_PPPOE ) { + }else if (network_addr[0 ] ==NAT25_PPPOE ) {unsigned long x; - x = networkAddr[0 ] ^ networkAddr[1 ] ^ networkAddr[2 ] ^ networkAddr[3 ] ^ networkAddr[4 ] ^ networkAddr[5 ] ^ networkAddr[6 ] ^ networkAddr[7 ] ^ networkAddr[8 ]; + x = network_addr[0 ] ^ network_addr[1 ] ^ network_addr[2 ] ^ network_addr[3 ] ^ network_addr[4 ] ^ network_addr[5 ] ^ network_addr[6 ] ^ network_addr[7 ] ^ network_addr[8 ];return x & (NAT25_HASH_SIZE -1 ); - }else if (networkAddr[0 ] ==NAT25_IPV6 ) { + }else if (network_addr[0 ] ==NAT25_IPV6 ) {unsigned long x; - x = networkAddr[1 ] ^ networkAddr[2 ] ^ networkAddr[3 ] ^ networkAddr[4 ] ^ networkAddr[5 ] ^ - networkAddr[6 ] ^ networkAddr[7 ] ^ networkAddr[8 ] ^ networkAddr[9 ] ^ networkAddr[10 ] ^ - networkAddr[11 ] ^ networkAddr[12 ] ^ networkAddr[13 ] ^ networkAddr[14 ] ^ networkAddr[15 ] ^ - networkAddr[16 ]; + x = network_addr[1 ] ^ network_addr[2 ] ^ network_addr[3 ] ^ network_addr[4 ] ^ network_addr[5 ] ^ + network_addr[6 ] ^ network_addr[7 ] ^ network_addr[8 ] ^ network_addr[9 ] ^ network_addr[10 ] ^ + network_addr[11 ] ^ network_addr[12 ] ^ network_addr[13 ] ^ network_addr[14 ] ^ network_addr[15 ] ^ + network_addr[16 ];return x & (NAT25_HASH_SIZE -1 ); }else {
@@ -241,7 +241,7 @@ static int __nat25_network_hash(unsigned char *networkAddr)int i;for (i =0 ; i <MAX_NETWORK_ADDR_LEN ; i++) - x ^= networkAddr[i]; + x ^= network_addr[i];return x & (NAT25_HASH_SIZE -1 ); }
@@ -269,17 +269,17 @@ static void __network_hash_unlink(struct nat25_network_db_entry *ent) }static void __nat25_db_network_insert (struct adapter *priv, -unsigned char *macAddr,unsigned char *networkAddr) +unsigned char *mac_addr,unsigned char *network_addr) {struct nat25_network_db_entry *db;int hash;spin_lock_bh (&priv->br_ext_lock); - hash =__nat25_network_hash (networkAddr); + hash =__nat25_network_hash (network_addr); db = priv->nethash[hash];while (db) { -if (!memcmp (db->networkAddr, networkAddr,MAX_NETWORK_ADDR_LEN )) { -memcpy (db->macAddr, macAddr,ETH_ALEN ); +if (!memcmp (db->networkAddr, network_addr,MAX_NETWORK_ADDR_LEN )) { +memcpy (db->macAddr, mac_addr,ETH_ALEN ); db->ageing_timer = jiffies;spin_unlock_bh (&priv->br_ext_lock);return ;
@@ -291,8 +291,8 @@ static void __nat25_db_network_insert(struct adapter *priv,spin_unlock_bh (&priv->br_ext_lock);return ; } -memcpy (db->networkAddr, networkAddr,MAX_NETWORK_ADDR_LEN ); -memcpy (db->macAddr, macAddr,ETH_ALEN ); +memcpy (db->networkAddr, network_addr,MAX_NETWORK_ADDR_LEN ); +memcpy (db->macAddr, mac_addr,ETH_ALEN );atomic_set (&db->use_count,1 ); db->ageing_timer = jiffies;
@@ -366,7 +366,7 @@ void nat25_db_expire(struct adapter *priv)int nat25_db_handle (struct adapter *priv,struct sk_buff *skb,int method) {unsigned short protocol; -unsigned char networkAddr[MAX_NETWORK_ADDR_LEN ]; +unsigned char network_addr[MAX_NETWORK_ADDR_LEN ];unsigned int tmp;if (!skb)
@@ -395,9 +395,9 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)if (iph->saddr ==0 )return 0 ; tmp =be32_to_cpu (iph->saddr); -__nat25_generate_ipv4_network_addr (networkAddr, &tmp); +__nat25_generate_ipv4_network_addr (network_addr, &tmp);/* record source IP address and , source mac address into db */ -__nat25_db_network_insert (priv, skb->data +ETH_ALEN , networkAddr); +__nat25_db_network_insert (priv, skb->data +ETH_ALEN , network_addr);return 0 ;default :return -1 ;
@@ -421,8 +421,8 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)memcpy (arp_ptr,GET_MY_HWADDR (priv),ETH_ALEN ); arp_ptr += arp->ar_hln; sender = (unsigned int *)arp_ptr; -__nat25_generate_ipv4_network_addr (networkAddr, sender); -__nat25_db_network_insert (priv, skb->data +ETH_ALEN , networkAddr); +__nat25_generate_ipv4_network_addr (network_addr, sender); +__nat25_db_network_insert (priv, skb->data +ETH_ALEN , network_addr);return 0 ;default :return -1 ;
@@ -495,9 +495,9 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)return -1 ; } }else {/* session phase */ -__nat25_generate_pppoe_network_addr (networkAddr, skb->data, &ph->sid); +__nat25_generate_pppoe_network_addr (network_addr, skb->data, &ph->sid); -__nat25_db_network_insert (priv, skb->data +ETH_ALEN , networkAddr); +__nat25_db_network_insert (priv, skb->data +ETH_ALEN , network_addr); if (!priv->ethBrExtInfo.addPPPoETag && priv->pppoe_connection_in_progress &&
@@ -548,8 +548,8 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)return -1 ;case NAT25_INSERT :if (memcmp (&iph->saddr,"\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0" ,16 )) { -__nat25_generate_ipv6_network_addr (networkAddr, (unsigned int *)&iph->saddr); -__nat25_db_network_insert (priv, skb->data +ETH_ALEN , networkAddr); +__nat25_generate_ipv6_network_addr (network_addr, (unsigned int *)&iph->saddr); +__nat25_db_network_insert (priv, skb->data +ETH_ALEN , network_addr);if (iph->nexthdr ==IPPROTO_ICMPV6 && skb->len > (ETH_HLEN +sizeof (*iph) +4 )) {
@@ -639,17 +639,17 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb) } } -void *scdb_findEntry (struct adapter *priv,unsigned char *ipAddr) +void *scdb_findEntry (struct adapter *priv,unsigned char *ip_addr) { -unsigned char networkAddr[MAX_NETWORK_ADDR_LEN ]; +unsigned char network_addr[MAX_NETWORK_ADDR_LEN ];struct nat25_network_db_entry *db;int hash; -__nat25_generate_ipv4_network_addr (networkAddr, (unsigned int *)ipAddr); - hash =__nat25_network_hash (networkAddr); +__nat25_generate_ipv4_network_addr (network_addr, (unsigned int *)ip_addr); + hash =__nat25_network_hash (network_addr); db = priv->nethash[hash];while (db) { -if (!memcmp (db->networkAddr, networkAddr,MAX_NETWORK_ADDR_LEN )) { +if (!memcmp (db->networkAddr, network_addr,MAX_NETWORK_ADDR_LEN )) {return (void *)db; }
--
2.30.2
PATCH 2/4 staging: r8188eu: reformat long computation lines
In reply to Deepak R VarmaReformat long running computation instructions to improve code readability. Address checkpatch script complaints like:
CHECK: line length of 171 exceeds 100 columns
---
Changes in v1:
1. Further improve the formatting per feedback from [email protected]
drivers/staging/r8188eu/core/rtw_br_ext.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
drivers/staging/r8188eu/core/rtw_br_ext.c +10 -7
@@ -211,8 +211,9 @@ static int __nat25_network_hash(unsigned char *network_addr) }else if (network_addr[0 ] ==NAT25_IPX ) {unsigned long x; - x = network_addr[1 ] ^ network_addr[2 ] ^ network_addr[3 ] ^ network_addr[4 ] ^ network_addr[5 ] ^ - network_addr[6 ] ^ network_addr[7 ] ^ network_addr[8 ] ^ network_addr[9 ] ^ network_addr[10 ]; + x = network_addr[1 ] ^ network_addr[2 ] ^ network_addr[3 ] ^ network_addr[4 ] ^ + network_addr[5 ] ^ network_addr[6 ] ^ network_addr[7 ] ^ network_addr[8 ] ^ + network_addr[9 ] ^ network_addr[10 ];return x & (NAT25_HASH_SIZE -1 ); }else if (network_addr[0 ] ==NAT25_APPLE ) {
@@ -224,16 +225,18 @@ static int __nat25_network_hash(unsigned char *network_addr) }else if (network_addr[0 ] ==NAT25_PPPOE ) {unsigned long x; - x = network_addr[0 ] ^ network_addr[1 ] ^ network_addr[2 ] ^ network_addr[3 ] ^ network_addr[4 ] ^ network_addr[5 ] ^ network_addr[6 ] ^ network_addr[7 ] ^ network_addr[8 ]; + x = network_addr[0 ] ^ network_addr[1 ] ^ network_addr[2 ] ^ network_addr[3 ] ^ + network_addr[4 ] ^ network_addr[5 ] ^ network_addr[6 ] ^ network_addr[7 ] ^ + network_addr[8 ];return x & (NAT25_HASH_SIZE -1 ); }else if (network_addr[0 ] ==NAT25_IPV6 ) {unsigned long x; - x = network_addr[1 ] ^ network_addr[2 ] ^ network_addr[3 ] ^ network_addr[4 ] ^ network_addr[5 ] ^ - network_addr[6 ] ^ network_addr[7 ] ^ network_addr[8 ] ^ network_addr[9 ] ^ network_addr[10 ] ^ - network_addr[11 ] ^ network_addr[12 ] ^ network_addr[13 ] ^ network_addr[14 ] ^ network_addr[15 ] ^ - network_addr[16 ]; + x = network_addr[1 ] ^ network_addr[2 ] ^ network_addr[3 ] ^ network_addr[4 ] ^ + network_addr[5 ] ^ network_addr[6 ] ^ network_addr[7 ] ^ network_addr[8 ] ^ + network_addr[9 ] ^ network_addr[10 ] ^ network_addr[11 ] ^ network_addr[12 ] ^ + network_addr[13 ] ^ network_addr[14 ] ^ network_addr[15 ] ^ network_addr[16 ];return x & (NAT25_HASH_SIZE -1 ); }else {
--
2.30.2
PATCH 3/4 staging: r8188eu: remove {} for single statement blocks
In reply to Deepak R VarmaAs per the Linux kernel coding-style guidelines, there is no need to use {} for single statement blocks. Issue flagged by checkpatch script.
---
Changes in v1:
1. Improve patch description language to make it simpler. Feedback received from [email protected]
drivers/staging/r8188eu/core/rtw_br_ext.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/r8188eu/core/rtw_br_ext.c +1 -2
@@ -652,9 +652,8 @@ void *scdb_findEntry(struct adapter *priv, unsigned char *ip_addr) hash =__nat25_network_hash (network_addr); db = priv->nethash[hash];while (db) { -if (!memcmp (db->networkAddr, network_addr,MAX_NETWORK_ADDR_LEN )) { +if (!memcmp (db->networkAddr, network_addr,MAX_NETWORK_ADDR_LEN ))return (void *)db; - } db = db->next_hash; }
--
2.30.2
PATCH 4/4 staging: r8188eu: use htons macro instead of __constant_htons
In reply to Deepak R VarmaMacro "htons" is more efficient and clearer. It should be used for constants instead of the __contast_htons macro. Resolves following checkpatch script complaint:
WARNING: __constant_htons should be htons
---
Changes in v1:
1. Correct spelling in patch description.
drivers/staging/r8188eu/core/rtw_br_ext.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/staging/r8188eu/core/rtw_br_ext.c +3 -3
@@ -609,14 +609,14 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb)if (!priv->ethBrExtInfo.dhcp_bcst_disable) {__be16 protocol = *((__be16 *)(skb->data +2 *ETH_ALEN )); -if (protocol ==__constant_htons (ETH_P_IP )) {/* IP */ +if (protocol ==htons (ETH_P_IP )) {/* IP */ struct iphdr *iph = (struct iphdr *)(skb->data +ETH_HLEN );if (iph->protocol ==IPPROTO_UDP ) {/* UDP */ struct udphdr *udph = (struct udphdr *)((size_t )iph + (iph->ihl <<2 )); -if ((udph->source ==__constant_htons (CLIENT_PORT )) && - (udph->dest ==__constant_htons (SERVER_PORT ))) {/* DHCP request */ +if ((udph->source ==htons (CLIENT_PORT )) && + (udph->dest ==htons (SERVER_PORT ))) {/* DHCP request */ struct dhcpMessage *dhcph = (struct dhcpMessage *)((size_t )udph +sizeof (struct udphdr ));u32 cookie =be32_to_cpu ((__be32 )dhcph->cookie);
--
2.30.2
Quoted 13 lines · Deepak R Varma
Address different kinds of checkpatch complains for the staging/r8188eu module. The patches are required to be applied in sequence.
Changes in v1:
1. Improve language / grammar for the patch descriptions
2. Further improve code reformattingDeepak R Varma (4):
staging: r8188eu: use Linux kernel variable naming convention
staging: r8188eu: reformat long computation lines
staging: r8188eu: remove {} for single statement blocks
staging: r8188eu: use htons macro instead of__constant_htons
Quoted 9 lines
drivers/staging/r8188eu/core/rtw_br_ext.c | 122 +++++++++++----------- 1 file changed, 62 insertions(+), 60 deletions(-)--
2.30.2
I think this patch series should be v2 as the first one was a v1. The next one should be the v3.
Please do variable changes driver wide and not only limited to a file. Example:
This line contains the old variable:
void *scdb_findEntry(struct adapter *priv, unsigned char *ipAddr);
But in this line you have already changed ip_addr.
void *scdb_findEntry(struct adapter *priv, unsigned char *ip_addr)
{
unsigned char network_addr[MAX_NETWORK_ADDR_LEN];
struct nat25_network_db_entry *db;
int hash;...
Please change all networkAddr and not only some.
Is it possible to changing __constant_htons as well in the entire driver?
Driver can be applied and compiled.
Tested device.
Thanks
Bye Philipp
Quoted 17 lines · Philipp Hortmann
Quoted 16 lines · Deepak R Varma
Address different kinds of checkpatch complains for the staging/r8188eu module. The patches are required to be applied in sequence.
Changes in v1:
1. Improve language / grammar for the patch descriptions
2. Further improve code reformattingDeepak R Varma (4):
staging: r8188eu: use Linux kernel variable naming convention
staging: r8188eu: reformat long computation lines
staging: r8188eu: remove {} for single statement blocks
staging: r8188eu: use htons macro instead of__constant_htonsdrivers/staging/r8188eu/core/rtw_br_ext.c | 122 +++++++++++----------- 1 file changed, 62 insertions(+), 60 deletions(-)
Quoted 6 lines
Quoted 6 lines
--
2.30.2
Hello Philipp,
Thank you for testing the patch set and the following feedback. Much appreciate.
Quoted 3 lines
I think this patch series should be v2 as the first one was a v1. The next one should be the v3.
Okay. I will switch it to v3 if there is opportunity for another revision of this patch set.
Quoted 6 lines
Please do variable changes driver wide and not only limited to a file. Example:
This line contains the old variable:
void *scdb_findEntry(struct adapter *priv, unsigned char *ipAddr);
Quoted 7 lines
But in this line you have already changed
ip_addr.
void *scdb_findEntry(struct adapter *priv, unsigned char*ip_addr){ unsigned char network_addr[MAX_NETWORK_ADDR_LEN]; struct nat25_network_db_entry *db; int hash;...
Since this is my first patch set, I wanted to be small and manageable. I also made changes only to function parameters and local variable. I am now comfortable to make similar change for the other files and symbols such as functions and structures. I will send in a separate patch set for the remaining files of this driver.
Quoted 3 lines
Please change all networkAddr and not only some.
For this file, I changed all networkAddr definitions except those that are part of global structure definition. I will include those changes in the next patch set.
Quoted 2 lines
Is it possible to changing
__constant_htonsas well in the entire driver?
Sure, in the next patch set.
Quoted 3 lines
Driver can be applied and compiled.
Tested device.
Thank you again.
./drv
Quoted 6 lines
Thanks
Bye Philipp