* [PATCH 0/4] staging: r8188eu: trivial code cleanup patches
@ 2022-10-17 13:20 Deepak R Varma
2022-10-17 13:21 ` [PATCH 1/4] staging: r8188eu: use Linux kernel variable naming convention Deepak R Varma
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Deepak R Varma @ 2022-10-17 13:20 UTC (permalink / raw)
To: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel
Cc: kumarpraveen, saurabh.truth
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/4] staging: r8188eu: use Linux kernel variable naming convention
2022-10-17 13:20 [PATCH 0/4] staging: r8188eu: trivial code cleanup patches Deepak R Varma
@ 2022-10-17 13:21 ` Deepak R Varma
2022-10-17 13:56 ` Julia Lawall
2022-10-17 13:26 ` Deepak R Varma
` (2 subsequent siblings)
3 siblings, 1 reply; 23+ messages in thread
From: Deepak R Varma @ 2022-10-17 13:21 UTC (permalink / raw)
To: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel
Cc: kumarpraveen, saurabh.truth
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-17 13:20 [PATCH 0/4] staging: r8188eu: trivial code cleanup patches Deepak R Varma
@ 2022-10-17 13:26 ` Deepak R Varma
2022-10-17 13:26 ` Deepak R Varma
` (2 subsequent siblings)
3 siblings, 0 replies; 23+ messages in thread
From: Deepak R Varma @ 2022-10-17 13:22 UTC (permalink / raw)
To: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel
Cc: kumarpraveen, saurabh.truth
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/4] staging: r8188eu: remove {} for single statement blocks
2022-10-17 13:20 [PATCH 0/4] staging: r8188eu: trivial code cleanup patches Deepak R Varma
2022-10-17 13:21 ` [PATCH 1/4] staging: r8188eu: use Linux kernel variable naming convention Deepak R Varma
2022-10-17 13:26 ` Deepak R Varma
@ 2022-10-17 13:23 ` Deepak R Varma
2022-10-17 13:57 ` Julia Lawall
2022-10-17 13:24 ` [PATCH 4/4] staging: r8188eu: use htons macro instead of __constant_htons Deepak R Varma
3 siblings, 1 reply; 23+ messages in thread
From: Deepak R Varma @ 2022-10-17 13:23 UTC (permalink / raw)
To: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel
Cc: kumarpraveen, saurabh.truth
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/4] staging: r8188eu: use htons macro instead of __constant_htons
2022-10-17 13:20 [PATCH 0/4] staging: r8188eu: trivial code cleanup patches Deepak R Varma
` (2 preceding siblings ...)
2022-10-17 13:23 ` [PATCH 3/4] staging: r8188eu: remove {} for single statement blocks Deepak R Varma
@ 2022-10-17 13:24 ` Deepak R Varma
2022-10-19 6:08 ` Joe Perches
3 siblings, 1 reply; 23+ messages in thread
From: Deepak R Varma @ 2022-10-17 13:24 UTC (permalink / raw)
To: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel
Cc: kumarpraveen, saurabh.truth
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/4] staging: r8188eu: reformat long computation lines
@ 2022-10-17 13:26 ` Deepak R Varma
0 siblings, 0 replies; 23+ messages in thread
From: Deepak R Varma @ 2022-10-17 13:26 UTC (permalink / raw)
To: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel
Cc: kumarpraveen, saurabh.truth
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] staging: r8188eu: use Linux kernel variable naming convention
2022-10-17 13:21 ` [PATCH 1/4] staging: r8188eu: use Linux kernel variable naming convention Deepak R Varma
@ 2022-10-17 13:56 ` Julia Lawall
2022-10-17 14:12 ` Deepak R Varma
0 siblings, 1 reply; 23+ messages in thread
From: Julia Lawall @ 2022-10-17 13:56 UTC (permalink / raw)
To: Deepak R Varma
Cc: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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
>
>
>
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/4] staging: r8188eu: remove {} for single statement blocks
2022-10-17 13:23 ` [PATCH 3/4] staging: r8188eu: remove {} for single statement blocks Deepak R Varma
@ 2022-10-17 13:57 ` Julia Lawall
2022-10-17 14:13 ` Deepak R Varma
0 siblings, 1 reply; 23+ messages in thread
From: Julia Lawall @ 2022-10-17 13:57 UTC (permalink / raw)
To: Deepak R Varma
Cc: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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
>
>
>
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-17 13:26 ` Deepak R Varma
(?)
@ 2022-10-17 14:09 ` Greg KH
2022-10-17 14:10 ` Deepak R Varma
2022-10-18 11:21 ` David Laight
-1 siblings, 2 replies; 23+ messages in thread
From: Greg KH @ 2022-10-17 14:09 UTC (permalink / raw)
To: Deepak R Varma
Cc: outreachy, Larry.Finger, phil, paskripkin, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-17 14:09 ` Greg KH
@ 2022-10-17 14:10 ` Deepak R Varma
2022-10-17 14:52 ` Greg KH
2022-10-18 11:21 ` David Laight
1 sibling, 1 reply; 23+ messages in thread
From: Deepak R Varma @ 2022-10-17 14:10 UTC (permalink / raw)
To: Greg KH
Cc: outreachy, Larry.Finger, phil, paskripkin, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] staging: r8188eu: use Linux kernel variable naming convention
2022-10-17 13:56 ` Julia Lawall
@ 2022-10-17 14:12 ` Deepak R Varma
0 siblings, 0 replies; 23+ messages in thread
From: Deepak R Varma @ 2022-10-17 14:12 UTC (permalink / raw)
To: Julia Lawall
Cc: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/4] staging: r8188eu: remove {} for single statement blocks
2022-10-17 13:57 ` Julia Lawall
@ 2022-10-17 14:13 ` Deepak R Varma
0 siblings, 0 replies; 23+ messages in thread
From: Deepak R Varma @ 2022-10-17 14:13 UTC (permalink / raw)
To: Julia Lawall
Cc: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-17 14:10 ` Deepak R Varma
@ 2022-10-17 14:52 ` Greg KH
0 siblings, 0 replies; 23+ messages in thread
From: Greg KH @ 2022-10-17 14:52 UTC (permalink / raw)
To: Deepak R Varma
Cc: outreachy, Larry.Finger, phil, paskripkin, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-17 14:09 ` Greg KH
2022-10-17 14:10 ` Deepak R Varma
@ 2022-10-18 11:21 ` David Laight
2022-10-18 12:42 ` Deepak R Varma
1 sibling, 1 reply; 23+ messages in thread
From: David Laight @ 2022-10-18 11:21 UTC (permalink / raw)
To: 'Greg KH', Deepak R Varma
Cc: outreachy, Larry.Finger, phil, paskripkin, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-18 11:21 ` David Laight
@ 2022-10-18 12:42 ` Deepak R Varma
2022-10-19 5:43 ` Joe Perches
0 siblings, 1 reply; 23+ messages in thread
From: Deepak R Varma @ 2022-10-18 12:42 UTC (permalink / raw)
To: David Laight
Cc: 'Greg KH',
outreachy, Larry.Finger, phil, paskripkin, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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)
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-18 12:42 ` Deepak R Varma
@ 2022-10-19 5:43 ` Joe Perches
2022-10-19 6:17 ` Deepak R Varma
0 siblings, 1 reply; 23+ messages in thread
From: Joe Perches @ 2022-10-19 5:43 UTC (permalink / raw)
To: Deepak R Varma, David Laight
Cc: 'Greg KH',
outreachy, Larry.Finger, phil, paskripkin, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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);
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] staging: r8188eu: use htons macro instead of __constant_htons
2022-10-17 13:24 ` [PATCH 4/4] staging: r8188eu: use htons macro instead of __constant_htons Deepak R Varma
@ 2022-10-19 6:08 ` Joe Perches
2022-10-19 9:27 ` Deepak R Varma
2022-11-05 15:00 ` Deepak R Varma
0 siblings, 2 replies; 23+ messages in thread
From: Joe Perches @ 2022-10-19 6:08 UTC (permalink / raw)
To: Deepak R Varma, outreachy, Larry.Finger, phil, paskripkin,
gregkh, linux-staging, linux-kernel
Cc: kumarpraveen, saurabh.truth
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;
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-19 5:43 ` Joe Perches
@ 2022-10-19 6:17 ` Deepak R Varma
2022-10-19 6:38 ` Joe Perches
0 siblings, 1 reply; 23+ messages in thread
From: Deepak R Varma @ 2022-10-19 6:17 UTC (permalink / raw)
To: Joe Perches
Cc: David Laight, 'Greg KH',
outreachy, Larry.Finger, phil, paskripkin, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-19 6:17 ` Deepak R Varma
@ 2022-10-19 6:38 ` Joe Perches
2022-10-19 6:44 ` Deepak R Varma
0 siblings, 1 reply; 23+ messages in thread
From: Joe Perches @ 2022-10-19 6:38 UTC (permalink / raw)
To: Deepak R Varma
Cc: David Laight, 'Greg KH',
outreachy, Larry.Finger, phil, paskripkin, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-19 6:38 ` Joe Perches
@ 2022-10-19 6:44 ` Deepak R Varma
2022-10-19 9:02 ` Deepak R Varma
0 siblings, 1 reply; 23+ messages in thread
From: Deepak R Varma @ 2022-10-19 6:44 UTC (permalink / raw)
To: Joe Perches
Cc: David Laight, 'Greg KH',
outreachy, Larry.Finger, phil, paskripkin, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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.
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] staging: r8188eu: reformat long computation lines
2022-10-19 6:44 ` Deepak R Varma
@ 2022-10-19 9:02 ` Deepak R Varma
0 siblings, 0 replies; 23+ messages in thread
From: Deepak R Varma @ 2022-10-19 9:02 UTC (permalink / raw)
To: Joe Perches
Cc: David Laight, 'Greg KH',
outreachy, Larry.Finger, phil, paskripkin, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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
>
> >
>
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] staging: r8188eu: use htons macro instead of __constant_htons
2022-10-19 6:08 ` Joe Perches
@ 2022-10-19 9:27 ` Deepak R Varma
2022-11-05 15:00 ` Deepak R Varma
1 sibling, 0 replies; 23+ messages in thread
From: Deepak R Varma @ 2022-10-19 9:27 UTC (permalink / raw)
To: Joe Perches
Cc: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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.
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] staging: r8188eu: use htons macro instead of __constant_htons
2022-10-19 6:08 ` Joe Perches
2022-10-19 9:27 ` Deepak R Varma
@ 2022-11-05 15:00 ` Deepak R Varma
1 sibling, 0 replies; 23+ messages in thread
From: Deepak R Varma @ 2022-11-05 15:00 UTC (permalink / raw)
To: Joe Perches
Cc: outreachy, Larry.Finger, phil, paskripkin, gregkh, linux-staging,
linux-kernel, kumarpraveen, saurabh.truth
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;
>
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2022-11-05 15:00 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-17 13:20 [PATCH 0/4] staging: r8188eu: trivial code cleanup patches Deepak R Varma
2022-10-17 13:21 ` [PATCH 1/4] staging: r8188eu: use Linux kernel variable naming convention Deepak R Varma
2022-10-17 13:56 ` Julia Lawall
2022-10-17 14:12 ` Deepak R Varma
2022-10-17 13:22 ` [PATCH 2/4] staging: r8188eu: reformat long computation lines Deepak R Varma
2022-10-17 13:22 ` Deepak R Varma
2022-10-17 13:26 ` Deepak R Varma
2022-10-17 14:09 ` Greg KH
2022-10-17 14:10 ` Deepak R Varma
2022-10-17 14:52 ` Greg KH
2022-10-18 11:21 ` David Laight
2022-10-18 12:42 ` Deepak R Varma
2022-10-19 5:43 ` Joe Perches
2022-10-19 6:17 ` Deepak R Varma
2022-10-19 6:38 ` Joe Perches
2022-10-19 6:44 ` Deepak R Varma
2022-10-19 9:02 ` Deepak R Varma
2022-10-17 13:23 ` [PATCH 3/4] staging: r8188eu: remove {} for single statement blocks Deepak R Varma
2022-10-17 13:57 ` Julia Lawall
2022-10-17 14:13 ` Deepak R Varma
2022-10-17 13:24 ` [PATCH 4/4] staging: r8188eu: use htons macro instead of __constant_htons Deepak R Varma
2022-10-19 6:08 ` Joe Perches
2022-10-19 9:27 ` Deepak R Varma
2022-11-05 15:00 ` Deepak R Varma
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.