diff mbox

[NET] ethtool: silence warning on bit loss

Message ID 1476400435-18400-1-git-send-email-jesse.brandeburg@intel.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Jesse Brandeburg Oct. 13, 2016, 11:13 p.m. UTC
Sparse was complaining when we went to prototype some code
using ethtool_cmd_speed_set and SPEED_100000, which uses
the upper 16 bits of __u32 speed for the first time.

CHECK
...
.../uapi/linux/ethtool.h:123:28: warning:
  cast truncates bits from constant value (186a0 becomes 86a0)

The warning is actually bogus, as no bits are really lost, but
we can get rid of the sparse warning with this one small change.

Reported-by: Preethi Banala <preethi.banala@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 include/uapi/linux/ethtool.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

David Laight Oct. 14, 2016, 9:26 a.m. UTC | #1
From: Jesse Brandeburg
> Sent: 14 October 2016 00:14
> Sparse was complaining when we went to prototype some code
> using ethtool_cmd_speed_set and SPEED_100000, which uses
> the upper 16 bits of __u32 speed for the first time.
...
> Reported-by: Preethi Banala <preethi.banala@intel.com>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
>  include/uapi/linux/ethtool.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 099a420..8e54723 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -119,8 +119,7 @@ struct ethtool_cmd {
>  static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
>  					 __u32 speed)
>  {
> -
> -	ep->speed = (__u16)speed;
> +	ep->speed = (__u16)(speed & 0xFFFF);
>  	ep->speed_hi = (__u16)(speed >> 16);

I suspect that deleting both (__u16) casts also fixes it?

	David
David Miller Oct. 14, 2016, 8:06 p.m. UTC | #2
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
Date: Thu, 13 Oct 2016 16:13:55 -0700

> Sparse was complaining when we went to prototype some code
> using ethtool_cmd_speed_set and SPEED_100000, which uses
> the upper 16 bits of __u32 speed for the first time.
> 
> CHECK
> ...
> .../uapi/linux/ethtool.h:123:28: warning:
>   cast truncates bits from constant value (186a0 becomes 86a0)
> 
> The warning is actually bogus, as no bits are really lost, but
> we can get rid of the sparse warning with this one small change.
> 
> Reported-by: Preethi Banala <preethi.banala@intel.com>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Ok, I'll apply this.

There were alternative suggestions but I like this patch
because it makes it explicit what is going on.

Just removing the u16 cast requires the reader to implicitly
understand and know the types in the structure.
diff mbox

Patch

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 099a420..8e54723 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -119,8 +119,7 @@  struct ethtool_cmd {
 static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
 					 __u32 speed)
 {
-
-	ep->speed = (__u16)speed;
+	ep->speed = (__u16)(speed & 0xFFFF);
 	ep->speed_hi = (__u16)(speed >> 16);
 }