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

[Michael Niedermayer]

On Sat, Nov 15, 2008 at 10:05:05AM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Sat, Nov 15, 2008 at 7:01 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Fri, Nov 14, 2008 at 05:56:02PM -0500, Ronald S. Bultje wrote:
> >> +    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
> 
> With "this", you mean the random assigning of rmctx->pb? I can make
> that stop (e.g. a PayloadContext->pb, and use a pointer to that as
> rmctx->pb), but the data in it is local too, so I'd have to use
> url_resetbuf() instead of this rmctx->pb=NULL. Let me know if you like
> that better.

what is the problem of just passing pb as argument to the functions that need
it?
Or to place pb in the context instead of a pointer to it?

[...]
>          ff_rm_retrieve_cache (rdt->rmctx, st, pkt);
>          if (rm->audio_pkt_cnt == 0 &&
> -            st->codec->codec_id == CODEC_ID_AAC)
> -            url_close_buf (pb);
> +            st->codec->codec_id == CODEC_ID_AAC) {
> +            av_freep(&rdt->rmctx->pb);
> +        }

cosmetic adding of {}

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

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- 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/20081116/627fc980/attachment.pgp>