[PATCH 0/4] staging: r8188eu: trivial code cleanup patches

[PATCH 0/4] staging: r8188eu: trivial code cleanup patches

[Deepak R Varma]

Address different kinds of checkpatch complains for the staging/r8188eu module.
The patches are required to be applied in sequence.


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 | 125 +++++++++++-----------
 1 file changed, 65 insertions(+), 60 deletions(-)

--
2.30.2

[PATCH 1/4] staging: r8188eu: use Linux kernel variable naming convention

[Deepak R Varma]

Follow Linux Kernel coding style variable naming convention instead of using
camelCase style. Address following checkpatch script complaints:
	CHECK: Avoid CamelCase: <tagLen>
	CHECK: Avoid CamelCase: <tagType>
	CHECK: Avoid CamelCase: <networkAddr>
	CHECK: Avoid CamelCase: <ipAddr>
	CHECK: Avoid CamelCase: <macAddr>

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 drivers/staging/r8188eu/core/rtw_br_ext.c | 112 +++++++++++-----------
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index 4c5f30792a46..79daf8f269d6 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -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 char
 	return 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

Re: [PATCH 1/4] staging: r8188eu: use Linux kernel variable naming convention

[Julia Lawall]

On Mon, 17 Oct 2022, Deepak R Varma wrote:

> Follow Linux Kernel coding style variable naming convention instead of using

Follow -> Follow the

> camelCase style. Address following checkpatch script complaints:

Address following -> Address the following

> 	CHECK: Avoid CamelCase: <tagLen>
> 	CHECK: Avoid CamelCase: <tagType>
> 	CHECK: Avoid CamelCase: <networkAddr>
> 	CHECK: Avoid CamelCase: <ipAddr>
> 	CHECK: Avoid CamelCase: <macAddr>

Copy pasting the checkpatch output doesn't really need necessary.  You
could just say This affects the variables bla, bla', bla'', etc.

julia

> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
>  drivers/staging/r8188eu/core/rtw_br_ext.c | 112 +++++++++++-----------
>  1 file changed, 56 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> index 4c5f30792a46..79daf8f269d6 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -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 char
>  	return 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
>
>
>
>
>

Re: [PATCH 1/4] staging: r8188eu: use Linux kernel variable naming convention

[Deepak R Varma]

On Mon, Oct 17, 2022 at 03:56:02PM +0200, Julia Lawall wrote:
>
>
> On Mon, 17 Oct 2022, Deepak R Varma wrote:
>
> > Follow Linux Kernel coding style variable naming convention instead of using
>
> Follow -> Follow the
>
> > camelCase style. Address following checkpatch script complaints:
>
> Address following -> Address the following
>
> > 	CHECK: Avoid CamelCase: <tagLen>
> > 	CHECK: Avoid CamelCase: <tagType>
> > 	CHECK: Avoid CamelCase: <networkAddr>
> > 	CHECK: Avoid CamelCase: <ipAddr>
> > 	CHECK: Avoid CamelCase: <macAddr>
>
> Copy pasting the checkpatch output doesn't really need necessary.  You
> could just say This affects the variables bla, bla', bla'', etc.

Sounds good. I will include the suggestions in next version.

Thank you Julia.
./drv

>
> julia
>
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> >  drivers/staging/r8188eu/core/rtw_br_ext.c | 112 +++++++++++-----------
> >  1 file changed, 56 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > index 4c5f30792a46..79daf8f269d6 100644
> > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > @@ -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 char
> >  	return 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 3/4] staging: r8188eu: remove {} for single statement blocks

[Deepak R Varma]

As per the Linux kernel coding-style guidelines, there is no need to
use {} for single statement blocks. Address following checkpatch script
complaint:
	WARNING: braces {} are not necessary for single statement blocks

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 drivers/staging/r8188eu/core/rtw_br_ext.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index 427da7e8ba4c..290affe50d0b 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -655,9 +655,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

Re: [PATCH 3/4] staging: r8188eu: remove {} for single statement blocks

[Julia Lawall]

On Mon, 17 Oct 2022, Deepak R Varma wrote:

> As per the Linux kernel coding-style guidelines, there is no need to
> use {} for single statement blocks. Address following checkpatch script
> complaint:
> 	WARNING: braces {} are not necessary for single statement blocks

It's nice to say something like "Problem identified using checkpatch".
But putting the verbatim checkpatch message that says what you just said
doesn't seem necessary.

julia

>
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
>  drivers/staging/r8188eu/core/rtw_br_ext.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> index 427da7e8ba4c..290affe50d0b 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -655,9 +655,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
>
>
>
>
>

Re: [PATCH 3/4] staging: r8188eu: remove {} for single statement blocks

[Deepak R Varma]

On Mon, Oct 17, 2022 at 03:57:16PM +0200, Julia Lawall wrote:
>
>
> On Mon, 17 Oct 2022, Deepak R Varma wrote:
>
> > As per the Linux kernel coding-style guidelines, there is no need to
> > use {} for single statement blocks. Address following checkpatch script
> > complaint:
> > 	WARNING: braces {} are not necessary for single statement blocks
>
> It's nice to say something like "Problem identified using checkpatch".
> But putting the verbatim checkpatch message that says what you just said
> doesn't seem necessary.

Understood. That sounds better. Thank you Julia. Will include your feedback in
the next revision.

./drv

>
> julia
>
> >
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> >  drivers/staging/r8188eu/core/rtw_br_ext.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > index 427da7e8ba4c..290affe50d0b 100644
> > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > @@ -655,9 +655,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

[Deepak R Varma]

Macro "htons" is more efficiant 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

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 drivers/staging/r8188eu/core/rtw_br_ext.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index 290affe50d0b..334360de23da 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -612,14 +612,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

Re: [PATCH 4/4] staging: r8188eu: use htons macro instead of __constant_htons

[Joe Perches]

On Mon, 2022-10-17 at 18:54 +0530, Deepak R Varma wrote:
> Macro "htons" is more efficiant and clearer. It should be used for
> constants instead of the __contast_htons macro. Resolves following

typo: __constant_htons

> checkpatch script complaint:
> 	WARNING: __constant_htons should be htons
[]
> diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
[]
> @@ -612,14 +612,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 */

OK, this bit seems fine

>  					struct dhcpMessage *dhcph =
>  						(struct dhcpMessage *)((size_t)udph + sizeof(struct udphdr));

IMO: this existing code however is ugly.
     Casting a pointer to a size_t isn't great.

Perhaps:

				struct dhcpMessage *dhcp;

				dhcp = (void *)udhp + sizeof(struct udphdr);

in a separate patch.

> 					u32 cookie = be32_to_cpu((__be32)dhcph->cookie);

And dhcph->cookie already is a __be32 so the cast is pointless.

drivers/staging/r8188eu/core/rtw_br_ext.c-598-  __be32 cookie;

Re: [PATCH 4/4] staging: r8188eu: use htons macro instead of __constant_htons

[Deepak R Varma]

On Tue, Oct 18, 2022 at 11:08:06PM -0700, Joe Perches wrote:
> On Mon, 2022-10-17 at 18:54 +0530, Deepak R Varma wrote:
> > Macro "htons" is more efficiant and clearer. It should be used for
> > constants instead of the __contast_htons macro. Resolves following
>
> typo: __constant_htons
>
> > checkpatch script complaint:
> > 	WARNING: __constant_htons should be htons
> []
> > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> []
> > @@ -612,14 +612,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 */
>
> OK, this bit seems fine
>
> >  					struct dhcpMessage *dhcph =
> >  						(struct dhcpMessage *)((size_t)udph + sizeof(struct udphdr));
>
> IMO: this existing code however is ugly.
>      Casting a pointer to a size_t isn't great.
>
> Perhaps:
>
> 				struct dhcpMessage *dhcp;
>
> 				dhcp = (void *)udhp + sizeof(struct udphdr);
>
> in a separate patch.
>
> > 					u32 cookie = be32_to_cpu((__be32)dhcph->cookie);
>
> And dhcph->cookie already is a __be32 so the cast is pointless.
>
> drivers/staging/r8188eu/core/rtw_br_ext.c-598-  __be32 cookie;

Thank you Joe. I will include this code clean up as separates patches are resend
the patch set.

>
>

Re: [PATCH 4/4] staging: r8188eu: use htons macro instead of __constant_htons

[Deepak R Varma]

On Tue, Oct 18, 2022 at 11:08:06PM -0700, Joe Perches wrote:
> On Mon, 2022-10-17 at 18:54 +0530, Deepak R Varma wrote:
> > Macro "htons" is more efficiant and clearer. It should be used for
> > constants instead of the __contast_htons macro. Resolves following
>
> typo: __constant_htons
>
> > checkpatch script complaint:
> > 	WARNING: __constant_htons should be htons
> []
> > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> []
> > @@ -612,14 +612,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 */
>
> OK, this bit seems fine
>
> >  					struct dhcpMessage *dhcph =
> >  						(struct dhcpMessage *)((size_t)udph + sizeof(struct udphdr));
>
> IMO: this existing code however is ugly.
>      Casting a pointer to a size_t isn't great.

Hello Joe,
Other thank looking ugly, is there any impact / risk associated with such
casting? I tried to look for the reasons myself but did not find anything
relevant or to the point.

Thank you,
./drv

>
> Perhaps:
>
> 				struct dhcpMessage *dhcp;
>
> 				dhcp = (void *)udhp + sizeof(struct udphdr);
>
> in a separate patch.
>
> > 					u32 cookie = be32_to_cpu((__be32)dhcph->cookie);
>
> And dhcph->cookie already is a __be32 so the cast is pointless.
>
> drivers/staging/r8188eu/core/rtw_br_ext.c-598-  __be32 cookie;
>

[PATCH 2/4] staging: r8188eu: reformat long computation lines

[Deepak R Varma]

Reformat long running computation instructions to improve code readability.
Address following checkpatch script complaints:
	CHECK: line length of 171 exceeds 100 columns
	CHECK: line length of 113 exceeds 100 columns

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index 79daf8f269d6..427da7e8ba4c 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -211,8 +211,10 @@ 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 +226,20 @@ 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 2/4] staging: r8188eu: reformat long computation lines

[Deepak R Varma]

Reformat long running computation instructions to improve code readability.
Address following checkpatch script complaints:
	CHECK: line length of 171 exceeds 100 columns
	CHECK: line length of 113 exceeds 100 columns

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index 79daf8f269d6..427da7e8ba4c 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -211,8 +211,10 @@ 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 +226,20 @@ 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

Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines

[Greg KH]

On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> Reformat long running computation instructions to improve code readability.
> Address following checkpatch script complaints:
> 	CHECK: line length of 171 exceeds 100 columns
> 	CHECK: line length of 113 exceeds 100 columns
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
>  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> index 79daf8f269d6..427da7e8ba4c 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -211,8 +211,10 @@ 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] ^

Why not go out to [4] here and then you are one line shorter?

> +		    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 +226,20 @@ 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] ^

Same here


> +		    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];

And here.

thanks,

greg k-h

Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines

[Deepak R Varma]

On Mon, Oct 17, 2022 at 04:09:49PM +0200, Greg KH wrote:
> On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > Reformat long running computation instructions to improve code readability.
> > Address following checkpatch script complaints:
> > 	CHECK: line length of 171 exceeds 100 columns
> > 	CHECK: line length of 113 exceeds 100 columns
> >
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> >  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > index 79daf8f269d6..427da7e8ba4c 100644
> > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > @@ -211,8 +211,10 @@ 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] ^
>
> Why not go out to [4] here and then you are one line shorter?

Thank you for the feedback.
Arranging 4 on a line still made the line extend 90+ columns. It appeared wide
to me. Stacking three in one line made it look better.
I will resend the patch with 4 in line and let you suggest if that is
acceptable.

Thank you,
./drv

>
> > +		    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 +226,20 @@ 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] ^
>
> Same here
>
>
> > +		    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];
>
> And here.
>
> thanks,
>
> greg k-h
>

Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines

[Greg KH]

On Mon, Oct 17, 2022 at 07:40:46PM +0530, Deepak R Varma wrote:
> On Mon, Oct 17, 2022 at 04:09:49PM +0200, Greg KH wrote:
> > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > Reformat long running computation instructions to improve code readability.
> > > Address following checkpatch script complaints:
> > > 	CHECK: line length of 171 exceeds 100 columns
> > > 	CHECK: line length of 113 exceeds 100 columns
> > >
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > >  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
> > >  1 file changed, 13 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > index 79daf8f269d6..427da7e8ba4c 100644
> > > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > @@ -211,8 +211,10 @@ 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] ^
> >
> > Why not go out to [4] here and then you are one line shorter?
> 
> Thank you for the feedback.
> Arranging 4 on a line still made the line extend 90+ columns.

As the tool said, you can go up to 100 columns.

thanks,

greg k-h

RE: [PATCH 2/4] staging: r8188eu: reformat long computation lines

[David Laight]

From: Greg KH
> Sent: 17 October 2022 15:10
> 
> On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > Reformat long running computation instructions to improve code readability.
> > Address following checkpatch script complaints:
> > 	CHECK: line length of 171 exceeds 100 columns
> > 	CHECK: line length of 113 exceeds 100 columns
> >
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> >  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > index 79daf8f269d6..427da7e8ba4c 100644
> > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > @@ -211,8 +211,10 @@ 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] ^
> 
> Why not go out to [4] here and then you are one line shorter?

and/or use a shorter variable name....

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines

[Deepak R Varma]

On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> From: Greg KH
> > Sent: 17 October 2022 15:10
> >
> > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > Reformat long running computation instructions to improve code readability.
> > > Address following checkpatch script complaints:
> > > 	CHECK: line length of 171 exceeds 100 columns
> > > 	CHECK: line length of 113 exceeds 100 columns
> > >
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > >  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 +++++++++++++-------
> > >  1 file changed, 13 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > index 79daf8f269d6..427da7e8ba4c 100644
> > > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > @@ -211,8 +211,10 @@ 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] ^
> >
> > Why not go out to [4] here and then you are one line shorter?
>
> and/or use a shorter variable name....
Hi David,
I have already re-submitted the patch set with 4 in line arrangement. Do you
still suggest using shorter variable names?

Thank you,
./drv

>
> 	David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
>

Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines

[Joe Perches]

On Tue, 2022-10-18 at 18:12 +0530, Deepak R Varma wrote:
> On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> > From: Greg KH
> > > Sent: 17 October 2022 15:10
> > > 
> > > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > > Reformat long running computation instructions to improve code readability.
> > > > Address following checkpatch script complaints:
> > > > 	CHECK: line length of 171 exceeds 100 columns
> > > > 	CHECK: line length of 113 exceeds 100 columns
[]
> > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
[]
> > > > @@ -211,8 +211,10 @@ 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] ^
> > > 
> > > Why not go out to [4] here and then you are one line shorter?
> > 
> > and/or use a shorter variable name....
> Hi David,
> I have already re-submitted the patch set with 4 in line arrangement. Do you
> still suggest using shorter variable names?

Assuming this code is not performance sensitive, I suggest not just
molifying checkpatch but perhaps improving the code by adding a helper
function something like:

u8 xor_array_u8(u8 *x, size_t len)
{
	size_t i;
	u8 xor = x[0];

	for (i = 1; i < len; i++)
		xor ^= x[i];

	return xor;
}

so for instance this could be:

		x = xor_array_u8(&network_addr[1], 10);

Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines

[Deepak R Varma]

On Tue, Oct 18, 2022 at 10:43:07PM -0700, Joe Perches wrote:
> On Tue, 2022-10-18 at 18:12 +0530, Deepak R Varma wrote:
> > On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> > > From: Greg KH
> > > > Sent: 17 October 2022 15:10
> > > >
> > > > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > > > Reformat long running computation instructions to improve code readability.
> > > > > Address following checkpatch script complaints:
> > > > > 	CHECK: line length of 171 exceeds 100 columns
> > > > > 	CHECK: line length of 113 exceeds 100 columns
> []
> > > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> []
> > > > > @@ -211,8 +211,10 @@ 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] ^
> > > >
> > > > Why not go out to [4] here and then you are one line shorter?
> > >
> > > and/or use a shorter variable name....
> > Hi David,
> > I have already re-submitted the patch set with 4 in line arrangement. Do you
> > still suggest using shorter variable names?
>
> Assuming this code is not performance sensitive, I suggest not just
> molifying checkpatch but perhaps improving the code by adding a helper
> function something like:
>
> u8 xor_array_u8(u8 *x, size_t len)
> {
> 	size_t i;
> 	u8 xor = x[0];
>
> 	for (i = 1; i < len; i++)
> 		xor ^= x[i];
>
> 	return xor;
> }
>
> so for instance this could be:
>
> 		x = xor_array_u8(&network_addr[1], 10);
>

Hi Joe,
Great suggestion. Thank you.
Is there a way to confirm that this improvement won't impact performance? Will I
need any specific hardware / device to run tests?

./drv

Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines

[Joe Perches]

On Wed, 2022-10-19 at 11:47 +0530, Deepak R Varma wrote:
> On Tue, Oct 18, 2022 at 10:43:07PM -0700, Joe Perches wrote:
> > On Tue, 2022-10-18 at 18:12 +0530, Deepak R Varma wrote:
> > > On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> > > > From: Greg KH
> > > > > Sent: 17 October 2022 15:10
> > > > > 
> > > > > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > > > > Reformat long running computation instructions to improve code readability.
> > > > > > Address following checkpatch script complaints:
> > > > > > 	CHECK: line length of 171 exceeds 100 columns
> > > > > > 	CHECK: line length of 113 exceeds 100 columns
> > []
> > > > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > []
> > > > > > @@ -211,8 +211,10 @@ 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] ^
> > > > > 
> > > > > Why not go out to [4] here and then you are one line shorter?
> > > > 
> > > > and/or use a shorter variable name....
> > > Hi David,
> > > I have already re-submitted the patch set with 4 in line arrangement. Do you
> > > still suggest using shorter variable names?
> > 
> > Assuming this code is not performance sensitive, I suggest not just
> > molifying checkpatch but perhaps improving the code by adding a helper
> > function something like:
> > 
> > u8 xor_array_u8(u8 *x, size_t len)
> > {
> > 	size_t i;
> > 	u8 xor = x[0];
> > 
> > 	for (i = 1; i < len; i++)
> > 		xor ^= x[i];
> > 
> > 	return xor;
> > }
> > 
> > so for instance this could be:
> > 
> > 		x = xor_array_u8(&network_addr[1], 10);
> > 
> 
> Hi Joe,
> Great suggestion. Thank you.
> Is there a way to confirm that this improvement won't impact performance? Will I
> need any specific hardware / device to run tests?

I suggest reading the code to see if the uses are in some fast path.

Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines

[Deepak R Varma]

On Tue, Oct 18, 2022 at 11:38:22PM -0700, Joe Perches wrote:
> On Wed, 2022-10-19 at 11:47 +0530, Deepak R Varma wrote:
> > On Tue, Oct 18, 2022 at 10:43:07PM -0700, Joe Perches wrote:
> > > On Tue, 2022-10-18 at 18:12 +0530, Deepak R Varma wrote:
> > > > On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> > > > > From: Greg KH
> > > > > > Sent: 17 October 2022 15:10
> > > > > >
> > > > > > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > > > > > Reformat long running computation instructions to improve code readability.
> > > > > > > Address following checkpatch script complaints:
> > > > > > > 	CHECK: line length of 171 exceeds 100 columns
> > > > > > > 	CHECK: line length of 113 exceeds 100 columns
> > > []
> > > > > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > []
> > > > > > > @@ -211,8 +211,10 @@ 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] ^
> > > > > >
> > > > > > Why not go out to [4] here and then you are one line shorter?
> > > > >
> > > > > and/or use a shorter variable name....
> > > > Hi David,
> > > > I have already re-submitted the patch set with 4 in line arrangement. Do you
> > > > still suggest using shorter variable names?
> > >
> > > Assuming this code is not performance sensitive, I suggest not just
> > > molifying checkpatch but perhaps improving the code by adding a helper
> > > function something like:
> > >
> > > u8 xor_array_u8(u8 *x, size_t len)
> > > {
> > > 	size_t i;
> > > 	u8 xor = x[0];
> > >
> > > 	for (i = 1; i < len; i++)
> > > 		xor ^= x[i];
> > >
> > > 	return xor;
> > > }
> > >
> > > so for instance this could be:
> > >
> > > 		x = xor_array_u8(&network_addr[1], 10);
> > >
> >
> > Hi Joe,
> > Great suggestion. Thank you.
> > Is there a way to confirm that this improvement won't impact performance? Will I
> > need any specific hardware / device to run tests?
>
> I suggest reading the code to see if the uses are in some fast path.

Sounds good. Thank you for your guidance.

>

Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines

[Deepak R Varma]

On Wed, Oct 19, 2022 at 12:14:38PM +0530, Deepak R Varma wrote:
> On Tue, Oct 18, 2022 at 11:38:22PM -0700, Joe Perches wrote:
> > On Wed, 2022-10-19 at 11:47 +0530, Deepak R Varma wrote:
> > > On Tue, Oct 18, 2022 at 10:43:07PM -0700, Joe Perches wrote:
> > > > On Tue, 2022-10-18 at 18:12 +0530, Deepak R Varma wrote:
> > > > > On Tue, Oct 18, 2022 at 11:21:26AM +0000, David Laight wrote:
> > > > > > From: Greg KH
> > > > > > > Sent: 17 October 2022 15:10
> > > > > > >
> > > > > > > On Mon, Oct 17, 2022 at 06:52:50PM +0530, Deepak R Varma wrote:
> > > > > > > > Reformat long running computation instructions to improve code readability.
> > > > > > > > Address following checkpatch script complaints:
> > > > > > > > 	CHECK: line length of 171 exceeds 100 columns
> > > > > > > > 	CHECK: line length of 113 exceeds 100 columns
> > > > []
> > > > > > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > > > []
> > > > > > > > @@ -211,8 +211,10 @@ 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] ^
> > > > > > >
> > > > > > > Why not go out to [4] here and then you are one line shorter?
> > > > > >
> > > > > > and/or use a shorter variable name....
> > > > > Hi David,
> > > > > I have already re-submitted the patch set with 4 in line arrangement. Do you
> > > > > still suggest using shorter variable names?
> > > >
> > > > Assuming this code is not performance sensitive, I suggest not just
> > > > molifying checkpatch but perhaps improving the code by adding a helper
> > > > function something like:
> > > >
> > > > u8 xor_array_u8(u8 *x, size_t len)
> > > > {
> > > > 	size_t i;
> > > > 	u8 xor = x[0];
> > > >
> > > > 	for (i = 1; i < len; i++)
> > > > 		xor ^= x[i];
> > > >
> > > > 	return xor;
> > > > }
> > > >
> > > > so for instance this could be:
> > > >
> > > > 		x = xor_array_u8(&network_addr[1], 10);
> > > >
> > >
> > > Hi Joe,
> > > Great suggestion. Thank you.
> > > Is there a way to confirm that this improvement won't impact performance? Will I
> > > need any specific hardware / device to run tests?
> >
> > I suggest reading the code to see if the uses are in some fast path.
>
> Sounds good. Thank you for your guidance.

Hi Joe,
based on the code review so far, I am unable to determine if the chain of
function calls are part of any fast path. There is not enough code comments or
documentation available with this code.

Considering my Outreachy patch submission targets and timelines, I am unable to
spend much time on this research right now; unless an expert can confirm it is
okay to add the routine you outlined. Else, I will put this in on my TODO list
and revisit when I have time.

R8188EU maintainers / experts,
Can you confirm if it is sensible to implement the helper function suggested by
Joe? If yes, I will include the improvement in my current patch set and resubmit
the set for review.

Thank you,
./drv






>
> >
>
>
>