RE: [patch 2/5] drivers/media/video: move dereference after NULL test

2010-03-12 Thread Julia Lawall 
From: Julia Lawall ju...@diku.dk

In quickcam_messenger.c, if the NULL test on uvd is needed, then the
dereference should be after the NULL test.

In vpif_display.c, std_info is initialized to the address of a structure
field.  This seems unlikely to be NULL.  Test std_info-stdid instead.

In saa7134-alsa.c, the function is only called from one place, where the
chip argument has already been dereferenced.  On the other hand, if it
should be kept, then card should be initialized after it.

A simplified version of the semantic match that detects this problem is as
follows (http://coccinelle.lip6.fr/):

// smpl
@match exists@
expression x, E;
identifier fld;
@@

* x-fld
  ... when != \(x = E\|x\)
* x == NULL
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk

---
 drivers/media/video/davinci/vpif_display.c|2 +-
 drivers/media/video/saa7134/saa7134-alsa.c|2 --
 drivers/media/video/usbvideo/quickcam_messenger.c |3 ++-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/usbvideo/quickcam_messenger.c 
b/drivers/media/video/usbvideo/quickcam_messenger.c
index 803d3e4..f0043d0 100644
--- a/drivers/media/video/usbvideo/quickcam_messenger.c
+++ b/drivers/media/video/usbvideo/quickcam_messenger.c
@@ -692,12 +692,13 @@ static int qcm_start_data(struct uvd *uvd)
 
 static void qcm_stop_data(struct uvd *uvd)
 {
-   struct qcm *cam = (struct qcm *) uvd-user_data;
+   struct qcm *cam;
int i, j;
int ret;
 
if ((uvd == NULL) || (!uvd-streaming) || (uvd-dev == NULL))
return;
+   cam = (struct qcm *) uvd-user_data;
 
ret = qcm_camera_off(uvd);
if (ret)
diff --git a/drivers/media/video/saa7134/saa7134-alsa.c 
b/drivers/media/video/saa7134/saa7134-alsa.c
index d48c450..d3bd82a 100644
--- a/drivers/media/video/saa7134/saa7134-alsa.c
+++ b/drivers/media/video/saa7134/saa7134-alsa.c
@@ -1011,8 +1011,6 @@ static int snd_card_saa7134_new_mixer(snd_card_saa7134_t 
* chip)
unsigned int idx;
int err, addr;
 
-   if (snd_BUG_ON(!chip))
-   return -EINVAL;
strcpy(card-mixername, SAA7134 Mixer);
 
for (idx = 0; idx  ARRAY_SIZE(snd_saa7134_volume_controls); idx++) {
diff --git a/drivers/media/video/davinci/vpif_display.c 
b/drivers/media/video/davinci/vpif_display.c
index dfddef7..b2dce78 100644
--- a/drivers/media/video/davinci/vpif_display.c
+++ b/drivers/media/video/davinci/vpif_display.c
@@ -383,7 +383,7 @@ static int vpif_get_std_info(struct channel_obj *ch)
int index;
 
std_info-stdid = vid_ch-stdid;
-   if (!std_info)
+   if (!std_info-stdid)
return -1;
 
for (index = 0; index  ARRAY_SIZE(ch_params); index++) {
--
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: [patch 2/5] drivers/media/video: move dereference after NULL test

2010-03-12 Thread Karicheri, Muralidharan 
For drivers/media/video/davinci/vpif_display.c

Acked-by: Muralidharan Karicheri m-kariche...@ti.com

Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
phone: 301-407-9583
email: m-kariche...@ti.com

-Original Message-
From: Julia Lawall [mailto:ju...@diku.dk]
Sent: Friday, March 12, 2010 4:16 AM
To: Karicheri, Muralidharan
Cc: a...@linux-foundation.org; mche...@infradead.org; linux-
me...@vger.kernel.org
Subject: RE: [patch 2/5] drivers/media/video: move dereference after NULL
test

From: Julia Lawall ju...@diku.dk

In quickcam_messenger.c, if the NULL test on uvd is needed, then the
dereference should be after the NULL test.

In vpif_display.c, std_info is initialized to the address of a structure
field.  This seems unlikely to be NULL.  Test std_info-stdid instead.

In saa7134-alsa.c, the function is only called from one place, where the
chip argument has already been dereferenced.  On the other hand, if it
should be kept, then card should be initialized after it.

A simplified version of the semantic match that detects this problem is as
follows (http://coccinelle.lip6.fr/):

// smpl
@match exists@
expression x, E;
identifier fld;
@@

* x-fld
  ... when != \(x = E\|x\)
* x == NULL
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk

---
 drivers/media/video/davinci/vpif_display.c|2 +-
 drivers/media/video/saa7134/saa7134-alsa.c|2 --
 drivers/media/video/usbvideo/quickcam_messenger.c |3 ++-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/usbvideo/quickcam_messenger.c
b/drivers/media/video/usbvideo/quickcam_messenger.c
index 803d3e4..f0043d0 100644
--- a/drivers/media/video/usbvideo/quickcam_messenger.c
+++ b/drivers/media/video/usbvideo/quickcam_messenger.c
@@ -692,12 +692,13 @@ static int qcm_start_data(struct uvd *uvd)

 static void qcm_stop_data(struct uvd *uvd)
 {
-  struct qcm *cam = (struct qcm *) uvd-user_data;
+  struct qcm *cam;
   int i, j;
   int ret;

   if ((uvd == NULL) || (!uvd-streaming) || (uvd-dev == NULL))
   return;
+  cam = (struct qcm *) uvd-user_data;

   ret = qcm_camera_off(uvd);
   if (ret)
diff --git a/drivers/media/video/saa7134/saa7134-alsa.c
b/drivers/media/video/saa7134/saa7134-alsa.c
index d48c450..d3bd82a 100644
--- a/drivers/media/video/saa7134/saa7134-alsa.c
+++ b/drivers/media/video/saa7134/saa7134-alsa.c
@@ -1011,8 +1011,6 @@ static int
snd_card_saa7134_new_mixer(snd_card_saa7134_t * chip)
   unsigned int idx;
   int err, addr;

-  if (snd_BUG_ON(!chip))
-  return -EINVAL;
   strcpy(card-mixername, SAA7134 Mixer);

   for (idx = 0; idx  ARRAY_SIZE(snd_saa7134_volume_controls); idx++) {
diff --git a/drivers/media/video/davinci/vpif_display.c
b/drivers/media/video/davinci/vpif_display.c
index dfddef7..b2dce78 100644
--- a/drivers/media/video/davinci/vpif_display.c
+++ b/drivers/media/video/davinci/vpif_display.c
@@ -383,7 +383,7 @@ static int vpif_get_std_info(struct channel_obj *ch)
   int index;

   std_info-stdid = vid_ch-stdid;
-  if (!std_info)
+  if (!std_info-stdid)
   return -1;

   for (index = 0; index  ARRAY_SIZE(ch_params); index++) {
--
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

[patch 2/5] drivers/media/video: move dereference after NULL test

2010-03-11 Thread akpm 
From: Julia Lawall ju...@diku.dk

In quickcam_messenger.c, if the NULL test on uvd is needed, then the
dereference should be after the NULL test.

In vpif_display.c, std_info is initialized to the address of a structure
field.  This seems unlikely to be NULL.  If it could somehow be NULL, then
the assignment should be moved after the NULL test.  Alternatively, perhaps
the NULL test is intended to test std_info-stdid rather than std_info?

In saa7134-alsa.c, the function is only called from one place, where the
chip argument has already been dereferenced.  On the other hand, if it
should be kept, then card should be initialized after it.

A simplified version of the semantic match that detects this problem is as
follows (http://coccinelle.lip6.fr/):

// smpl
@match exists@
expression x, E;
identifier fld;
@@

* x-fld
  ... when != \(x = E\|x\)
* x == NULL
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk
Cc: Mauro Carvalho Chehab mche...@infradead.org
Signed-off-by: Andrew Morton a...@linux-foundation.org
---

 drivers/media/video/davinci/vpif_display.c|2 --
 drivers/media/video/saa7134/saa7134-alsa.c|2 --
 drivers/media/video/usbvideo/quickcam_messenger.c |3 ++-
 3 files changed, 2 insertions(+), 5 deletions(-)

diff -puN 
drivers/media/video/davinci/vpif_display.c~drivers-media-video-move-dereference-after-null-test
 drivers/media/video/davinci/vpif_display.c
--- 
a/drivers/media/video/davinci/vpif_display.c~drivers-media-video-move-dereference-after-null-test
+++ a/drivers/media/video/davinci/vpif_display.c
@@ -383,8 +383,6 @@ static int vpif_get_std_info(struct chan
int index;
 
std_info-stdid = vid_ch-stdid;
-   if (!std_info)
-   return -1;
 
for (index = 0; index  ARRAY_SIZE(ch_params); index++) {
config = ch_params[index];
diff -puN 
drivers/media/video/saa7134/saa7134-alsa.c~drivers-media-video-move-dereference-after-null-test
 drivers/media/video/saa7134/saa7134-alsa.c
--- 
a/drivers/media/video/saa7134/saa7134-alsa.c~drivers-media-video-move-dereference-after-null-test
+++ a/drivers/media/video/saa7134/saa7134-alsa.c
@@ -1011,8 +1011,6 @@ static int snd_card_saa7134_new_mixer(sn
unsigned int idx;
int err, addr;
 
-   if (snd_BUG_ON(!chip))
-   return -EINVAL;
strcpy(card-mixername, SAA7134 Mixer);
 
for (idx = 0; idx  ARRAY_SIZE(snd_saa7134_volume_controls); idx++) {
diff -puN 
drivers/media/video/usbvideo/quickcam_messenger.c~drivers-media-video-move-dereference-after-null-test
 drivers/media/video/usbvideo/quickcam_messenger.c
--- 
a/drivers/media/video/usbvideo/quickcam_messenger.c~drivers-media-video-move-dereference-after-null-test
+++ a/drivers/media/video/usbvideo/quickcam_messenger.c
@@ -692,12 +692,13 @@ static int qcm_start_data(struct uvd *uv
 
 static void qcm_stop_data(struct uvd *uvd)
 {
-   struct qcm *cam = (struct qcm *) uvd-user_data;
+   struct qcm *cam;
int i, j;
int ret;
 
if ((uvd == NULL) || (!uvd-streaming) || (uvd-dev == NULL))
return;
+   cam = (struct qcm *) uvd-user_data;
 
ret = qcm_camera_off(uvd);
if (ret)
_
--
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: [patch 2/5] drivers/media/video: move dereference after NULL test

2010-03-11 Thread Karicheri, Muralidharan 

-Original Message-
From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
ow...@vger.kernel.org] On Behalf Of a...@linux-foundation.org
Sent: Thursday, March 11, 2010 5:02 PM
To: mche...@infradead.org
Cc: linux-media@vger.kernel.org; a...@linux-foundation.org; ju...@diku.dk
Subject: [patch 2/5] drivers/media/video: move dereference after NULL test

From: Julia Lawall ju...@diku.dk

In quickcam_messenger.c, if the NULL test on uvd is needed, then the
dereference should be after the NULL test.

In vpif_display.c, std_info is initialized to the address of a structure
field.  This seems unlikely to be NULL.  If it could somehow be NULL, then
the assignment should be moved after the NULL test.  Alternatively, perhaps
the NULL test is intended to test std_info-stdid rather than std_info?

In saa7134-alsa.c, the function is only called from one place, where the
chip argument has already been dereferenced.  On the other hand, if it
should be kept, then card should be initialized after it.

A simplified version of the semantic match that detects this problem is as
follows (http://coccinelle.lip6.fr/):

// smpl
@match exists@
expression x, E;
identifier fld;
@@

* x-fld
  ... when != \(x = E\|x\)
* x == NULL
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk
Cc: Mauro Carvalho Chehab mche...@infradead.org
Signed-off-by: Andrew Morton a...@linux-foundation.org
---

 drivers/media/video/davinci/vpif_display.c|2 --
 drivers/media/video/saa7134/saa7134-alsa.c|2 --
 drivers/media/video/usbvideo/quickcam_messenger.c |3 ++-
 3 files changed, 2 insertions(+), 5 deletions(-)

diff -puN drivers/media/video/davinci/vpif_display.c~drivers-media-video-
move-dereference-after-null-test drivers/media/video/davinci/vpif_display.c
--- a/drivers/media/video/davinci/vpif_display.c~drivers-media-video-move-
dereference-after-null-test
+++ a/drivers/media/video/davinci/vpif_display.c
@@ -383,8 +383,6 @@ static int vpif_get_std_info(struct chan
   int index;

   std_info-stdid = vid_ch-stdid;
-  if (!std_info)
-  return -1;

Please change it as 

if (!std_info-stdid)
return -1;

Murali  

   for (index = 0; index  ARRAY_SIZE(ch_params); index++) {
   config = ch_params[index];
diff -puN drivers/media/video/saa7134/saa7134-alsa.c~drivers-media-video-
move-dereference-after-null-test drivers/media/video/saa7134/saa7134-alsa.c
--- a/drivers/media/video/saa7134/saa7134-alsa.c~drivers-media-video-move-
dereference-after-null-test
+++ a/drivers/media/video/saa7134/saa7134-alsa.c
@@ -1011,8 +1011,6 @@ static int snd_card_saa7134_new_mixer(sn
   unsigned int idx;
   int err, addr;

-  if (snd_BUG_ON(!chip))
-  return -EINVAL;
   strcpy(card-mixername, SAA7134 Mixer);

   for (idx = 0; idx  ARRAY_SIZE(snd_saa7134_volume_controls); idx++) {
diff -puN drivers/media/video/usbvideo/quickcam_messenger.c~drivers-media-
video-move-dereference-after-null-test
drivers/media/video/usbvideo/quickcam_messenger.c
--- a/drivers/media/video/usbvideo/quickcam_messenger.c~drivers-media-
video-move-dereference-after-null-test
+++ a/drivers/media/video/usbvideo/quickcam_messenger.c
@@ -692,12 +692,13 @@ static int qcm_start_data(struct uvd *uv

 static void qcm_stop_data(struct uvd *uvd)
 {
-  struct qcm *cam = (struct qcm *) uvd-user_data;
+  struct qcm *cam;
   int i, j;
   int ret;

   if ((uvd == NULL) || (!uvd-streaming) || (uvd-dev == NULL))
   return;
+  cam = (struct qcm *) uvd-user_data;

   ret = qcm_camera_off(uvd);
   if (ret)
_
--
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
--
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: [patch 2/5] drivers/media/video: move dereference after NULL test

2010-03-11 Thread Andrew Morton 
On Thu, 11 Mar 2010 16:38:21 -0600
Karicheri, Muralidharan m-kariche...@ti.com wrote:

 diff -puN drivers/media/video/davinci/vpif_display.c~drivers-media-video-
 move-dereference-after-null-test drivers/media/video/davinci/vpif_display.c
 --- a/drivers/media/video/davinci/vpif_display.c~drivers-media-video-move-
 dereference-after-null-test
 +++ a/drivers/media/video/davinci/vpif_display.c
 @@ -383,8 +383,6 @@ static int vpif_get_std_info(struct chan
  int index;
 
  std_info-stdid = vid_ch-stdid;
 -if (!std_info)
 -return -1;
 
 Please change it as 
 
 if (!std_info-stdid)
   return -1;

Could you please do this, and send the patch?  It's better that way as
you're more familar with the code and maybe can even test it.

--
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: [patch 2/5] drivers/media/video: move dereference after NULL test

2010-03-11 Thread Julia Lawall 
Oops, my mistake.  I will fix that.

julia


On Thu, 11 Mar 2010, Karicheri, Muralidharan wrote:

 
 -Original Message-
 From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
 ow...@vger.kernel.org] On Behalf Of a...@linux-foundation.org
 Sent: Thursday, March 11, 2010 5:02 PM
 To: mche...@infradead.org
 Cc: linux-media@vger.kernel.org; a...@linux-foundation.org; ju...@diku.dk
 Subject: [patch 2/5] drivers/media/video: move dereference after NULL test
 
 From: Julia Lawall ju...@diku.dk
 
 In quickcam_messenger.c, if the NULL test on uvd is needed, then the
 dereference should be after the NULL test.
 
 In vpif_display.c, std_info is initialized to the address of a structure
 field.  This seems unlikely to be NULL.  If it could somehow be NULL, then
 the assignment should be moved after the NULL test.  Alternatively, perhaps
 the NULL test is intended to test std_info-stdid rather than std_info?
 
 In saa7134-alsa.c, the function is only called from one place, where the
 chip argument has already been dereferenced.  On the other hand, if it
 should be kept, then card should be initialized after it.
 
 A simplified version of the semantic match that detects this problem is as
 follows (http://coccinelle.lip6.fr/):
 
 // smpl
 @match exists@
 expression x, E;
 identifier fld;
 @@
 
 * x-fld
   ... when != \(x = E\|x\)
 * x == NULL
 // /smpl
 
 Signed-off-by: Julia Lawall ju...@diku.dk
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Signed-off-by: Andrew Morton a...@linux-foundation.org
 ---
 
  drivers/media/video/davinci/vpif_display.c|2 --
  drivers/media/video/saa7134/saa7134-alsa.c|2 --
  drivers/media/video/usbvideo/quickcam_messenger.c |3 ++-
  3 files changed, 2 insertions(+), 5 deletions(-)
 
 diff -puN drivers/media/video/davinci/vpif_display.c~drivers-media-video-
 move-dereference-after-null-test drivers/media/video/davinci/vpif_display.c
 --- a/drivers/media/video/davinci/vpif_display.c~drivers-media-video-move-
 dereference-after-null-test
 +++ a/drivers/media/video/davinci/vpif_display.c
 @@ -383,8 +383,6 @@ static int vpif_get_std_info(struct chan
  int index;
 
  std_info-stdid = vid_ch-stdid;
 -if (!std_info)
 -return -1;
 
 Please change it as 
 
 if (!std_info-stdid)
   return -1;
 
 Murali
 
  for (index = 0; index  ARRAY_SIZE(ch_params); index++) {
  config = ch_params[index];
 diff -puN drivers/media/video/saa7134/saa7134-alsa.c~drivers-media-video-
 move-dereference-after-null-test drivers/media/video/saa7134/saa7134-alsa.c
 --- a/drivers/media/video/saa7134/saa7134-alsa.c~drivers-media-video-move-
 dereference-after-null-test
 +++ a/drivers/media/video/saa7134/saa7134-alsa.c
 @@ -1011,8 +1011,6 @@ static int snd_card_saa7134_new_mixer(sn
  unsigned int idx;
  int err, addr;
 
 -if (snd_BUG_ON(!chip))
 -return -EINVAL;
  strcpy(card-mixername, SAA7134 Mixer);
 
  for (idx = 0; idx  ARRAY_SIZE(snd_saa7134_volume_controls); idx++) {
 diff -puN drivers/media/video/usbvideo/quickcam_messenger.c~drivers-media-
 video-move-dereference-after-null-test
 drivers/media/video/usbvideo/quickcam_messenger.c
 --- a/drivers/media/video/usbvideo/quickcam_messenger.c~drivers-media-
 video-move-dereference-after-null-test
 +++ a/drivers/media/video/usbvideo/quickcam_messenger.c
 @@ -692,12 +692,13 @@ static int qcm_start_data(struct uvd *uv
 
  static void qcm_stop_data(struct uvd *uvd)
  {
 -struct qcm *cam = (struct qcm *) uvd-user_data;
 +struct qcm *cam;
  int i, j;
  int ret;
 
  if ((uvd == NULL) || (!uvd-streaming) || (uvd-dev == NULL))
  return;
 +cam = (struct qcm *) uvd-user_data;
 
  ret = qcm_camera_off(uvd);
  if (ret)
 _
 --
 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
 
--
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