On Mon, Feb 01, 2010 at 11:07:21AM +0100, Jean Delvare wrote:
> On Mon, 1 Feb 2010 09:51:42 +0000, d binderman wrote:
> >
> >
> > Hello there,
> >
> > I just ran the sourceforge tool cppcheck over the source code of the
> > new Linux kernel 2.6.33-rc6
> >
> > It said
> >
> > [./i2c/busses/i2c-highlander.c:284]: (style) Warning - using char variable in bit operation
> >
> > The source code is
> >
> > static int highlander_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr,
> > unsigned short flags, char read_write,
> > u8 command, int size,
> > union i2c_smbus_data *data)
> > {
> > struct highlander_i2c_dev *dev = i2c_get_adapdata(adap);
> > int read = read_write & I2C_SMBUS_READ;
> >
> > In C, chars can be signed or unsigned, so the value written into
> > local variable read by sign extension is not certain.
> >
> > Suggest new code
> >
> > static int highlander_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr,
> > unsigned short flags, char read_write,
> > u8 command, int size,
> > union i2c_smbus_data *data)
> > {
> > struct highlander_i2c_dev *dev = i2c_get_adapdata(adap);
> > int read = ((unsigned char) read_write) & I2C_SMBUS_READ;
>
> read_write can only have value 0 or 1, so we don't care if chars are
> signed or not. That being said, the code above looks odd, the value of
> read_write can be used in the code directly without using an
> intermediate variable:
>
> iowrite16((addr << 1) | read_write, dev->base + SMSMADR);
>
> This would solve your warning issue and make the code more simple too.
Don't forget to convert the 'if (read)' below, too.
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature
