[vlc-devel] [PATCH 1/2] cvpx: fix error case in Open

[Thomas Guillem]

On Thu, Aug 13, 2020, at 11:07, Alexandre Janniaux wrote:
> Hi,
> 
> On Wed, Aug 12, 2020 at 10:34:56AM +0200, Thomas Guillem wrote:
> > Hello,
> >
> > On Tue, Jul 28, 2020, at 13:54, Alexandre Janniaux wrote:
> > > As the code was returning `ret` but didn't set it before jumping to the
> > > error: label. In case of error, p_sys wasn't NULL too, which could lead
> > > to failure in other filters.
> > > ---
> > >  modules/video_chroma/cvpx.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/modules/video_chroma/cvpx.c b/modules/video_chroma/cvpx.c
> > > index 00736d8ea0..147d064ff7 100644
> > > --- a/modules/video_chroma/cvpx.c
> > > +++ b/modules/video_chroma/cvpx.c
> > > @@ -361,7 +361,8 @@ static int Open(vlc_object_t *obj)
> > >      return VLC_SUCCESS;
> > >  error:
> > >      Close(obj);
> > > -    return ret;
> > > +    p_filter->p_sys = NULL;
> >
> > Not needed if patch 2/2 is applied, right?
> >
> 
> Yes it is, different submodule. :)
> 
> >
> > > +    return ret == VLC_SUCCESS ? VLC_EGENERIC : ret;
> >
> > This seems counterintuitive.
> >
> > Either, always return VLC_EGENERIC or set ret before all goto errors;
> 
> 
> Better with the following?
> 
>    if (ret != VLC_SUCCESS)
>       return ret;
> 
>    return VLC_EGENERIC;
I still prefer  "return VLC_EGENERIC;" or setting ret before goto.

But I'm nitpicking now, you can do as you want.

> 
> >
> > >  #undef CASE_CVPX_INPUT
> > >  #undef CASE_CVPX_OUTPUT
> > >  }
> > > --
> > > 2.27.0
> > >
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel