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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
