Skip to contentSkip to conversation

staging: r8188eu: fix too many leading tabs

PATCH Superseded Joash Naidoo 4 participants 5 messages Sep 19, 2022 – Sep 27, 2022 Staging

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 staging: r8188eu: fix too many leading tabs

Coding style fix. Fix too many leading tabs and line length.

---
Changes in v3:
- Fix flipped condition mistake
- move skb NULL check before dereferencing it
Changes in v2:
- Flip additional nested if conditions and don't reverse the last if statement
- Move declarations to start of function
- Separate converting __constant_htons to htons to another patch

---
 drivers/staging/r8188eu/core/rtw_br_ext.c | 69 ++++++++++++-----------
 1 file changed, 36 insertions(+), 33 deletions(-)
drivers/staging/r8188eu/core/rtw_br_ext.c +36 -33
index bca20fe5c..833488dd5 100644
@@ -604,39 +604,42 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb)
 	if (!skb)
 		return;
 
-	if (!priv->ethBrExtInfo.dhcp_bcst_disable) {
-		__be16 protocol = *((__be16 *)(skb->data + 2 * ETH_ALEN));
-
-		if (protocol == __constant_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 */
-					struct dhcpMessage *dhcph =
-						(struct dhcpMessage *)((size_t)udph + sizeof(struct udphdr));
-					u32 cookie = be32_to_cpu((__be32)dhcph->cookie);
-
-					if (cookie == DHCP_MAGIC) { /*  match magic word */
-						if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
-							/*  if not broadcast */
-							register int sum = 0;
-
-							/*  or BROADCAST flag */
-							dhcph->flags |= htons(BROADCAST_FLAG);
-							/*  recalculate checksum */
-							sum = ~(udph->check) & 0xffff;
-							sum += be16_to_cpu(dhcph->flags);
-							while (sum >> 16)
-								sum = (sum & 0xffff) + (sum >> 16);
-							udph->check = ~sum;
-						}
-					}
-				}
-			}
-		}
+	__be16 protocol = *((__be16 *)(skb->data + 2 * ETH_ALEN));
+	struct iphdr *iph = (struct iphdr *)(skb->data + ETH_HLEN);
+	struct udphdr *udph = (struct udphdr *)((size_t)iph + (iph->ihl << 2));
+	/*  DHCP request */
+	struct dhcpMessage *dhcph =
+		(struct dhcpMessage *)((size_t)udph + sizeof(struct udphdr));
+	u32 cookie = be32_to_cpu((__be32)dhcph->cookie);
+
+	if (priv->ethBrExtInfo.dhcp_bcst_disable)
+		return;
+
+	if (protocol != htons(ETH_P_IP)) /*  IP */
+		return;
+
+	if (iph->protocol != IPPROTO_UDP) /*  UDP */
+		return;
+
+	if ((udph->source != __constant_htons(CLIENT_PORT)) ||
+	    (udph->dest != __constant_htons(SERVER_PORT)))
+		return;
+
+	if (cookie != DHCP_MAGIC) /*  match magic word */
+		return;
+
+	if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
+		/*  if not broadcast */
+		register int sum = 0;
+
+		/*  or BROADCAST flag */
+		dhcph->flags |= htons(BROADCAST_FLAG);
+		/*  recalculate checksum */
+		sum = ~(udph->check) & 0xffff;
+		sum += be16_to_cpu(dhcph->flags);
+		while (sum >> 16)
+			sum = (sum & 0xffff) + (sum >> 16);
+		udph->check = ~sum;
 	}
 }
 
Signature · 1 line
2.35.1
maintainer
#1 lore raw
Quoted 11 lines · Joash Naidoo

Coding style fix. Fix too many leading tabs and line length.

---
Changes in v3:
- Fix flipped condition mistake
- move skb NULL check before dereferencing it
Changes in v2:
- Flip additional nested if conditions and don't reverse the last if statement
- Move declarations to start of function
- Separate converting __constant_htons to htons to another patch

Always test-build your changes before sending them out so you do not get grumpy emails from maintainers asking why you didn't test-build your changes...

thanks,

greg k-h

#2 lore raw
Quoted 15 lines · Greg KH
Quoted 13 lines · Joash Naidoo

Coding style fix. Fix too many leading tabs and line length.

---
Changes in v3:
- Fix flipped condition mistake
- move skb NULL check before dereferencing it
Changes in v2:
- Flip additional nested if conditions and don't reverse the last if statement
- Move declarations to start of function
- Separate converting __constant_htons to htons to another patch

Quoted 7 lines

Always test-build your changes before sending them out so you do not get grumpy emails from maintainers asking why you didn't test-build your changes...

thanks,

greg k-h

Hi Greg,

Indeed building and testing is a vital step before sending changes. Is this message referring to my V3 patch not building? Or referring to my earlier versions?

Before sending I built my changes with:

make M=drivers/staging/r8188eu clean
make M=drivers/staging/r8188eu

I have a TP-Link Wifi dongle (Model: TL-WN725N) which I used to test. I enabled the driver as module with make menuconfig, built, installed (make modules_install && make install) and booted the modified kernel image (vmlinuz-6.0.0-rc5+). Then I ensured my wpa_supplicant service runs fine and then just checked if I could still connect to Wifi and browse the web. (I understand this is limited, since I didn't catch my error in V2) Furthermore, I also ran:

dmesg -t -l crit
dmesg -t -l warn
dmesg -t -l err

I have only gotten the following relevant warning:

r8188eu: module is from the staging directory, the quality is unknown, you have been warned.

If there is anything I still may be lacking, please advise. Thank you for your support.

Kind Regards,
Joash

#3 lore raw
Quoted 20 lines · Joash Naidoo
Quoted 17 lines · Greg KH
Quoted 11 lines · Joash Naidoo

Coding style fix. Fix too many leading tabs and line length.

---
Changes in v3:
    - Fix flipped condition mistake
    - move skb NULL check before dereferencing it
Changes in v2:
    - Flip additional nested if conditions and don't reverse     the last if statement
    - Move declarations to start of function
    - Separate converting __constant_htons to htons to another     patch

Always test-build your changes before sending them out so you do not get grumpy emails from maintainers asking why you didn't test-build your changes...

Quoted 9 lines
Quoted 3 lines

thanks,

greg k-h

Hi Greg,

Indeed building and testing is a vital step before sending changes. Is this message referring to my V3 patch not building? Or referring to my earlier versions?

Hi Joash,

v3 introduces a gcc warning.

drivers/staging/r8188eu/core/rtw_br_ext.c: In function 'dhcp_flag_bcast':
drivers/staging/r8188eu/core/rtw_br_ext.c:606:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]

  606 |         __be16 protocol = *((__be16 *)(skb->data + 2 * ETH_ALEN));
      |         ^~~~~~

regards,
Michael

#4 lore raw
In reply to Joash Naidoo
Quoted 27 lines · Joash Naidoo
Quoted 15 lines · Greg KH
Quoted 13 lines · Joash Naidoo

Coding style fix. Fix too many leading tabs and line length.

---
Changes in v3:
- Fix flipped condition mistake
- move skb NULL check before dereferencing it
Changes in v2:
- Flip additional nested if conditions and don't reverse the
last if statement
- Move declarations to start of function
- Separate converting __constant_htons to htons to another
patch

Quoted 7 lines

Always test-build your changes before sending them out so you do not get grumpy emails from maintainers asking why you didn't test-build your changes...

thanks,

greg k-h

Hi Greg,

Quoted 9 lines

Indeed building and testing is a vital step before sending changes. Is this message referring to my V3 patch not building? Or referring to my earlier versions?

Before sending I built my changes with:

make M=drivers/staging/r8188eu clean
make M=drivers/staging/r8188eu

CC [M] drivers/staging/r8188eu/core/rtw_br_ext.o
drivers/staging/r8188eu/core/rtw_br_ext.c: In function ‘dhcp_flag_bcast’:
drivers/staging/r8188eu/core/rtw_br_ext.c:607:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]

  607 |         __be16 protocol = *((__be16 *)(skb->data + 2 * ETH_ALEN));
      |         ^~~~~~

cc1: all warnings being treated as errors

regards,
dan carpenter