* [patch 2/5] drivers/media/video: move dereference after NULL test
@ 2010-03-11 22:02 akpm
2010-03-11 22:38 ` Karicheri, Muralidharan
0 siblings, 1 reply; 6+ messages in thread
From: akpm @ 2010-03-11 22:02 UTC (permalink / raw)
To: mchehab; +Cc: linux-media, akpm, julia
From: Julia Lawall <julia@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 <julia@diku.dk>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Signed-off-by: Andrew Morton <akpm@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)
_
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [patch 2/5] drivers/media/video: move dereference after NULL test
2010-03-11 22:02 [patch 2/5] drivers/media/video: move dereference after NULL test akpm
@ 2010-03-11 22:38 ` Karicheri, Muralidharan
2010-03-11 22:59 ` Andrew Morton
2010-03-12 6:28 ` Julia Lawall
0 siblings, 2 replies; 6+ messages in thread
From: Karicheri, Muralidharan @ 2010-03-11 22:38 UTC (permalink / raw)
To: akpm@linux-foundation.org, mchehab@infradead.org
Cc: linux-media@vger.kernel.org, julia@diku.dk
>-----Original Message-----
>From: linux-media-owner@vger.kernel.org [mailto:linux-media-
>owner@vger.kernel.org] On Behalf Of akpm@linux-foundation.org
>Sent: Thursday, March 11, 2010 5:02 PM
>To: mchehab@infradead.org
>Cc: linux-media@vger.kernel.org; akpm@linux-foundation.org; julia@diku.dk
>Subject: [patch 2/5] drivers/media/video: move dereference after NULL test
>
>From: Julia Lawall <julia@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 <julia@diku.dk>
>Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
>Signed-off-by: Andrew Morton <akpm@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 majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2/5] drivers/media/video: move dereference after NULL test
2010-03-11 22:38 ` Karicheri, Muralidharan
@ 2010-03-11 22:59 ` Andrew Morton
2010-03-12 6:28 ` Julia Lawall
1 sibling, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2010-03-11 22:59 UTC (permalink / raw)
To: Karicheri, Muralidharan
Cc: mchehab@infradead.org, linux-media@vger.kernel.org, julia@diku.dk
On Thu, 11 Mar 2010 16:38:21 -0600
"Karicheri, Muralidharan" <m-karicheri2@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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [patch 2/5] drivers/media/video: move dereference after NULL test
2010-03-11 22:38 ` Karicheri, Muralidharan
2010-03-11 22:59 ` Andrew Morton
@ 2010-03-12 6:28 ` Julia Lawall
2010-03-12 9:15 ` Julia Lawall
1 sibling, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2010-03-12 6:28 UTC (permalink / raw)
To: Karicheri, Muralidharan
Cc: akpm@linux-foundation.org, mchehab@infradead.org,
linux-media@vger.kernel.org
Oops, my mistake. I will fix that.
julia
On Thu, 11 Mar 2010, Karicheri, Muralidharan wrote:
>
> >-----Original Message-----
> >From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> >owner@vger.kernel.org] On Behalf Of akpm@linux-foundation.org
> >Sent: Thursday, March 11, 2010 5:02 PM
> >To: mchehab@infradead.org
> >Cc: linux-media@vger.kernel.org; akpm@linux-foundation.org; julia@diku.dk
> >Subject: [patch 2/5] drivers/media/video: move dereference after NULL test
> >
> >From: Julia Lawall <julia@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 <julia@diku.dk>
> >Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> >Signed-off-by: Andrew Morton <akpm@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 majordomo@vger.kernel.org
> >More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [patch 2/5] drivers/media/video: move dereference after NULL test
2010-03-12 6:28 ` Julia Lawall
@ 2010-03-12 9:15 ` Julia Lawall
2010-03-12 22:40 ` Karicheri, Muralidharan
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2010-03-12 9:15 UTC (permalink / raw)
To: Karicheri, Muralidharan
Cc: akpm@linux-foundation.org, mchehab@infradead.org,
linux-media@vger.kernel.org
From: Julia Lawall <julia@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 <julia@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++) {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [patch 2/5] drivers/media/video: move dereference after NULL test
2010-03-12 9:15 ` Julia Lawall
@ 2010-03-12 22:40 ` Karicheri, Muralidharan
0 siblings, 0 replies; 6+ messages in thread
From: Karicheri, Muralidharan @ 2010-03-12 22:40 UTC (permalink / raw)
To: Julia Lawall
Cc: akpm@linux-foundation.org, mchehab@infradead.org,
linux-media@vger.kernel.org
For drivers/media/video/davinci/vpif_display.c
Acked-by: Muralidharan Karicheri <m-karicheri2@ti.com>
Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
phone: 301-407-9583
email: m-karicheri2@ti.com
>-----Original Message-----
>From: Julia Lawall [mailto:julia@diku.dk]
>Sent: Friday, March 12, 2010 4:16 AM
>To: Karicheri, Muralidharan
>Cc: akpm@linux-foundation.org; mchehab@infradead.org; linux-
>media@vger.kernel.org
>Subject: RE: [patch 2/5] drivers/media/video: move dereference after NULL
>test
>
>From: Julia Lawall <julia@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 <julia@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++) {
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-03-12 22:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-11 22:02 [patch 2/5] drivers/media/video: move dereference after NULL test akpm
2010-03-11 22:38 ` Karicheri, Muralidharan
2010-03-11 22:59 ` Andrew Morton
2010-03-12 6:28 ` Julia Lawall
2010-03-12 9:15 ` Julia Lawall
2010-03-12 22:40 ` Karicheri, Muralidharan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox