smatch report: cx231xx: incorrect check in cx231xx_write_i2c_data()

2010-12-23 Thread Dan Carpenter 
Hi,

I was doing an audit and I came across this.

drivers/media/video/cx231xx/cx231xx-core.c +1644 cx231xx_write_i2c_data(14)
warn: 'saddr_len' is non-zero. Did you mean 'saddr'

  1642  if (saddr_len == 0)
  1643  saddr = 0;
  1644  else if (saddr_len == 0)
  1645  saddr = 0xff;

We check saddr_len == 0 twice which doesn't make sense.  I'm not sure
what the correct fix is though.  It's been this way since the driver was
merged.

regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: smatch report: cx231xx: incorrect check in cx231xx_write_i2c_data()

2010-12-23 Thread Andy Walls 
On Thu, 2010-12-23 at 19:43 +0300, Dan Carpenter wrote:
 Hi,
 
 I was doing an audit and I came across this.
 
 drivers/media/video/cx231xx/cx231xx-core.c +1644 cx231xx_write_i2c_data(14)
   warn: 'saddr_len' is non-zero. Did you mean 'saddr'
 
   1642  if (saddr_len == 0)
   1643  saddr = 0;
   1644  else if (saddr_len == 0)
   1645  saddr = 0xff;
 
 We check saddr_len == 0 twice which doesn't make sense.  I'm not sure
 what the correct fix is though.

Given that saddr probably means sub-address, that saddr_len
probably means sub-address length, that saddr is only a 16 bit value,
and this switch in cx231xx_send_usb_command():

/* set index value */
switch (saddr_len) {
case 0:
ven_req.wIndex = 0; /* need check */
break;
case 1:
ven_req.wIndex = (req_data-saddr_dat  0xff);
break;
case 2:
ven_req.wIndex = req_data-saddr_dat;
break;
}

and noting that req_data-saddr_dat holds what was saddr;
the if statement, and the many others like it, should probably be:

if (saddr_len == 0)
saddr = 0;
else if (saddr_len == 1)   - == 1
saddr = 0xff;


Regards,
Andy

   It's been this way since the driver was
 merged.
 
 regards,
 dan carpenter



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

Re: smatch report: cx231xx: incorrect check in cx231xx_write_i2c_data()

2010-12-23 Thread Dan Carpenter 
On Thu, Dec 23, 2010 at 01:34:52PM -0500, Andy Walls wrote:
 and noting that req_data-saddr_dat holds what was saddr;
 the if statement, and the many others like it, [...]

The others I see are:
cx231xx_write_i2c_data()
cx231xx_read_i2c_data()
cx231xx_write_i2c_master()
cx231xx_read_i2c_master()

regards,
dan carpenter


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