Hi,
This patch series fixes some sparse warnings in rtw_brr_ext.c
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 | 14 +++++++-------
1 file changed, 7 insertions(+), 7 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 ntohs()
Signed-off-by: Aakash Hemadri <[email protected]>
---
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..404fa8904e47 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 = ntohs(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
Unnecessary double cast, remove them.
Signed-off-by: Aakash Hemadri <[email protected]>
---
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 404fa8904e47..6a0462ce6230 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -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 = 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 u16, remove htons() as __be16 degrades.
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 d4acf02ca64f..14b2935cab98 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -674,13 +674,13 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb)
u32 cookie = dhcph->cookie;
if (cookie == DHCP_MAGIC) { /* match magic word */
- if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
+ if (!(dhcph->flags & 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 |= 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 ntohs()
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 6a0462ce6230..d4acf02ca64f 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,
+ ntohs(iph->payload_len),
IPPROTO_ICMPV6,
- csum_partial((__u8 *)hdr, iph->payload_len, 0));
+ csum_partial((__u8 *)hdr, ntohs(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 expects unsigned short not __be16, so remove htons and cast
MAGIC_CODE as unsigned short
Signed-off-by: Aakash Hemadri <[email protected]>
---
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 14b2935cab98..600a38330579 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -513,7 +513,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
/* insert the magic_code+client mac in relay tag */
pMagic = (unsigned short *)tag->tag_data;
- *pMagic = htons(MAGIC_CODE);
+ *pMagic = (unsigned short)MAGIC_CODE;
memcpy(tag->tag_data+MAGIC_CODE_LEN, skb->data+ETH_ALEN, ETH_ALEN);
/* Add relay tag */
--
2.32.0
On Thursday, August 19, 2021 10:17:54 AM CEST Aakash Hemadri wrote:
> Fix sparse warning:
> > rtw_br_ext.c:836:54: warning: cast to restricted __be32
>
> Unnecessary double cast, remove them.
Are you sure that you had a *double* cast?
In the line that you changed I see only a cast and a swap
(or, better, a potential swap).
Regards,
Fabio
>
> Signed-off-by: Aakash Hemadri <[email protected]>
> ---
> 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 404fa8904e47..6a0462ce6230 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -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 = dhcph->cookie;
>
> if (cookie == DHCP_MAGIC) { /* match magic word */
> if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
> --
> 2.32.0
>
>
>
On Thu, Aug 19, 2021 at 01:47:54PM +0530, Aakash Hemadri wrote:
> Fix sparse warning:
> > rtw_br_ext.c:836:54: warning: cast to restricted __be32
>
> Unnecessary double cast, remove them.
>
> Signed-off-by: Aakash Hemadri <[email protected]>
> ---
> 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 404fa8904e47..6a0462ce6230 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -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 = dhcph->cookie;
Wait, what? The cookie was in big endian format, and now you just
ignore it? Why is this ok? This breaks the code, have you tested this?
thanks,
greg k-h
On Thu, Aug 19, 2021 at 01:47:56PM +0530, Aakash Hemadri wrote:
> 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 u16, remove htons() as __be16 degrades.
Um, are you sure?
>
> 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 d4acf02ca64f..14b2935cab98 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -674,13 +674,13 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb)
> u32 cookie = dhcph->cookie;
>
> if (cookie == DHCP_MAGIC) { /* match magic word */
> - if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
> + if (!(dhcph->flags & BROADCAST_FLAG)) {
So you now just ignore the fact that the code used to properly check
BROADCAST_FLAG being in big endian mode, and now you assume it is native
endian?
Why is this ok? Did you test this?
thanks,
greg k-h
On 21/08/19 07:19PM, Greg Kroah-Hartman wrote:
> On Thu, Aug 19, 2021 at 01:47:54PM +0530, Aakash Hemadri wrote:
> > Fix sparse warning:
> > > rtw_br_ext.c:836:54: warning: cast to restricted __be32
> >
> > Unnecessary double cast, remove them.
> >
> > Signed-off-by: Aakash Hemadri <[email protected]>
> > ---
> > 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 404fa8904e47..6a0462ce6230 100644
> > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > @@ -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 = dhcph->cookie;
>
> Wait, what? The cookie was in big endian format, and now you just
> ignore it? Why is this ok? This breaks the code, have you tested this?
Ah, I assumed clearing a sparse warning was enough to make sure my
change was correct. My understanding of endianness is incorrect.
Will redo this.
Thanks,
Aakash Hemadri
On Thursday, August 19, 2021 7:20:44 PM CEST Greg Kroah-Hartman wrote:
> On Thu, Aug 19, 2021 at 01:47:56PM +0530, Aakash Hemadri wrote:
> > 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 u16, remove htons() as __be16 degrades.
>
> Um, are you sure?
>
> >
> > 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 d4acf02ca64f..14b2935cab98 100644
> > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > @@ -674,13 +674,13 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb)
> > u32 cookie = dhcph->cookie;
> >
> > if (cookie == DHCP_MAGIC) { /* match magic word */
> > - if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
> > + if (!(dhcph->flags & BROADCAST_FLAG)) {
>
> So you now just ignore the fact that the code used to properly check
> BROADCAST_FLAG being in big endian mode, and now you assume it is native
> endian?
>
> Why is this ok? Did you test this?
>
> thanks,
>
> greg k-h
>
Aakash,
Building on the objections you had from Greg I suggest that, before attempting
anew to address problems like these, you get a better understanding of the topics of
native and network endianness and of the API that (conditionally) swap bytes
in a variable between little endian and big endian representation.
To start with, please note that the following code leads to tests for "v.vub[0] == 0xDD"
which is true on little endian architectures while "v.vub[0] == 0xAA" is true on big
endian ones...
union {
u32 vud;
u8 vub[4];
} v;
v.vud = 0xAABBCCDD;
Also note that API like cpu_to_be32(), htonl(), be32_to_cpu(), ntohl, and the likes are
used to (conditionally) swap bytes (i.e., change the arrangement of the bytes in a
multi-bytes variable).
Casts have very different purposes and usage patterns and, above all, they cannot
magically change the endianness of a variable.
Regards,
Fabio
On 8/19/21 3:17 AM, 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 ntohs()
>
> 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 6a0462ce6230..d4acf02ca64f 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,
> + ntohs(iph->payload_len),
> IPPROTO_ICMPV6,
> - csum_partial((__u8 *)hdr, iph->payload_len, 0));
> + csum_partial((__u8 *)hdr, ntohs(iph->payload_len), 0));
> }
> }
> }
>
This patch is correct, but I prefer that you use be16_to_cpu() rather than
ntohs(). I think it makes the code easier to read.
Larry
On 8/19/21 3:17 AM, Aakash Hemadri wrote:
> Fix sparse warning:
>> rtw_br_ext.c:836:54: warning: cast to restricted __be32
>
> Unnecessary double cast, remove them.
>
> Signed-off-by: Aakash Hemadri <[email protected]>
> ---
> 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 404fa8904e47..6a0462ce6230 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -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 = dhcph->cookie;
>
> if (cookie == DHCP_MAGIC) { /* match magic word */
> if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
>
This patch is wrong. All the documentation I could find tells me that the
multi-byte entries in dhcph are big-endian, thus the new line should read:
u32 cookie = be32_to_cpu(dhcph->cookie);
combined with:
@@ -649,7 +650,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 */
};
The old code was, in fact, correct, but not in a way that satisfied Sparse.
Larry
On 8/19/21 3:17 AM, 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 ntohs()
>
> 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 6a0462ce6230..d4acf02ca64f 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,
> + ntohs(iph->payload_len),
> IPPROTO_ICMPV6,
> - csum_partial((__u8 *)hdr, iph->payload_len, 0));
> + csum_partial((__u8 *)hdr, ntohs(iph->payload_len), 0));
> }
> }
> }
>
This patch is correct. Again, I like be16_to_cpu() better than ntohs(), but that
is not a deal breaker. The kernel is split on the usage.
Larry
On 21/08/20 05:10PM, Fabio M. De Francesco wrote:
> Building on the objections you had from Greg I suggest that, before attempting
> anew to address problems like these, you get a better understanding of the topics of
> native and network endianness and of the API that (conditionally) swap bytes
> in a variable between little endian and big endian representation.
>
> To start with, please note that the following code leads to tests for "v.vub[0] == 0xDD"
> which is true on little endian architectures while "v.vub[0] == 0xAA" is true on big
> endian ones...
>
> union {
> u32 vud;
> u8 vub[4];
> } v;
>
> v.vud = 0xAABBCCDD;
>
> Also note that API like cpu_to_be32(), htonl(), be32_to_cpu(), ntohl, and the likes are
> used to (conditionally) swap bytes (i.e., change the arrangement of the bytes in a
> multi-bytes variable).
>
> Casts have very different purposes and usage patterns and, above all, they cannot
> magically change the endianness of a variable.
>
> Regards,
>
> Fabio
>
Thanks for the explanation Fabio!
Will rework and send it through!
Thanks,
Aakash Hemadri
On 21/08/20 04:38PM, Larry Finger wrote:
> This patch is correct, but I prefer that you use be16_to_cpu() rather than
> ntohs(). I think it makes the code easier to read.
Sure Larry, Will use be16_to_cpu()
Thanks,
Aakash Hemadri
On 21/08/20 04:44PM, Larry Finger wrote:
> This patch is wrong. All the documentation I could find tells me that the
> multi-byte entries in dhcph are big-endian, thus the new line should read:
>
> u32 cookie = be32_to_cpu(dhcph->cookie);
> combined with:
>
> @@ -649,7 +650,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 */
> };
>
> The old code was, in fact, correct, but not in a way that satisfied Sparse.
>
> Larry
Thanks for the review Larry. I understand now, will rework and send it
through
Thanks,
Aakash Hemadri
From: Larry Finger
> Sent: 20 August 2021 22:45
>
> On 8/19/21 3:17 AM, Aakash Hemadri wrote:
> > Fix sparse warning:
> >> rtw_br_ext.c:836:54: warning: cast to restricted __be32
> >
> > Unnecessary double cast, remove them.
> >
> > Signed-off-by: Aakash Hemadri <[email protected]>
> > ---
> > 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 404fa8904e47..6a0462ce6230 100644
> > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > @@ -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 = dhcph->cookie;
> >
> > if (cookie == DHCP_MAGIC) { /* match magic word */
> > if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
> >
>
> This patch is wrong. All the documentation I could find tells me that the
> multi-byte entries in dhcph are big-endian, thus the new line should read:
>
> u32 cookie = be32_to_cpu(dhcph->cookie);
Modulo anything that really means it should get_unaligned_be32().
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On 21/08/22 09:30PM, David Laight wrote:
> Modulo anything that really means it should get_unaligned_be32().
>
> David
Thanks for your reviews David!
Will make this change!
Thanks,
Aakash Hemadri
On 8/22/21 4:30 PM, David Laight wrote:
> From: Larry Finger
>> Sent: 20 August 2021 22:45
>> u32 cookie = be32_to_cpu(dhcph->cookie);
>
> Modulo anything that really means it should get_unaligned_be32().
True, but cookie is 32-bit aligned, thus the code is correct.
Larry