* 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
[parent not found: <34B38BE41EDBA046A4AFBB591FA311320249B057C6@NBMBX01.bbnet.ad>]
* [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