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

[Laurent Aimar]

On Mon, Aug 25, 2008, Pierre d'Herbemont wrote:
> 
> 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.

Read my second mail, sorry for that.
> 
> >> And that convention is really counter intuitive. (and
> >> prone to leak)
> > Which convention ?
> 
> Passing ownership  to the calling function.

We give the ownership to the decoder/packetizer, so it is perfectly sane.

And this way, refcounting is strictly useless (unless you want to share a
buffer but then you have to ensure that no one modify it, which we don't).

-- 
fenrir