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

[Andy Walls <awalls@md.metrocast.net>]

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