[07/18] rv34: Fix a memory leak on errors
diff mbox

Message ID 1379358389-64839-7-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö Sept. 16, 2013, 7:06 p.m. 
---
 libavcodec/rv34.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Diego Biurrun Sept. 16, 2013, 9:27 p.m. | #1 
On Mon, Sep 16, 2013 at 10:06:18PM +0300, Martin Storsjö wrote:
> --- a/libavcodec/rv34.c
> +++ b/libavcodec/rv34.c
> @@ -1495,8 +1495,10 @@ av_cold int ff_rv34_decode_init(AVCodecContext *avctx)
>  
> -    if ((ret = rv34_decoder_alloc(r)) < 0)
> +    if ((ret = rv34_decoder_alloc(r)) < 0) {
> +        ff_MPV_common_end(&r->s);
>          return ret;
> +    }

Don't you have to do the same thing on line 1520?

Diego
Martin Storsjö Sept. 17, 2013, 9:13 a.m. | #2 
On Mon, 16 Sep 2013, Diego Biurrun wrote:

> On Mon, Sep 16, 2013 at 10:06:18PM +0300, Martin Storsjö wrote:
>> --- a/libavcodec/rv34.c
>> +++ b/libavcodec/rv34.c
>> @@ -1495,8 +1495,10 @@ av_cold int ff_rv34_decode_init(AVCodecContext *avctx)
>>
>> -    if ((ret = rv34_decoder_alloc(r)) < 0)
>> +    if ((ret = rv34_decoder_alloc(r)) < 0) {
>> +        ff_MPV_common_end(&r->s);
>>          return ret;
>> +    }
>
> Don't you have to do the same thing on line 1520?

Possibly - I'm not sure how the error handling/cleanup is handled for the 
threading case off-hand (the sample I have doesn't get that far). Janne or 
Kostya?

// Martin
Kostya Shishkov Sept. 17, 2013, 10 a.m. | #3 
On Tue, Sep 17, 2013 at 12:13:49PM +0300, Martin Storsjö wrote:
> On Mon, 16 Sep 2013, Diego Biurrun wrote:
> 
> >On Mon, Sep 16, 2013 at 10:06:18PM +0300, Martin Storsjö wrote:
> >>--- a/libavcodec/rv34.c
> >>+++ b/libavcodec/rv34.c
> >>@@ -1495,8 +1495,10 @@ av_cold int ff_rv34_decode_init(AVCodecContext *avctx)
> >>
> >>-    if ((ret = rv34_decoder_alloc(r)) < 0)
> >>+    if ((ret = rv34_decoder_alloc(r)) < 0) {
> >>+        ff_MPV_common_end(&r->s);
> >>         return ret;
> >>+    }
> >
> >Don't you have to do the same thing on line 1520?
> 
> Possibly - I'm not sure how the error handling/cleanup is handled
> for the threading case off-hand (the sample I have doesn't get that
> far). Janne or Kostya?

No idea either.
Janne Grunau Sept. 18, 2013, 7:57 p.m. | #4 
On 2013-09-17 12:13:49 +0300, Martin Storsjö wrote:
> On Mon, 16 Sep 2013, Diego Biurrun wrote:
> 
> > On Mon, Sep 16, 2013 at 10:06:18PM +0300, Martin Storsjö wrote:
> >> --- a/libavcodec/rv34.c
> >> +++ b/libavcodec/rv34.c
> >> @@ -1495,8 +1495,10 @@ av_cold int ff_rv34_decode_init(AVCodecContext *avctx)
> >>
> >> -    if ((ret = rv34_decoder_alloc(r)) < 0)
> >> +    if ((ret = rv34_decoder_alloc(r)) < 0) {
> >> +        ff_MPV_common_end(&r->s);
> >>          return ret;
> >> +    }
> >
> > Don't you have to do the same thing on line 1520?
> 
> Possibly - I'm not sure how the error handling/cleanup is handled for the 
> threading case off-hand (the sample I have doesn't get that far). Janne or 
> Kostya?

Yes, it needs the same change. It's probably hard to hit in practice but
it would leak memory on the error path.

Janne

Patch
diff mbox 

diff --git a/libavcodec/rv34.c b/libavcodec/rv34.c
index af37855..d182d39 100644
--- a/libavcodec/rv34.c
+++ b/libavcodec/rv34.c
@@ -1495,8 +1495,10 @@  av_cold int ff_rv34_decode_init(AVCodecContext *avctx)
         ff_rv40dsp_init(&r->rdsp);
 #endif
 
-    if ((ret = rv34_decoder_alloc(r)) < 0)
+    if ((ret = rv34_decoder_alloc(r)) < 0) {
+        ff_MPV_common_end(&r->s);
         return ret;
+    }
 
     if(!intra_vlcs[0].cbppattern[0].bits)
         rv34_init_tables();