[FFmpeg-devel] [PATCH] Fix leak in mov.c

[Albert Astals Cid]

El Wednesday 21 May 2008 19:23:22 Baptiste Coudurier va escriure:
> Hi,
>
> Albert Astals Cid wrote:
> > El Wednesday 21 May 2008 00:59:12 Baptiste Coudurier va escriure:
> >> [...]
> >>
> >> Code must be under #ifdef CONFIG_ZLIB, otherwise it is just dead code.
> >
> > Right.
> >
> > [...]
> >
> > Index: mov.c
> > ===================================================================
> > --- mov.c	(revision 13199)
> > +++ mov.c	(working copy)
> > @@ -1586,9 +1586,9 @@
> >      }
> >      get_buffer(pb, cmov_data, cmov_len);
> >      if(uncompress (moov_data, (uLongf *) &moov_len, (const Bytef
> > *)cmov_data, cmov_len) != Z_OK) -        return -1;
> > +        goto error;
> >      if(init_put_byte(&ctx, moov_data, moov_len, 0, NULL, NULL, NULL,
> > NULL) != 0) -        return -1;
> > +        goto error;
> >      atom.type = MKTAG('m','o','o','v');
> >      atom.offset = 0;
> >      atom.size = moov_len;
> > @@ -1599,6 +1599,10 @@
> >      av_free(moov_data);
> >      av_free(cmov_data);
> >      return ret;
> > +error:
> > +    av_free(moov_data);
> > +    av_free(cmov_data);
> > +    return -1;
> >  #else
> >      av_log(c->fc, AV_LOG_ERROR, "this file requires zlib support
> > compiled in\n"); return -1;
>
> Seems there is still code duplication here :)

Do you manage to get any new contributor with such nitpicking?

Can't YOU fix the code duplication, the important part of my mail is that it 
informs you of a leak that should be fixed and that you did not know it 
existed.

It's you that know how exactly you want the code written, not me, i've still 
not developed a mind reader although i'd like and if you don't tell me what 
you want i can only guess and we'll be in this ridiculous ping-pong game 
loosing my and your time.

So either tellme exactly how you want the patch (do you want me to set ret 
to -1 in the if and put the error label before the existant av_free? do you 
want me to join both if in a single condition to save a goto?) or fix it 
yourself.

Yours trully,
  Albert