From: Mauro Carvalho Chehab <mchehab@infradead.org>
To: julia@diku.dk, "Karicheri, Muralidharan" <m-karicheri2@ti.com>
Cc: akpm@linux-foundation.org, linux-media@vger.kernel.org
Subject: Re: [patch 1/7] drivers/media/video: move dereference after NULL test
Date: Wed, 03 Feb 2010 06:30:42 -0200 [thread overview]
Message-ID: <4B693432.4040101@infradead.org> (raw)
In-Reply-To: <201002022240.o12Mekvr018902@imap1.linux-foundation.org>
Hi Julia,
> From: Julia Lawall <julia@diku.dk>
> 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];
IMO, the better would be to move the if to happen before the usage of std_info, and make it return
a proper error code, instead of -1.
Murali,
Any comments?
> 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");
The better here is to keep the BUG_ON and moving this initialization:
struct snd_card *card = chip->card;
to happen after the test.
>
> 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)
OK.
Cheers,
Mauro
next prev parent reply other threads:[~2010-02-03 8:30 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-02 22:40 [patch 1/7] drivers/media/video: move dereference after NULL test akpm
2010-02-03 8:30 ` Mauro Carvalho Chehab [this message]
2010-02-03 12:07 ` Julia Lawall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B693432.4040101@infradead.org \
--to=mchehab@infradead.org \
--cc=akpm@linux-foundation.org \
--cc=julia@diku.dk \
--cc=linux-media@vger.kernel.org \
--cc=m-karicheri2@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox