diff mbox series

i2c: busses: Convert parameter to __le32

Message ID 20190921125804.GA10910@gmail.com
State Rejected
Headers show
Series i2c: busses: Convert parameter to __le32 | expand

Commit Message

Adam Zerella Sept. 21, 2019, 12:58 p.m. UTC
The assignment of `serial` is using le32_to_cpu() without
first converting the parameter `dev->ibuffer` to __le32.

This produces a Sparse warning of:

`warning: cast to restricted __le32`

Signed-off-by: Adam Zerella <adam.zerella@gmail.com>
---
 drivers/i2c/busses/i2c-diolan-u2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Guenter Roeck Sept. 22, 2019, 1:25 p.m. UTC | #1
On Sat, Sep 21, 2019 at 10:58:04PM +1000, Adam Zerella wrote:
> The assignment of `serial` is using le32_to_cpu() without
> first converting the parameter `dev->ibuffer` to __le32.
> 
> This produces a Sparse warning of:
> 
> `warning: cast to restricted __le32`
> 
> Signed-off-by: Adam Zerella <adam.zerella@gmail.com>
> ---
>  drivers/i2c/busses/i2c-diolan-u2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-diolan-u2c.c b/drivers/i2c/busses/i2c-diolan-u2c.c
> index 382f105e0fe3..32de47eda950 100644
> --- a/drivers/i2c/busses/i2c-diolan-u2c.c
> +++ b/drivers/i2c/busses/i2c-diolan-u2c.c
> @@ -289,7 +289,7 @@ static void diolan_get_serial(struct i2c_diolan_u2c *dev)
>  
>  	ret = diolan_usb_cmd(dev, CMD_GET_SERIAL, true);
>  	if (ret >= 4) {
> -		serial = le32_to_cpu(*(u32 *)dev->ibuffer);
> +		serial = le32_to_cpu(cpu_to_le32(*(u32 *)dev->ibuffer));

This doesn't make sense. Converting the data from cpu to le32 and
then back to le32 would be a no-op, not a conversion, and the code
would then be wrong on big-endian systems.

Not that it matters much here - it would just display a wrong serial
number. However, if you are making that kind of changes elsewhere to
the kernel, you are trying to mess it up really badly.

Guenter

>  		dev_info(&dev->interface->dev,
>  			 "Diolan U2C serial number %u\n", serial);
>  	}
> -- 
> 2.21.0
>
Adam Zerella Sept. 25, 2019, 1:04 p.m. UTC | #2
On Sun, Sep 22, 2019 at 06:25:02AM -0700, Guenter Roeck wrote:
> On Sat, Sep 21, 2019 at 10:58:04PM +1000, Adam Zerella wrote:
> > The assignment of `serial` is using le32_to_cpu() without
> > first converting the parameter `dev->ibuffer` to __le32.
> > 
> > This produces a Sparse warning of:
> > 
> > `warning: cast to restricted __le32`
> > 
> > Signed-off-by: Adam Zerella <adam.zerella@gmail.com>
> > ---
> >  drivers/i2c/busses/i2c-diolan-u2c.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-diolan-u2c.c b/drivers/i2c/busses/i2c-diolan-u2c.c
> > index 382f105e0fe3..32de47eda950 100644
> > --- a/drivers/i2c/busses/i2c-diolan-u2c.c
> > +++ b/drivers/i2c/busses/i2c-diolan-u2c.c
> > @@ -289,7 +289,7 @@ static void diolan_get_serial(struct i2c_diolan_u2c *dev)
> >  
> >  	ret = diolan_usb_cmd(dev, CMD_GET_SERIAL, true);
> >  	if (ret >= 4) {
> > -		serial = le32_to_cpu(*(u32 *)dev->ibuffer);
> > +		serial = le32_to_cpu(cpu_to_le32(*(u32 *)dev->ibuffer));
> 
> This doesn't make sense. Converting the data from cpu to le32 and
> then back to le32 would be a no-op, not a conversion, and the code
> would then be wrong on big-endian systems.

What I was trying to achieve was to resolve the semantic warning that
Sparse was generating. I figured if serial le32_to_cpu() was expecting
a __le32 variable type then it would be ok to convert it, looking at
the code a second time I see my mistake, BE systems would be hella 
confused.

> Not that it matters much here - it would just display a wrong serial
> number. However, if you are making that kind of changes elsewhere to
> the kernel, you are trying to mess it up really badly.

I'll admit I'm trying to get into kernel development and have heard Sparse
warnings can be something to fix, though I might ask around on
kernelnewbies to clarify my understanding on what I'm doing :)

> Guenter
> 
> >  		dev_info(&dev->interface->dev,
> >  			 "Diolan U2C serial number %u\n", serial);
> >  	}
> > -- 
> > 2.21.0
> >
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-diolan-u2c.c b/drivers/i2c/busses/i2c-diolan-u2c.c
index 382f105e0fe3..32de47eda950 100644
--- a/drivers/i2c/busses/i2c-diolan-u2c.c
+++ b/drivers/i2c/busses/i2c-diolan-u2c.c
@@ -289,7 +289,7 @@  static void diolan_get_serial(struct i2c_diolan_u2c *dev)
 
 	ret = diolan_usb_cmd(dev, CMD_GET_SERIAL, true);
 	if (ret >= 4) {
-		serial = le32_to_cpu(*(u32 *)dev->ibuffer);
+		serial = le32_to_cpu(cpu_to_le32(*(u32 *)dev->ibuffer));
 		dev_info(&dev->interface->dev,
 			 "Diolan U2C serial number %u\n", serial);
 	}