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
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 <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Aakash Hemadri <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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)
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
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
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