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@xxxxxxxxx>
> > ---
> > 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
> >
