linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: r8188eu: clean up endianness issues
@ 2021-08-18 15:52 Aakash Hemadri
  2021-08-18 20:40 ` Greg Kroah-Hartman
  2021-08-20 10:26 ` David Laight
  0 siblings, 2 replies; 4+ messages in thread
From: Aakash Hemadri @ 2021-08-18 15:52 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman
  Cc: linux-staging, linux-kernel, Bjorn Helgaas, Shuah Khan

Fix these sparse warnings:

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

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

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

> rtw_br_ext.c:664:45: warning: cast to restricted __be16
> rtw_br_ext.c:771:84: warning: incorrect type in argument 3 (different base types)
> rtw_br_ext.c:771:84:    expected unsigned int [usertype] len

Cast MAGIC_CODE as unsigned short

> 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

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

Unnecessary double cast?

> 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

Signed-off-by: Aakash Hemadri <aakashhemadri123@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_br_ext.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index e8eea95a52e3..8eb7475726e1 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;
@@ -598,7 +598,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 */
@@ -661,7 +661,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
 					}
 
 					pMagic = (unsigned short *)tag->tag_data;
-					if (ntohs(*pMagic) != MAGIC_CODE) {
+					if (*pMagic != (unsigned short)MAGIC_CODE) {
 						DEBUG_ERR("Can't find MAGIC_CODE in %s packet!\n",
 							(ph->code == PADO_CODE ? "PADO" : "PADS"));
 						return -1;
@@ -768,9 +768,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));
 					}
 				}
 			}
@@ -833,16 +833,16 @@ 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))) {
+						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);

base-commit: cbfa6f33e3a685c329d78e06b0cf1dcb23c9d849
-- 
2.32.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] staging: r8188eu: clean up endianness issues
  2021-08-18 15:52 [PATCH] staging: r8188eu: clean up endianness issues Aakash Hemadri
@ 2021-08-18 20:40 ` Greg Kroah-Hartman
  2021-08-20 10:26 ` David Laight
  1 sibling, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2021-08-18 20:40 UTC (permalink / raw)
  To: Aakash Hemadri
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel,
	Bjorn Helgaas, Shuah Khan

On Wed, Aug 18, 2021 at 09:22:36PM +0530, Aakash Hemadri wrote:
> Fix these sparse warnings:
> 
> > rtw_br_ext.c:73:23: warning: restricted __be16 degrades to integer
> 
> Here tag->tag_len is be16, use ntohs()
> 
> > rtw_br_ext.c:601:57: warning: incorrect type in assignment (different base types)
> > rtw_br_ext.c:601:57:    expected unsigned short
> > rtw_br_ext.c:601:57:    got restricted __be16 [usertype]
> 
> > rtw_br_ext.c:664:45: warning: cast to restricted __be16
> > rtw_br_ext.c:771:84: warning: incorrect type in argument 3 (different base types)
> > rtw_br_ext.c:771:84:    expected unsigned int [usertype] len
> 
> Cast MAGIC_CODE as unsigned short
> 
> > 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
> 
> > rtw_br_ext.c:836:54: warning: cast to restricted __be32
> 
> Unnecessary double cast?
> 
> > 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

That is a lot of different things all at once.  Please break this up
into one-logical-change at a time and send a patch series.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH] staging: r8188eu: clean up endianness issues
  2021-08-18 15:52 [PATCH] staging: r8188eu: clean up endianness issues Aakash Hemadri
  2021-08-18 20:40 ` Greg Kroah-Hartman
@ 2021-08-20 10:26 ` David Laight
  2021-08-20 17:13   ` Larry Finger
  1 sibling, 1 reply; 4+ messages in thread
From: David Laight @ 2021-08-20 10:26 UTC (permalink / raw)
  To: 'Aakash Hemadri',
	Larry Finger, Phillip Potter, Greg Kroah-Hartman
  Cc: linux-staging, linux-kernel, Bjorn Helgaas, Shuah Khan

From: Aakash Hemadri
> Sent: 18 August 2021 16:53
> 
> Fix these sparse warnings:

Did you test this code before and after the changes?

I think you've changed the behaviour on LE systems which
are probably the ones it was actually tested on.

Don't blindly change code to fix sparse warnings.

	David

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] staging: r8188eu: clean up endianness issues
  2021-08-20 10:26 ` David Laight
@ 2021-08-20 17:13   ` Larry Finger
  0 siblings, 0 replies; 4+ messages in thread
From: Larry Finger @ 2021-08-20 17:13 UTC (permalink / raw)
  To: David Laight, 'Aakash Hemadri',
	Phillip Potter, Greg Kroah-Hartman
  Cc: linux-staging, linux-kernel, Bjorn Helgaas, Shuah Khan

On 8/20/21 5:26 AM, David Laight wrote:
> From: Aakash Hemadri
>> Sent: 18 August 2021 16:53
>>
>> Fix these sparse warnings:
> 
> Did you test this code before and after the changes?
> 
> I think you've changed the behaviour on LE systems which
> are probably the ones it was actually tested on.
> 
> Don't blindly change code to fix sparse warnings.

I'm late getting into this conversation. Gmail thought the original patches were 
spam.

First of all, we need to change the behavior as the original code is wrong; 
however, the code involves PPPoE, which I have never used, and never tested. I 
still need to check the correct endian values for one of the packet types. I 
hate reading IEEE official documents!!!!

I should have a review of the patches later today.

Larry


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-20 17:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18 15:52 [PATCH] staging: r8188eu: clean up endianness issues Aakash Hemadri
2021-08-18 20:40 ` Greg Kroah-Hartman
2021-08-20 10:26 ` David Laight
2021-08-20 17:13   ` Larry Finger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).