Re: i2c/busses/i2c-highlander.c: using char variable in bit operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux