diff mbox

[FFmpeg-devel,2/2] libschroedingerdec: fix leaking of framewithpts

Message ID 301ff917-83b3-7036-c3e4-bbfe84279e59@googlemail.com
State Superseded
Headers show

Commit Message

Andreas Cadhalpun Nov. 13, 2016, 10:25 p.m. UTC 
Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
---
 libavcodec/libschroedingerdec.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

Comments

Michael Niedermayer Nov. 15, 2016, 2:18 a.m. UTC | #1 
On Sun, Nov 13, 2016 at 11:25:32PM +0100, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
> ---
>  libavcodec/libschroedingerdec.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/libschroedingerdec.c b/libavcodec/libschroedingerdec.c
> index 1e392b3..83c790c 100644
> --- a/libavcodec/libschroedingerdec.c
> +++ b/libavcodec/libschroedingerdec.c
> @@ -218,6 +218,7 @@ static int libschroedinger_decode_frame(AVCodecContext *avctx,
>      int outer = 1;
>      SchroParseUnitContext parse_ctx;
>      LibSchroFrameContext *framewithpts = NULL;
> +    int ret;
>  
>      *got_frame = 0;
>  
> @@ -236,7 +237,8 @@ static int libschroedinger_decode_frame(AVCodecContext *avctx,
>              enc_buf->tag = schro_tag_new(av_malloc(sizeof(int64_t)), av_free);
>              if (!enc_buf->tag->value) {
>                  av_log(avctx, AV_LOG_ERROR, "Unable to allocate SchroTag\n");
> -                return AVERROR(ENOMEM);
> +                ret = AVERROR(ENOMEM);
> +                goto end;
>              }
>              AV_WN(64, enc_buf->tag->value, pts);
>              /* Push buffer into decoder. */
> @@ -267,8 +269,10 @@ static int libschroedinger_decode_frame(AVCodecContext *avctx,
>                  /* Decoder needs a frame - create one and push it in. */
>                  frame = ff_create_schro_frame(avctx,
>                                                p_schro_params->frame_format);
> -                if (!frame)
> -                    return AVERROR(ENOMEM);
> +                if (!frame) {
> +                    ret = AVERROR(ENOMEM);
> +                    goto end;
> +                }
>                  schro_decoder_add_output_picture(decoder, frame);
>                  break;
>  

this looks a bit strange
framewithpts is set to newly allocated memory below which is injected
into the que and IIUC that can occur multiple times
the free at the end for one of multiple such que entries feels wrong

[...]
diff mbox

Patch

diff --git a/libavcodec/libschroedingerdec.c b/libavcodec/libschroedingerdec.c
index 1e392b3..83c790c 100644
--- a/libavcodec/libschroedingerdec.c
+++ b/libavcodec/libschroedingerdec.c
@@ -218,6 +218,7 @@  static int libschroedinger_decode_frame(AVCodecContext *avctx,
     int outer = 1;
     SchroParseUnitContext parse_ctx;
     LibSchroFrameContext *framewithpts = NULL;
+    int ret;
 
     *got_frame = 0;
 
@@ -236,7 +237,8 @@  static int libschroedinger_decode_frame(AVCodecContext *avctx,
             enc_buf->tag = schro_tag_new(av_malloc(sizeof(int64_t)), av_free);
             if (!enc_buf->tag->value) {
                 av_log(avctx, AV_LOG_ERROR, "Unable to allocate SchroTag\n");
-                return AVERROR(ENOMEM);
+                ret = AVERROR(ENOMEM);
+                goto end;
             }
             AV_WN(64, enc_buf->tag->value, pts);
             /* Push buffer into decoder. */
@@ -267,8 +269,10 @@  static int libschroedinger_decode_frame(AVCodecContext *avctx,
                 /* Decoder needs a frame - create one and push it in. */
                 frame = ff_create_schro_frame(avctx,
                                               p_schro_params->frame_format);
-                if (!frame)
-                    return AVERROR(ENOMEM);
+                if (!frame) {
+                    ret = AVERROR(ENOMEM);
+                    goto end;
+                }
                 schro_decoder_add_output_picture(decoder, frame);
                 break;
 
@@ -282,7 +286,8 @@  static int libschroedinger_decode_frame(AVCodecContext *avctx,
                     framewithpts = av_malloc(sizeof(LibSchroFrameContext));
                     if (!framewithpts) {
                         av_log(avctx, AV_LOG_ERROR, "Unable to allocate FrameWithPts\n");
-                        return AVERROR(ENOMEM);
+                        ret = AVERROR(ENOMEM);
+                        goto end;
                     }
                     framewithpts->frame = frame;
                     framewithpts->pts   = AV_RN64(tag->value);
@@ -298,7 +303,8 @@  static int libschroedinger_decode_frame(AVCodecContext *avctx,
                 break;
 
             case SCHRO_DECODER_ERROR:
-                return -1;
+                ret = -1;
+                goto end;
                 break;
             }
         }
@@ -308,10 +314,9 @@  static int libschroedinger_decode_frame(AVCodecContext *avctx,
     framewithpts = ff_schro_queue_pop(&p_schro_params->dec_frame_queue);
 
     if (framewithpts && framewithpts->frame && framewithpts->frame->components[0].stride) {
-        int ret;
 
-        if ((ret = ff_get_buffer(avctx, avframe, 0)) < 0)
-            return ret;
+        if ((ret = ff_get_buffer(avctx, avframe, 0)) < 0) {}
+            goto end;
 
         memcpy(avframe->data[0],
                framewithpts->frame->components[0].data,
@@ -345,7 +350,10 @@  FF_ENABLE_DEPRECATION_WARNINGS
         data       = NULL;
         *got_frame = 0;
     }
-    return buf_size;
+    ret = buf_size;
+end:
+    av_freep(&framewithpts);
+    return ret;
 }