diff mbox series

[FFmpeg-devel,V1] lavfi/buffersrc: Remove redundancy free after ff_filter_frame fail

Message ID 1577856133-17464-1-git-send-email-mypopydev@gmail.com
State New
Headers show
Series [FFmpeg-devel,V1] lavfi/buffersrc: Remove redundancy free after ff_filter_frame fail | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Jun Zhao Jan. 1, 2020, 5:22 a.m. UTC
From: Jun Zhao <barryjzhao@tencent.com>

ff_filter_frame always free the frame in case of error, so we don't
need to free the frame after ff_filter_frame fail.

Signed-off-by: Jun Zhao <barryjzhao@tencent.com>
---
 libavfilter/buffersrc.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

Comments

Andreas Rheinhardt Jan. 4, 2020, 10:50 a.m. UTC | #1
Jun Zhao:
> From: Jun Zhao <barryjzhao@tencent.com>
> 
> ff_filter_frame always free the frame in case of error, so we don't
> need to free the frame after ff_filter_frame fail.
> 
> Signed-off-by: Jun Zhao <barryjzhao@tencent.com>
> ---
>  libavfilter/buffersrc.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
> index 64940d9..bf30f54 100644
> --- a/libavfilter/buffersrc.c
> +++ b/libavfilter/buffersrc.c
> @@ -240,10 +240,8 @@ static int av_buffersrc_add_frame_internal(AVFilterContext *ctx,
>      }
>  
>      ret = ff_filter_frame(ctx->outputs[0], copy);
> -    if (ret < 0) {
> -        av_frame_free(&copy);
> +    if (ret < 0)
>          return ret;
> -    }
>  
>      if ((flags & AV_BUFFERSRC_FLAG_PUSH)) {
>          ret = push_frame(ctx->graph);
> 

Given that ff_filter_frame can't reset copy to NULL upon freeing it,
this actually fixes a double-free. Coverity just complained about this
(it's CID 1457230).

- Andreas
Paul B Mahol Jan. 4, 2020, 1:10 p.m. UTC | #2
LGTM

On 1/1/20, Jun Zhao <mypopydev@gmail.com> wrote:
> From: Jun Zhao <barryjzhao@tencent.com>
>
> ff_filter_frame always free the frame in case of error, so we don't
> need to free the frame after ff_filter_frame fail.
>
> Signed-off-by: Jun Zhao <barryjzhao@tencent.com>
> ---
>  libavfilter/buffersrc.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
> index 64940d9..bf30f54 100644
> --- a/libavfilter/buffersrc.c
> +++ b/libavfilter/buffersrc.c
> @@ -240,10 +240,8 @@ static int
> av_buffersrc_add_frame_internal(AVFilterContext *ctx,
>      }
>
>      ret = ff_filter_frame(ctx->outputs[0], copy);
> -    if (ret < 0) {
> -        av_frame_free(&copy);
> +    if (ret < 0)
>          return ret;
> -    }
>
>      if ((flags & AV_BUFFERSRC_FLAG_PUSH)) {
>          ret = push_frame(ctx->graph);
> --
> 1.7.1
>
> _______________________________________________
> 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".
Nicolas George Jan. 4, 2020, 6:44 p.m. UTC | #3
Jun Zhao (12020-01-01):
> From: Jun Zhao <barryjzhao@tencent.com>
> 
> ff_filter_frame always free the frame in case of error, so we don't
> need to free the frame after ff_filter_frame fail.
> 
> Signed-off-by: Jun Zhao <barryjzhao@tencent.com>
> ---
>  libavfilter/buffersrc.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)

Thanks for the fix. Pushed with a few amendments to the commit message.

Regards,
diff mbox series

Patch

diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
index 64940d9..bf30f54 100644
--- a/libavfilter/buffersrc.c
+++ b/libavfilter/buffersrc.c
@@ -240,10 +240,8 @@  static int av_buffersrc_add_frame_internal(AVFilterContext *ctx,
     }
 
     ret = ff_filter_frame(ctx->outputs[0], copy);
-    if (ret < 0) {
-        av_frame_free(&copy);
+    if (ret < 0)
         return ret;
-    }
 
     if ((flags & AV_BUFFERSRC_FLAG_PUSH)) {
         ret = push_frame(ctx->graph);