* smatch report: cx231xx: incorrect check in cx231xx_write_i2c_data()
@ 2010-12-23 16:43 Dan Carpenter
2010-12-23 18:34 ` Andy Walls
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2010-12-23 16:43 UTC (permalink / raw)
To: Srinivasa.Deevi; +Cc: linux-media
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: smatch report: cx231xx: incorrect check in cx231xx_write_i2c_data()
2010-12-23 16:43 smatch report: cx231xx: incorrect check in cx231xx_write_i2c_data() Dan Carpenter
@ 2010-12-23 18:34 ` Andy Walls
2010-12-23 18:55 ` Dan Carpenter
[not found] ` <34B38BE41EDBA046A4AFBB591FA311320249B057C6@NBMBX01.bbnet.ad>
0 siblings, 2 replies; 5+ messages in thread
From: Andy Walls @ 2010-12-23 18:34 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Srinivasa.Deevi, linux-media
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: smatch report: cx231xx: incorrect check in cx231xx_write_i2c_data()
2010-12-23 18:34 ` Andy Walls
@ 2010-12-23 18:55 ` Dan Carpenter
[not found] ` <34B38BE41EDBA046A4AFBB591FA311320249B057C6@NBMBX01.bbnet.ad>
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2010-12-23 18:55 UTC (permalink / raw)
To: Andy Walls; +Cc: Srinivasa.Deevi, linux-media
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [patch] [media] cx231xxx: fix typo in saddr_len check
[not found] ` <34B38BE41EDBA046A4AFBB591FA311320249B057C6@NBMBX01.bbnet.ad>
@ 2010-12-23 19:38 ` Dan Carpenter
2010-12-23 19:40 ` Sri Deevi
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2010-12-23 19:38 UTC (permalink / raw)
To: Sri Deevi; +Cc: 'Andy Walls', linux-media@vger.kernel.org, mchehab
The original code compared "saddr_len" with zero twice in a nonsensical
way. I asked the list, and Andy Walls and Sri Deevi say that the second
check should be if "saddr_len == 1".
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/media/video/cx231xx/cx231xx-core.c b/drivers/media/video/cx231xx/cx231xx-core.c
index 44d124c..7d62d58 100644
--- a/drivers/media/video/cx231xx/cx231xx-core.c
+++ b/drivers/media/video/cx231xx/cx231xx-core.c
@@ -1515,7 +1515,7 @@ int cx231xx_read_i2c_master(struct cx231xx *dev, u8 dev_addr, u16 saddr,
if (saddr_len == 0)
saddr = 0;
- else if (saddr_len == 0)
+ else if (saddr_len == 1)
saddr &= 0xff;
/* prepare xfer_data struct */
@@ -1566,7 +1566,7 @@ int cx231xx_write_i2c_master(struct cx231xx *dev, u8 dev_addr, u16 saddr,
if (saddr_len == 0)
saddr = 0;
- else if (saddr_len == 0)
+ else if (saddr_len == 1)
saddr &= 0xff;
/* prepare xfer_data struct */
@@ -1600,7 +1600,7 @@ int cx231xx_read_i2c_data(struct cx231xx *dev, u8 dev_addr, u16 saddr,
if (saddr_len == 0)
saddr = 0;
- else if (saddr_len == 0)
+ else if (saddr_len == 1)
saddr &= 0xff;
/* prepare xfer_data struct */
@@ -1641,7 +1641,7 @@ int cx231xx_write_i2c_data(struct cx231xx *dev, u8 dev_addr, u16 saddr,
if (saddr_len == 0)
saddr = 0;
- else if (saddr_len == 0)
+ else if (saddr_len == 1)
saddr &= 0xff;
/* prepare xfer_data struct */
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [patch] [media] cx231xxx: fix typo in saddr_len check
2010-12-23 19:38 ` [patch] [media] cx231xxx: fix typo in saddr_len check Dan Carpenter
@ 2010-12-23 19:40 ` Sri Deevi
0 siblings, 0 replies; 5+ messages in thread
From: Sri Deevi @ 2010-12-23 19:40 UTC (permalink / raw)
To: 'Dan Carpenter'
Cc: 'Andy Walls', linux-media@vger.kernel.org,
mchehab@infradead.org
I am ok with the changes.
Signed-off-by: Srinivasa Deevi <Srinivasa.deevi@conexant.com>
Sri
-----Original Message-----
From: Dan Carpenter [mailto:error27@gmail.com]
Sent: Thursday, December 23, 2010 11:39 AM
To: Sri Deevi
Cc: 'Andy Walls'; linux-media@vger.kernel.org; mchehab@infradead.org
Subject: [patch] [media] cx231xxx: fix typo in saddr_len check
The original code compared "saddr_len" with zero twice in a nonsensical
way. I asked the list, and Andy Walls and Sri Deevi say that the second
check should be if "saddr_len == 1".
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/media/video/cx231xx/cx231xx-core.c b/drivers/media/video/cx231xx/cx231xx-core.c
index 44d124c..7d62d58 100644
--- a/drivers/media/video/cx231xx/cx231xx-core.c
+++ b/drivers/media/video/cx231xx/cx231xx-core.c
@@ -1515,7 +1515,7 @@ int cx231xx_read_i2c_master(struct cx231xx *dev, u8 dev_addr, u16 saddr,
if (saddr_len == 0)
saddr = 0;
- else if (saddr_len == 0)
+ else if (saddr_len == 1)
saddr &= 0xff;
/* prepare xfer_data struct */
@@ -1566,7 +1566,7 @@ int cx231xx_write_i2c_master(struct cx231xx *dev, u8 dev_addr, u16 saddr,
if (saddr_len == 0)
saddr = 0;
- else if (saddr_len == 0)
+ else if (saddr_len == 1)
saddr &= 0xff;
/* prepare xfer_data struct */
@@ -1600,7 +1600,7 @@ int cx231xx_read_i2c_data(struct cx231xx *dev, u8 dev_addr, u16 saddr,
if (saddr_len == 0)
saddr = 0;
- else if (saddr_len == 0)
+ else if (saddr_len == 1)
saddr &= 0xff;
/* prepare xfer_data struct */
@@ -1641,7 +1641,7 @@ int cx231xx_write_i2c_data(struct cx231xx *dev, u8 dev_addr, u16 saddr,
if (saddr_len == 0)
saddr = 0;
- else if (saddr_len == 0)
+ else if (saddr_len == 1)
saddr &= 0xff;
/* prepare xfer_data struct */
Conexant E-mail Firewall (Conexant.Com) made the following annotations
---------------------------------------------------------------------
********************** Legal Disclaimer ****************************
"This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you."
**********************************************************************
---------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-12-23 19:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-23 16:43 smatch report: cx231xx: incorrect check in cx231xx_write_i2c_data() Dan Carpenter
2010-12-23 18:34 ` Andy Walls
2010-12-23 18:55 ` Dan Carpenter
[not found] ` <34B38BE41EDBA046A4AFBB591FA311320249B057C6@NBMBX01.bbnet.ad>
2010-12-23 19:38 ` [patch] [media] cx231xxx: fix typo in saddr_len check Dan Carpenter
2010-12-23 19:40 ` Sri Deevi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox