[PATCH v4 0/5] staging: r8188eu: fix sparse warnings

[PATCH v4 0/5] staging: r8188eu: fix sparse warnings

[Aakash Hemadri]

Hi,
	This patch series fixes some sparse warnings in rtw_br_ext.c

Changes in v3 -> v4
- Added this changelog, as requested by Greg's patch bot

Changes in v2 -> v3
- Fixed incorrect usage/removal of endian swaps and checks

Changes in v1 -> v2
- Split patch

Aakash Hemadri (5):
  staging: r8188eu: restricted __be16 degrades to int
  staging: r8188eu: cast to restricted __be32
  staging: r8188eu: incorrect type in csum_ipv6_magic
  staging: r8188eu: restricted __be16 degrades to int
  staging: r8188eu: incorrect type in assignment

 drivers/staging/r8188eu/core/rtw_br_ext.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)


base-commit: 093991aaadf0fbb34184fa37a46e7a157da3f386
-- 
2.32.0

[PATCH v4 1/5] staging: r8188eu: restricted __be16 degrades to int

[Aakash Hemadri]

Fix sparse warning:
> rtw_br_ext.c:73:23: warning: restricted __be16 degrades to integer

Here tag->tag_len is be16, use be16_to_cpu()

Signed-off-by: Aakash Hemadri <aakashhemadri123@gmail.com>
---

Prefer using be16_to_cpu() over ntohs()

 drivers/staging/r8188eu/core/rtw_br_ext.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index ee52f28a1e56..f6d1f6029ec3 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -70,7 +70,7 @@ static int __nat25_add_pppoe_tag(struct sk_buff *skb, struct pppoe_tag *tag)
 	struct pppoe_hdr *ph = (struct pppoe_hdr *)(skb->data + ETH_HLEN);
 	int data_len;
 
-	data_len = tag->tag_len + TAG_HDR_LEN;
+	data_len = be16_to_cpu(tag->tag_len) + TAG_HDR_LEN;
 	if (skb_tailroom(skb) < data_len) {
 		_DEBUG_ERR("skb_tailroom() failed in add SID tag!\n");
 		return -1;
-- 
2.32.0

[PATCH v4 2/5] staging: r8188eu: cast to restricted __be32

[Aakash Hemadri]

Fix sparse warning:
> rtw_br_ext.c:836:54: warning: cast to restricted __be32

dhpch->cookie is be32, change it's type.

Suggested-by: Larry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: Aakash Hemadri <aakashhemadri123@gmail.com>
---

I had incorrectly removed bytsewapping without considering the hardware
runs in big endian rather change type to __be32 as sugessted by Larry.

 drivers/staging/r8188eu/core/rtw_br_ext.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index f6d1f6029ec3..f65d94bfa286 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -649,7 +649,7 @@ struct dhcpMessage {
 	u_int8_t chaddr[16];
 	u_int8_t sname[64];
 	u_int8_t file[128];
-	u_int32_t cookie;
+	__be32 cookie;
 	u_int8_t options[308]; /* 312 - cookie */
 };
 
@@ -671,7 +671,7 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb)
 				    (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);
+					u32 cookie = be32_to_cpu(dhcph->cookie);
 
 					if (cookie == DHCP_MAGIC) { /*  match magic word */
 						if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
-- 
2.32.0

[PATCH v4 3/5] staging: r8188eu: incorrect type in csum_ipv6_magic

[Aakash Hemadri]

Fix sparse warning:
> rtw_br_ext.c:771:84:    got restricted __be16 [usertype] payload_len
> rtw_br_ext.c:773:110: warning: incorrect type in argument 2
    (different base types)
> rtw_br_ext.c:773:110:    expected int len
> rtw_br_ext.c:773:110:    got restricted __be16 [usertype] payload_len

csum_ipv6_magic and csum_partial expect int len not __be16, use
be16_to_cpu()

Signed-off-by: Aakash Hemadri <aakashhemadri123@gmail.com>
---

No changes

 drivers/staging/r8188eu/core/rtw_br_ext.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index f65d94bfa286..26606093a3c3 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -615,9 +615,9 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
 						struct icmp6hdr  *hdr = (struct icmp6hdr *)(skb->data + ETH_HLEN + sizeof(*iph));
 						hdr->icmp6_cksum = 0;
 						hdr->icmp6_cksum = csum_ipv6_magic(&iph->saddr, &iph->daddr,
-										iph->payload_len,
+										be16_to_cpu(iph->payload_len),
 										IPPROTO_ICMPV6,
-										csum_partial((__u8 *)hdr, iph->payload_len, 0));
+										csum_partial((__u8 *)hdr, be16_to_cpu(iph->payload_len), 0));
 					}
 				}
 			}
-- 
2.32.0

RE: [PATCH v4 3/5] staging: r8188eu: incorrect type in csum_ipv6_magic

[David Laight]

From: Aakash Hemadri
> Sent: 23 August 2021 18:00
> 
> Fix sparse warning:
> > rtw_br_ext.c:771:84:    got restricted __be16 [usertype] payload_len
> > rtw_br_ext.c:773:110: warning: incorrect type in argument 2
>     (different base types)
> > rtw_br_ext.c:773:110:    expected int len
> > rtw_br_ext.c:773:110:    got restricted __be16 [usertype] payload_len
> 
> csum_ipv6_magic and csum_partial expect int len not __be16, use
> be16_to_cpu()

This is a behaviour change on LE systems.
Even if you are testing on BE the commit message should say
that it fixes the length passed to the checksum code and
was detected by sparse.

...
>  			hdr->icmp6_cksum = csum_ipv6_magic(&iph->saddr, &iph->daddr,
> -						iph->payload_len,
> +						be16_to_cpu(iph->payload_len),
>  						IPPROTO_ICMPV6,
> -						csum_partial((__u8 *)hdr, iph->payload_len, 0));
> +						csum_partial((__u8 *)hdr, be16_to_cpu(iph->payload_len), 0));

Oh - that code is indented far too many times.
I had to delete a load of tabs to stop my mail crapping it.

	David

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

Re: [PATCH v4 3/5] staging: r8188eu: incorrect type in csum_ipv6_magic

[Aakash Hemadri]

On 21/08/25 09:23AM, David Laight wrote:
> From: Aakash Hemadri
> > Sent: 23 August 2021 18:00
> > 
> > Fix sparse warning:
> > > rtw_br_ext.c:771:84:    got restricted __be16 [usertype] payload_len
> > > rtw_br_ext.c:773:110: warning: incorrect type in argument 2
> >     (different base types)
> > > rtw_br_ext.c:773:110:    expected int len
> > > rtw_br_ext.c:773:110:    got restricted __be16 [usertype] payload_len
> > 
> > csum_ipv6_magic and csum_partial expect int len not __be16, use
> > be16_to_cpu()
> 
> This is a behaviour change on LE systems.
> Even if you are testing on BE the commit message should say
> that it fixes the length passed to the checksum code and
> was detected by sparse.
> 

Thanks for the note david.

-Aakash

[PATCH v4 4/5] staging: r8188eu: restricted __be16 degrades to int

[Aakash Hemadri]

Fix sparse warning:
> rtw_br_ext.c:839:70: warning: restricted __be16 degrades to integer
> rtw_br_ext.c:845:70: warning: invalid assignment: |=
> rtw_br_ext.c:845:70:    left side has type unsigned short
> rtw_br_ext.c:845:70:    right side has type restricted __be16

dhcp->flag is be16. Change htons() to cpu_to_be16() for clarity.

Signed-off-by: Aakash Hemadri <aakashhemadri123@gmail.com>
---

Incorrectly removed htons() to satisfy sparse warning. Instead changed to
appropriate type and changed htons to cpu_to_be16 for clarity.

 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 26606093a3c3..83a4594a4214 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -641,7 +641,7 @@ struct dhcpMessage {
 	u_int8_t hops;
 	u_int32_t xid;
 	u_int16_t secs;
-	u_int16_t flags;
+	__be16 flags;
 	u_int32_t ciaddr;
 	u_int32_t yiaddr;
 	u_int32_t siaddr;
@@ -674,13 +674,13 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb)
 					u32 cookie = be32_to_cpu(dhcph->cookie);
 
 					if (cookie == DHCP_MAGIC) { /*  match magic word */
-						if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
+						if (!(dhcph->flags & cpu_to_be16(BROADCAST_FLAG))) {
 							/*  if not broadcast */
 							register int sum = 0;
 
 							DEBUG_INFO("DHCP: change flag of DHCP request to broadcast.\n");
 							/*  or BROADCAST flag */
-							dhcph->flags |= htons(BROADCAST_FLAG);
+							dhcph->flags |= cpu_to_be16(BROADCAST_FLAG);
 							/*  recalculate checksum */
 							sum = ~(udph->check) & 0xffff;
 							sum += be16_to_cpu(dhcph->flags);
-- 
2.32.0

[PATCH v4 5/5] staging: r8188eu: incorrect type in assignment

[Aakash Hemadri]

Fix sparse warning:
> rtw_br_ext.c:516:57: warning: incorrect type in assignment
    (different base types)
> rtw_br_ext.c:516:57:    expected unsigned short
> rtw_br_ext.c:516:57:    got restricted __be16 [usertype]

*pMagic holds __be16 change it's type to __be16

Signed-off-by: Aakash Hemadri <aakashhemadri123@gmail.com>
---

Incorrectly removed htons() to satisfy sparse warning. Instead make
pMagic use the appropriate bid endian type

 drivers/staging/r8188eu/core/rtw_br_ext.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index 83a4594a4214..14cf13516d34 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -474,7 +474,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
 		/*                Handle PPPoE frame                 */
 		/*---------------------------------------------------*/
 		struct pppoe_hdr *ph = (struct pppoe_hdr *)(skb->data + ETH_HLEN);
-		unsigned short *pMagic;
+		__be16 *pMagic;
 
 		switch (method) {
 		case NAT25_CHECK:
@@ -512,7 +512,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
 						tag->tag_len = htons(MAGIC_CODE_LEN+RTL_RELAY_TAG_LEN+old_tag_len);
 
 						/*  insert the magic_code+client mac in relay tag */
-						pMagic = (unsigned short *)tag->tag_data;
+						pMagic = (__be16 *)tag->tag_data;
 						*pMagic = htons(MAGIC_CODE);
 						memcpy(tag->tag_data+MAGIC_CODE_LEN, skb->data+ETH_ALEN, ETH_ALEN);
 
-- 
2.32.0

Re: [PATCH v4 0/5] staging: r8188eu: fix sparse warnings

[Aakash Hemadri]

On 21/08/23 10:30PM, Aakash Hemadri wrote:
> Hi,
> 	This patch series fixes some sparse warnings in rtw_br_ext.c
> 
> Changes in v3 -> v4
> - Added this changelog, as requested by Greg's patch bot
> 
> Changes in v2 -> v3
> - Fixed incorrect usage/removal of endian swaps and checks
> 
> Changes in v1 -> v2
> - Split patch
> 
> Aakash Hemadri (5):
>   staging: r8188eu: restricted __be16 degrades to int
>   staging: r8188eu: cast to restricted __be32
>   staging: r8188eu: incorrect type in csum_ipv6_magic
>   staging: r8188eu: restricted __be16 degrades to int
>   staging: r8188eu: incorrect type in assignment
> 
>  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> 
> base-commit: 093991aaadf0fbb34184fa37a46e7a157da3f386
> -- 
> 2.32.0
>

Hello greg, if there are any issues with the patchset do let me know,
Larry & Philip acked the previous ver of this patch.

I'd rather not add noise to the ml unnecessarily.
Also this patchset doesn't apply cleanly to the current staging-testing
I can resend fixing that if that's the issue.

-Aakash

Re: [PATCH v4 0/5] staging: r8188eu: fix sparse warnings

[Greg Kroah-Hartman]

On Sat, Aug 28, 2021 at 02:20:17PM +0530, Aakash Hemadri wrote:
> On 21/08/23 10:30PM, Aakash Hemadri wrote:
> > Hi,
> > 	This patch series fixes some sparse warnings in rtw_br_ext.c
> > 
> > Changes in v3 -> v4
> > - Added this changelog, as requested by Greg's patch bot
> > 
> > Changes in v2 -> v3
> > - Fixed incorrect usage/removal of endian swaps and checks
> > 
> > Changes in v1 -> v2
> > - Split patch
> > 
> > Aakash Hemadri (5):
> >   staging: r8188eu: restricted __be16 degrades to int
> >   staging: r8188eu: cast to restricted __be32
> >   staging: r8188eu: incorrect type in csum_ipv6_magic
> >   staging: r8188eu: restricted __be16 degrades to int
> >   staging: r8188eu: incorrect type in assignment
> > 
> >  drivers/staging/r8188eu/core/rtw_br_ext.c | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> > 
> > 
> > base-commit: 093991aaadf0fbb34184fa37a46e7a157da3f386
> > -- 
> > 2.32.0
> >
> 
> Hello greg, if there are any issues with the patchset do let me know,
> Larry & Philip acked the previous ver of this patch.
> 
> I'd rather not add noise to the ml unnecessarily.
> Also this patchset doesn't apply cleanly to the current staging-testing
> I can resend fixing that if that's the issue.

Please fix up and resend, my staging review queue is empty.

thanks,

greg k-h