re: [media] Add driver for Samsung S5K5BAF camera sensor

2014-01-08 Thread Dan Carpenter 
Hello Andrzej Hajda,

This is a semi-automatic email about new static checker warnings.

The patch 7d459937dc09: [media] Add driver for Samsung S5K5BAF 
camera sensor from Dec 5, 2013, leads to the following Smatch 
complaint:

drivers/media/i2c/s5k5baf.c:554 s5k5baf_fw_get_seq()
 warn: variable dereferenced before check 'fw' (see line 551)

drivers/media/i2c/s5k5baf.c
   550  struct s5k5baf_fw *fw = state-fw;
   551  u16 *data = fw-data + 2 * fw-count;
   ^
Dereference.

   552  int i;
   553  
   554  if (fw == NULL)
^^
Check.

   555  return NULL;
   556  

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: [media] Add driver for Samsung S5K5BAF camera sensor

2014-01-08 Thread Dan Carpenter 
Hello Andrzej Hajda,

The patch 7d459937dc09: [media] Add driver for Samsung S5K5BAF
camera sensor from Dec 5, 2013, leads to the following
static checker warning:

drivers/media/i2c/s5k5baf.c:1043 s5k5baf_set_power()
warn: add some parenthesis here?

drivers/media/i2c/s5k5baf.c
  1036  static int s5k5baf_set_power(struct v4l2_subdev *sd, int on)
  1037  {
  1038  struct s5k5baf *state = to_s5k5baf(sd);
  1039  int ret = 0;
  1040  
  1041  mutex_lock(state-lock);
  1042  
  1043  if (!on != state-power)
^^^
This would be cleaner if it were if (on == state-power)

  1044  goto out;
  1045  
  1046  if (on) {

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: [media] Add driver for Samsung S5K5BAF camera sensor

2014-01-08 Thread Sachin Kamat 
Hi Dan,

On 8 January 2014 14:39, Dan Carpenter dan.carpen...@oracle.com wrote:
 Hello Andrzej Hajda,

 This is a semi-automatic email about new static checker warnings.

 The patch 7d459937dc09: [media] Add driver for Samsung S5K5BAF
 camera sensor from Dec 5, 2013, leads to the following Smatch
 complaint:

 drivers/media/i2c/s5k5baf.c:554 s5k5baf_fw_get_seq()
  warn: variable dereferenced before check 'fw' (see line 551)

 drivers/media/i2c/s5k5baf.c
550  struct s5k5baf_fw *fw = state-fw;
551  u16 *data = fw-data + 2 * fw-count;
^
 Dereference.

552  int i;
553
554  if (fw == NULL)
 ^^
 Check.

555  return NULL;
556


A patch [1] to fix this has already been queued up by Mauro.

[1] https://linuxtv.org/patch/21292/

-- 
With warm regards,
Sachin
--
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: [media] Add driver for Samsung S5K5BAF camera sensor

2014-01-08 Thread Andrzej Hajda 
On 01/08/2014 10:58 AM, Dan Carpenter wrote:
 Hello Andrzej Hajda,
 
 The patch 7d459937dc09: [media] Add driver for Samsung S5K5BAF
 camera sensor from Dec 5, 2013, leads to the following
 static checker warning:
 
   drivers/media/i2c/s5k5baf.c:1043 s5k5baf_set_power()
   warn: add some parenthesis here?
 
 drivers/media/i2c/s5k5baf.c
   1036  static int s5k5baf_set_power(struct v4l2_subdev *sd, int on)
   1037  {
   1038  struct s5k5baf *state = to_s5k5baf(sd);
   1039  int ret = 0;
   1040  
   1041  mutex_lock(state-lock);
   1042  
   1043  if (!on != state-power)
 ^^^
 This would be cleaner if it were if (on == state-power)

This version works correctly only for 'on' equal 0 and 1, my version
works for all ints. On the other side documentation says only 0 and 1 is
allowed for s_power callbacks :)
I would stay with my version, similar approach is in other drivers.

Regards
Andrzej

 
   1044  goto out;
   1045  
   1046  if (on) {
 
 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: [media] Add driver for Samsung S5K5BAF camera sensor

2014-01-08 Thread Dan Carpenter 
On Wed, Jan 08, 2014 at 12:58:35PM +0100, Andrzej Hajda wrote:
 On 01/08/2014 10:58 AM, Dan Carpenter wrote:
  Hello Andrzej Hajda,
  
  The patch 7d459937dc09: [media] Add driver for Samsung S5K5BAF
  camera sensor from Dec 5, 2013, leads to the following
  static checker warning:
  
  drivers/media/i2c/s5k5baf.c:1043 s5k5baf_set_power()
  warn: add some parenthesis here?
  
  drivers/media/i2c/s5k5baf.c
1036  static int s5k5baf_set_power(struct v4l2_subdev *sd, int on)
1037  {
1038  struct s5k5baf *state = to_s5k5baf(sd);
1039  int ret = 0;
1040  
1041  mutex_lock(state-lock);
1042  
1043  if (!on != state-power)
  ^^^
  This would be cleaner if it were if (on == state-power)
 
 This version works correctly only for 'on' equal 0 and 1, my version
 works for all ints. On the other side documentation says only 0 and 1 is
 allowed for s_power callbacks :)
 I would stay with my version, similar approach is in other drivers.

Even if (!!on == state-power) like you do in s5k5baf_s_stream() would
be more readable than the current code.

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