diff mbox

[FFmpeg-devel,v2] avfilter/vf_libvmaf: Check for av_frame_alloc failure

Message ID 20191120152422.20392-1-lance.lmwang@gmail.com
State New
Headers show

Commit Message

Limin Wang Nov. 20, 2019, 3:24 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
Add Reviewed-by:, pleae help push it

 libavfilter/vf_libvmaf.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Limin Wang Nov. 27, 2019, 10:20 a.m. UTC | #1
On Wed, Nov 27, 2019 at 11:09:23AM +0100, Nikola Pajkovsky wrote:
> lance.lmwang@gmail.com writes:
> 
> > From: Limin Wang <lance.lmwang@gmail.com>
> >
> > Reviewed-by: Paul B Mahol <onemda@gmail.com>
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > Add Reviewed-by:, pleae help push it
> >
> >  libavfilter/vf_libvmaf.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
> > index ed3a383..14c3216 100644
> > --- a/libavfilter/vf_libvmaf.c
> > +++ b/libavfilter/vf_libvmaf.c
> > @@ -235,6 +235,9 @@ static av_cold int init(AVFilterContext *ctx)
> >  
> >      s->gref = av_frame_alloc();
> >      s->gmain = av_frame_alloc();
> > +    if (!s->gref || !s->gmain)
> > +        return AVERROR(ENOMEM);
> > +
> 
> When av_frame_alloc() return allocated memory for s->gref and
> av_frame_alloc() fails for s->gmain, it will leak memory for s->gref.
> 
> Correct code is:
> 
>     s->gref = av_frame_alloc();
>     if (!s->gref)
>         return AVERROR(ENOMEM);
> 
>     s->gmain = av_frame_alloc();
>     if (!s->gmain)
>         return AVERROR(ENOMEM);

Thank for your review, I recall they'll be freed in uninit function, so
my patch looks simple.


> 
> -- 
> Nikola
Paul B Mahol Nov. 27, 2019, 10:26 a.m. UTC | #2
On 11/27/19, Limin Wang <lance.lmwang@gmail.com> wrote:
> On Wed, Nov 27, 2019 at 11:09:23AM +0100, Nikola Pajkovsky wrote:
>> lance.lmwang@gmail.com writes:
>>
>> > From: Limin Wang <lance.lmwang@gmail.com>
>> >
>> > Reviewed-by: Paul B Mahol <onemda@gmail.com>
>> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> > ---
>> > Add Reviewed-by:, pleae help push it
>> >
>> >  libavfilter/vf_libvmaf.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>> >
>> > diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
>> > index ed3a383..14c3216 100644
>> > --- a/libavfilter/vf_libvmaf.c
>> > +++ b/libavfilter/vf_libvmaf.c
>> > @@ -235,6 +235,9 @@ static av_cold int init(AVFilterContext *ctx)
>> >
>> >      s->gref = av_frame_alloc();
>> >      s->gmain = av_frame_alloc();
>> > +    if (!s->gref || !s->gmain)
>> > +        return AVERROR(ENOMEM);
>> > +
>>
>> When av_frame_alloc() return allocated memory for s->gref and
>> av_frame_alloc() fails for s->gmain, it will leak memory for s->gref.
>>
>> Correct code is:
>>
>>     s->gref = av_frame_alloc();
>>     if (!s->gref)
>>         return AVERROR(ENOMEM);
>>
>>     s->gmain = av_frame_alloc();
>>     if (!s->gmain)
>>         return AVERROR(ENOMEM);
>
> Thank for your review, I recall they'll be freed in uninit function, so
> my patch looks simple.
>

Yes, patch should be applied as is.

>
>>
>> --
>> Nikola
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Michael Niedermayer Nov. 28, 2019, 2:33 p.m. UTC | #3
On Wed, Nov 27, 2019 at 11:26:15AM +0100, Paul B Mahol wrote:
> On 11/27/19, Limin Wang <lance.lmwang@gmail.com> wrote:
> > On Wed, Nov 27, 2019 at 11:09:23AM +0100, Nikola Pajkovsky wrote:
> >> lance.lmwang@gmail.com writes:
> >>
> >> > From: Limin Wang <lance.lmwang@gmail.com>
> >> >
> >> > Reviewed-by: Paul B Mahol <onemda@gmail.com>
> >> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >> > ---
> >> > Add Reviewed-by:, pleae help push it
> >> >
> >> >  libavfilter/vf_libvmaf.c | 3 +++
> >> >  1 file changed, 3 insertions(+)
> >> >
> >> > diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
> >> > index ed3a383..14c3216 100644
> >> > --- a/libavfilter/vf_libvmaf.c
> >> > +++ b/libavfilter/vf_libvmaf.c
> >> > @@ -235,6 +235,9 @@ static av_cold int init(AVFilterContext *ctx)
> >> >
> >> >      s->gref = av_frame_alloc();
> >> >      s->gmain = av_frame_alloc();
> >> > +    if (!s->gref || !s->gmain)
> >> > +        return AVERROR(ENOMEM);
> >> > +
> >>
> >> When av_frame_alloc() return allocated memory for s->gref and
> >> av_frame_alloc() fails for s->gmain, it will leak memory for s->gref.
> >>
> >> Correct code is:
> >>
> >>     s->gref = av_frame_alloc();
> >>     if (!s->gref)
> >>         return AVERROR(ENOMEM);
> >>
> >>     s->gmain = av_frame_alloc();
> >>     if (!s->gmain)
> >>         return AVERROR(ENOMEM);
> >
> > Thank for your review, I recall they'll be freed in uninit function, so
> > my patch looks simple.
> >
> 
> Yes, patch should be applied as is.

will apply

thx

[...]
diff mbox

Patch

diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
index ed3a383..14c3216 100644
--- a/libavfilter/vf_libvmaf.c
+++ b/libavfilter/vf_libvmaf.c
@@ -235,6 +235,9 @@  static av_cold int init(AVFilterContext *ctx)
 
     s->gref = av_frame_alloc();
     s->gmain = av_frame_alloc();
+    if (!s->gref || !s->gmain)
+        return AVERROR(ENOMEM);
+
     s->error = 0;
 
     s->vmaf_thread_created = 0;