On Wed, 3 Feb 2010, Mauro Carvalho Chehab wrote:

> Hi Julia,
> 
> > From: Julia Lawall <[email protected]>
>  
> 
> > 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.

The initializations are as follows:

static int vpif_get_std_info(struct channel_obj *ch)
{
        struct common_obj *common = &ch->common[VPIF_VIDEO_INDEX];
        struct video_obj *vid_ch = &ch->video;
        struct vpif_params *vpifparams = &ch->vpifparams;
        struct vpif_channel_config_params *std_info = &vpifparams->std_info;

While std_info could be an invalid address, I don't think it would be 
likely to be NULL.  An option would be to test whether ch is NULL.  But 
the function is static, and at all of the call sites either ch or a 
pointer derived from it has already been dereferenced, so perhaps the test 
is not necessary.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to