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

[Ronald S. Bultje]

Hi,

On Sun, Nov 16, 2008 at 1:35 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sat, Nov 15, 2008 at 10:05:05AM -0500, Ronald S. Bultje wrote:
>> 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?

No problem, I just thought that'd be ugly. I can do that if preferred.

> Or to place pb in the context instead of a pointer to it?

The buffer (for non-AC3 audio) that I open only lives in the local
context, so although pb itself could be accessed, the data in it would
still point to a no-longer-existing buffer, thus resulting in
virtually the same problem.

I'll do a patch that uses the first suggested solution.

Ronald