diff mbox series

[net-next,6/7] drivers: net: smc911x: Fix cast from pointer to integer of different size

Message ID 20201031004958.1059797-7-andrew@lunn.ch
State Changes Requested
Delegated to: David Miller
Headers show
Series smsc W=1 warning fixes | expand

Checks

Context Check Description
jkicinski/stable success Stable not CCed
jkicinski/header_inline success Link
jkicinski/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
jkicinski/checkpatch success total: 0 errors, 0 warnings, 0 checks, 12 lines checked
jkicinski/verify_fixes success Link
jkicinski/kdoc success Errors and warnings before: 0 this patch: 0
jkicinski/build_32bit success Errors and warnings before: 0 this patch: 0
jkicinski/module_param success Was 0 now: 0
jkicinski/verify_signedoff success Link
jkicinski/source_inline success Was 0 now: 0
jkicinski/subject_prefix success Link
jkicinski/tree_selection success Clearly marked for net-next
jkicinski/patch_count success Link
jkicinski/fixes_present success Link
jkicinski/cover_letter success Link

Commit Message

Andrew Lunn Oct. 31, 2020, 12:49 a.m. UTC
drivers/net/ethernet/smsc/smc911x.c: In function ‘smc911x_hardware_send_pkt’:
drivers/net/ethernet/smsc/smc911x.c:471:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  471 |  cmdA = (((u32)skb->data & 0x3) << 16) |

When built on 64bit targets, the skb->data pointer cannot be cast to a
u32 in a meaningful way. Use long instead.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/smsc/smc911x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Jakub Kicinski Nov. 2, 2020, 11:47 p.m. UTC | #1
On Sat, 31 Oct 2020 01:49:57 +0100 Andrew Lunn wrote:
> drivers/net/ethernet/smsc/smc911x.c: In function ‘smc911x_hardware_send_pkt’:
> drivers/net/ethernet/smsc/smc911x.c:471:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>   471 |  cmdA = (((u32)skb->data & 0x3) << 16) |
> 
> When built on 64bit targets, the skb->data pointer cannot be cast to a
> u32 in a meaningful way. Use long instead.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/ethernet/smsc/smc911x.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/smsc/smc911x.c b/drivers/net/ethernet/smsc/smc911x.c
> index 4ec292563f38..f37832540364 100644
> --- a/drivers/net/ethernet/smsc/smc911x.c
> +++ b/drivers/net/ethernet/smsc/smc911x.c
> @@ -466,9 +466,9 @@ static void smc911x_hardware_send_pkt(struct net_device *dev)
>  			TX_CMD_A_INT_FIRST_SEG_ | TX_CMD_A_INT_LAST_SEG_ |
>  			skb->len;
>  #else
> -	buf = (char*)((u32)skb->data & ~0x3);
> -	len = (skb->len + 3 + ((u32)skb->data & 3)) & ~0x3;
> -	cmdA = (((u32)skb->data & 0x3) << 16) |
> +	buf = (char *)((long)skb->data & ~0x3);
> +	len = (skb->len + 3 + ((long)skb->data & 3)) & ~0x3;
> +	cmdA = (((long)skb->data & 0x3) << 16) |

Probably best if you swap the (long) for something unsigned here as
well.
David Laight Nov. 3, 2020, 10:16 a.m. UTC | #2
From: Jakub Kicinski
> Sent: 02 November 2020 23:48
> 
> On Sat, 31 Oct 2020 01:49:57 +0100 Andrew Lunn wrote:
> > drivers/net/ethernet/smsc/smc911x.c: In function ‘smc911x_hardware_send_pkt’:
> > drivers/net/ethernet/smsc/smc911x.c:471:11: warning: cast from pointer to integer of different size
> [-Wpointer-to-int-cast]
> >   471 |  cmdA = (((u32)skb->data & 0x3) << 16) |
> >
> > When built on 64bit targets, the skb->data pointer cannot be cast to a
> > u32 in a meaningful way. Use long instead.
> >
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  drivers/net/ethernet/smsc/smc911x.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/smsc/smc911x.c b/drivers/net/ethernet/smsc/smc911x.c
> > index 4ec292563f38..f37832540364 100644
> > --- a/drivers/net/ethernet/smsc/smc911x.c
> > +++ b/drivers/net/ethernet/smsc/smc911x.c
> > @@ -466,9 +466,9 @@ static void smc911x_hardware_send_pkt(struct net_device *dev)
> >  			TX_CMD_A_INT_FIRST_SEG_ | TX_CMD_A_INT_LAST_SEG_ |
> >  			skb->len;
> >  #else
> > -	buf = (char*)((u32)skb->data & ~0x3);
> > -	len = (skb->len + 3 + ((u32)skb->data & 3)) & ~0x3;
> > -	cmdA = (((u32)skb->data & 0x3) << 16) |
> > +	buf = (char *)((long)skb->data & ~0x3);
> > +	len = (skb->len + 3 + ((long)skb->data & 3)) & ~0x3;
> > +	cmdA = (((long)skb->data & 0x3) << 16) |
> 
> Probably best if you swap the (long) for something unsigned here as
> well.

It would be much clearer with a temporary variable:
	offset = (unsigned long)skb->data & 3;
	buf = skb->data - offset;
	len = skb->len + offset;
	cmdA = offset << 16 | ...

   David

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

Patch

diff --git a/drivers/net/ethernet/smsc/smc911x.c b/drivers/net/ethernet/smsc/smc911x.c
index 4ec292563f38..f37832540364 100644
--- a/drivers/net/ethernet/smsc/smc911x.c
+++ b/drivers/net/ethernet/smsc/smc911x.c
@@ -466,9 +466,9 @@  static void smc911x_hardware_send_pkt(struct net_device *dev)
 			TX_CMD_A_INT_FIRST_SEG_ | TX_CMD_A_INT_LAST_SEG_ |
 			skb->len;
 #else
-	buf = (char*)((u32)skb->data & ~0x3);
-	len = (skb->len + 3 + ((u32)skb->data & 3)) & ~0x3;
-	cmdA = (((u32)skb->data & 0x3) << 16) |
+	buf = (char *)((long)skb->data & ~0x3);
+	len = (skb->len + 3 + ((long)skb->data & 3)) & ~0x3;
+	cmdA = (((long)skb->data & 0x3) << 16) |
 			TX_CMD_A_INT_FIRST_SEG_ | TX_CMD_A_INT_LAST_SEG_ |
 			skb->len;
 #endif