[vlc-devel] [PATCH 1/9] playlist: assign unique id to playlist items

[Rémi Denis-Courmont]

Le maanantaina 4. maaliskuuta 2019, 17.21.21 EET Romain Vimont a écrit :
> On Mon, Mar 04, 2019 at 05:06:22PM +0200, Rémi Denis-Courmont wrote:
> > Le maanantaina 4. maaliskuuta 2019, 16.28.27 EET Romain Vimont a écrit :
> > > On Mon, Mar 04, 2019 at 04:07:40PM +0200, Rémi Denis-Courmont wrote:
> > > > We also had assertions before. That did not prevent horrors along the
> > > > line
> > > > of lock/get/unlock ... lock/use/unlock.
> > > > 
> > > > In other words an assertion is not enough. We'd need a way to
> > > > invalidate
> > > > the item after unlocking.
> > > 
> > > The id is just a uint64_t, and it typically survives lock/unlock blocks.
> > > 
> > > The typical scenario is to retrieve the id when we have the real item
> > > 
> > > (e.g. to send the playlist content to the HTTP client):
> > >     lock()
> > >     item = Get(index)
> > >     id = GetId(item)
> > >     // ...
> > >     unlock()
> > 
> > The intended scenario is fine and all. That won't prevent the exact same
> > misuse> 
> > as with the legacy playlist:
> > 	lock()
> > 	item = Get(index)
> > 	unlock()
> > 	...
> > 	lock()
> > 	do_something(item)
> > 	unlock()
> > 
> > The assertion is helpless to catch that.
> 
> Yes, but this is independant of this patch about id.

In theory, you can indeed unlock and re-lock incorrectly even without 
identifiers. But in practice, there are hardly any way to get this wrong 
without identifiers, so it's not really independent.

And this is by far the most common bug with the old playlist.

-- 
雷米‧德尼-库尔蒙
http://www.remlab.net/