[FFmpeg-devel] [PATCH v5 2/3] avformat/tee: Fix leaks in tee muxer when open_slave fails

[Marton Balint]

On Thu, 14 Apr 2016, Marton Balint wrote:

>
> On Tue, 12 Apr 2016, sebechlebskyjan at gmail.com wrote:
>
>> From: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
>>
>> Calling close_slave in case error is to be returned from open_slave
>> will free allocated resources.
>>
>> Since failure can happen before bsfs array is initialized,
>> close_slave must check that bsfs is not NULL before accessing
>> tee_slave->bsfs[i] element.
>>
>> Slave muxer expects write_trailer to be called if it's
>> write_header suceeded (so resources allocated in write_header
>> are freed). Therefore if failure happens after successfull
>> write_header call, we must ensure that write_trailer of
>> that particular slave is called.
>
> Hmm, I guess you are right, I see no other way freeing 
> resources allocated in write_header then calling write_trailer. It does 
> make the code a bit more complex, but I don't really see a way to make it 
> more simple.
>
> So this looks good to me. Nicolas, any ideas improving this?
>

Actually I have given this some additional thought, and by using a new 
per-slave variable to keep the information if the header was written or 
not, I think your patch can be simplifed, also close_slave can be changed 
so it will write the trailer if necessary, in fact, the write_trailer 
function can use it as well.

I have modified your patch (see attached), could you please test/review it 
and check if I missed anything? I hope you don't mind, this kind of 
collaborative work is not that common in ffmpeg, but in this case it 
seemed easier moving those few lines around than describing what I wanted.

Thanks,
Marton
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avformat-tee-Fix-leaks-in-tee-muxer-when-open_slave-.patch
Type: text/x-patch
Size: 4999 bytes
Desc: 
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160415/aa416a3c/attachment.bin>