[vlc-devel] commit: Fix memleak (should fix #1081) ( Rémi ?Duraffort )

[Pierre d'Herbemont]

On Aug 25, 2008, at 1:39 AM, Laurent Aimar wrote:

> On Mon, Aug 25, 2008, Pierre d'Herbemont wrote:
>>
>> On Aug 25, 2008, at 12:35 AM, Ilkka Ollakka wrote:
>>
>>> On su 24. elokuuta 2008 21:14:35, git version control wrote:
>>>> vlc | branch: master | Rémi Duraffort <ivoire at videolan.org> | Sun
>>>> Aug 24 20:56:56 2008 +0200|
>>>> [4774dc6c9f7b04b40f90e4edc9ad1a1f8d256f1d] | committer: Rémi
>>>> Duraffort
>>>>
>>>> Fix memleak (should fix #1081)
>>>>
>>>
>>> This seems to cause segfault for me when module is unloaded in 64bit
>>> linux.
>>
>> The patch is wrong anyway: strangely in_block ownership goes to the
>> packetizer. IMO, we don't have refcounting for block, and that  
>> doesn't
>> seem right.
> You do not need block refcounting (and it would not be that easy, as a
> nearly anything that works with block can modify its content).

You can always not use refcounting :) but it may helps to get a sound  
memory management system. This doesn't seem to be the case with  
block_t. (And I don't see the point about content modifying, but it's  
true that I don't know much about block_t)

> Anyway, if a packetizer releases an input block it will set the block
> pointer to NULL if not, the block is not released.

And here, that's exactly why it doesn't work. (In mpga.c the in and  
out block are distinguished. Ownership of the input block is lost  
after calling pf_packetizer.)

> The ownership of the input block always stays to the caller.

huh? If the packetizer is responsible to release the input block then  
it means that ownership goes to it.

I understand that here we return a new block as return value. Of  
course ownership of that new blocks goes to the caller. And if the  
return value is NULL then it won't have to release it. But this is for  
the *output* block.

>> And that convention is really counter intuitive. (and
>> prone to leak)
> Which convention ?

Passing ownership  to the calling function.

Pierre.