2021-08-28 11:12:14

by Aakash Hemadri

[permalink] [raw]
Subject: [PATCH v5 2/3] staging: r8188eu: incorrect type in csum_ipv6_magic

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]>
---
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 aa56cd1a8490..38f1bd591da9 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


2021-08-28 11:38:54

by Kari Argillander

[permalink] [raw]
Subject: Re: [PATCH v5 2/3] staging: r8188eu: incorrect type in csum_ipv6_magic

[PATCH v5 2/3] staging: r8188eu: incorrect type in csum_ipv6_magic

Subject should start with imperative mood. Every patch in this series
needs this fix.

I actually check linux coding style and did not found mention about
this, but if you look kernel log then you see that it is at least
unwritten rule. You can check this if you want
https://chris.beams.io/posts/git-commit/#imperative

On Sat, Aug 28, 2021 at 04:40:45PM +0530, Aakash Hemadri wrote:
> 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]>
> ---
> 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 aa56cd1a8490..38f1bd591da9 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
>

2021-08-30 10:41:15

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v5 2/3] staging: r8188eu: incorrect type in csum_ipv6_magic

On Sat, Aug 28, 2021 at 02:37:26PM +0300, Kari Argillander wrote:
> [PATCH v5 2/3] staging: r8188eu: incorrect type in csum_ipv6_magic
>
> Subject should start with imperative mood. Every patch in this series
> needs this fix.
>
> I actually check linux coding style and did not found mention about
> this, but if you look kernel log then you see that it is at least
> unwritten rule. You can check this if you want
> https://chris.beams.io/posts/git-commit/#imperative

The imperitive rule is stupid. We're dealing with thousands of people
and lots of them don't speak English as a first language and it's hard
to write commit messages. Let's not make it harder than it already is.

If you look through the commit messages on this list people are like.
"Blah blah blah. Fix it!" I like that approach because it shows how
humanity can triumph over pointless beaucrats. But in terms of
understanding the commit adding "Fix it!" doesn't add any value.

The only thing which matters if if the commit message has the correct
information in an understandable way.

regards,
dan carpenter

2021-09-02 09:28:32

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v5 2/3] staging: r8188eu: incorrect type in csum_ipv6_magic

On Sat, Aug 28, 2021 at 04:40:45PM +0530, Aakash Hemadri wrote:
> 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]>
> ---
> 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 aa56cd1a8490..38f1bd591da9 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
>

Same question here, does this mean the driver is broken on little-endian
systems today? This should be tested...

thanks,

greg k-h