[FFmpeg-devel] [PATCH] fix small memleak in rdt.c

[Michael Niedermayer]

On Fri, Nov 14, 2008 at 05:56:02PM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Fri, Nov 14, 2008 at 4:48 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> [..]
> 
> I'll leave the discussion on removing / renaming etc. in the other
> thread, makes it simpler to keep track...
> 
> > On Fri, Nov 14, 2008 at 03:17:26PM -0500, Ronald S. Bultje wrote:
> >> -    url_close_buf(pb);
> >> +    url_close_buf(&pb);
> >
> > this isnt intended to stay or?
> 
> I guess it can go, it doesn't do anything. Removed in this version. I
> used av_alloc_put_byte() as suggested for the remaining occurrence.
> 
> >> @@ -250,32 +251,33 @@
> >>                    const uint8_t *buf, int len, int flags)
> >>  {
> >>      int seq = 1, res;
> >> -    ByteIOContext *pb = rdt->rmctx->pb;
> >> +    ByteIOContext pb;
> > [...]
> >> +        rdt->rmctx->pb = &pb;
> >
> > you know that pb is on the stack and this stores it in a context that i
> > do not know if it might live longer than the local variables ...
> 
> Yes, I added = NULL statements to make it clearer, but they're only
> used within the local context, never after the function has returned.
[...]

> +    rdt->rmctx->pb = &pb;
>      if (ff_rm_read_mdpr_codecdata(rdt->rmctx, st, size) < 0)
>          return -1;
> +    rdt->rmctx->pb = NULL;

what about the return? this pointer will still point to the local
stack in that case, its hard to proof no code will atempt to use this
pointer now and in the future ...
i simply think this design is a little ugly

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081115/b9487fe3/attachment.pgp>